Thread: Reconstructing Insert queries with indirection

Reconstructing Insert queries with indirection

From
Ashutosh Bapat
Date:
Hi All,<br />Consider following sequence of commands<br /><br />create type complex as (r float8, i float8);<br />
createtype quad as (c1 complex, c2 complex);<br /> create temp table quadtable(f1 int, q quad);<br /><br clear="all"
/>insertinto quadtable (f1, q.c1.r, q.c2.i) values(44,55,66);<br /><br />While parsing the INSERT query, we parse the
querywith three columns and three values in the target list, but during rewriting we combine q.c1.r and q.c2.i into a
singlecolumn in the form of FieldStore structure. In Postgres-XC, we deparse these parse trees, to be sent to other
PostgreSQLservers. The function processIndirection(), which deparses the indirections, can not handle more than one
fieldin FieldStore node.<br /><br />7344             /*<br />7345              * Print the field name.  There should
onlybe one target field in<br />7346              * stored rules.  There could be more than that in executable<br
/>7347             * target lists, but this function cannot be used for that case.<br /> 7348              */<br
/>7349            Assert(list_length(fstore->fieldnums) == 1);<br />7350             fieldname =
get_relid_attribute_name(typrelid,<br/>7351                                            
linitial_int(fstore->fieldnums));<br/> 7352             if (printit)<br />7353                 appendStringInfo(buf,
".%s",quote_identifier(fieldname));<br /><br />Why is this restriction here?<br /><br />The assertion is added by
commit858d1699. The notes for the commit have following paragraph related to FieldStore deparsing.<br /><br />    I
choseto represent an assignment ArrayRef as "array[subscripts] := source",<br />    which is fairly reasonable and
doesn'tomit any information.  However,<br />    FieldStore is problematic because the planner will fold multiple
assignments<br/>     to fields of the same composite column into one FieldStore, resulting in a<br />    structure that
ishard to understand at all, let alone display comprehensibly.<br />    So in that case I punted and just made it print
thesource expression(s).<br /><br />So, there doesn't seem to be any serious reason behind the restriction.<br /><br />
--<br />Best Wishes,<br />Ashutosh Bapat<br />EntepriseDB Corporation<br />The Enterprise Postgres Company<br /><br /> 

Re: Reconstructing Insert queries with indirection

From
Tom Lane
Date:
Ashutosh Bapat <ashutosh.bapat@enterprisedb.com> writes:
> Consider following sequence of commands

> create type complex as (r float8, i float8);
> create type quad as (c1 complex, c2 complex);
> create temp table quadtable(f1 int, q quad);

> insert into quadtable (f1, q.c1.r, q.c2.i) values(44,55,66);

> While parsing the INSERT query, we parse the query with three columns and
> three values in the target list, but during rewriting we combine q.c1.r and
> q.c2.i into a single column in the form of FieldStore structure. In
> Postgres-XC, we deparse these parse trees, to be sent to other PostgreSQL
> servers.

Well, basically you have a broken design there.  We are not going to
adopt a restriction that post-rewrite trees are necessarily exactly
representable as SQL, so there are going to be corner cases where this
approach fails.

> The assertion is added by commit 858d1699. The notes for the commit have
> following paragraph related to FieldStore deparsing.

>     I chose to represent an assignment ArrayRef as "array[subscripts] :=
> source",
>     which is fairly reasonable and doesn't omit any information.  However,
>     FieldStore is problematic because the planner will fold multiple
> assignments
>     to fields of the same composite column into one FieldStore, resulting
> in a
>     structure that is hard to understand at all, let alone display
> comprehensibly.
>     So in that case I punted and just made it print the source
> expression(s).

> So, there doesn't seem to be any serious reason behind the restriction.

If you have a proposal for some reasonable way to print the actual
meaning of the expression (and a patch to do it), we can certainly
consider changing that code.  I don't think it's possible to display it
as standard SQL, though.  The ArrayRef case is already not standard SQL.
        regards, tom lane


Re: Reconstructing Insert queries with indirection

From
Ashutosh Bapat
Date:


On Wed, Mar 21, 2012 at 10:58 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Ashutosh Bapat <ashutosh.bapat@enterprisedb.com> writes:
> Consider following sequence of commands

> create type complex as (r float8, i float8);
> create type quad as (c1 complex, c2 complex);
> create temp table quadtable(f1 int, q quad);

> insert into quadtable (f1, q.c1.r, q.c2.i) values(44,55,66);

> While parsing the INSERT query, we parse the query with three columns and
> three values in the target list, but during rewriting we combine q.c1.r and
> q.c2.i into a single column in the form of FieldStore structure. In
> Postgres-XC, we deparse these parse trees, to be sent to other PostgreSQL
> servers.

Well, basically you have a broken design there.  We are not going to
adopt a restriction that post-rewrite trees are necessarily exactly
representable as SQL, so there are going to be corner cases where this
approach fails.

That's an optimization, and in the cases it fails, we fall back to basics. If there are known differences, please let us know.
 
> The assertion is added by commit 858d1699. The notes for the commit have
> following paragraph related to FieldStore deparsing.

>     I chose to represent an assignment ArrayRef as "array[subscripts] :=
> source",
>     which is fairly reasonable and doesn't omit any information.  However,
>     FieldStore is problematic because the planner will fold multiple
> assignments
>     to fields of the same composite column into one FieldStore, resulting
> in a
>     structure that is hard to understand at all, let alone display
> comprehensibly.
>     So in that case I punted and just made it print the source
> expression(s).

> So, there doesn't seem to be any serious reason behind the restriction.

If you have a proposal for some reasonable way to print the actual
meaning of the expression (and a patch to do it), we can certainly
consider changing that code.  I don't think it's possible to display it
as standard SQL, though.  The ArrayRef case is already not standard SQL.


Let me try to come up with a patch.

                       regards, tom lane



--
Best Wishes,
Ashutosh Bapat
EntepriseDB Corporation
The Enterprise Postgres Company