Re: Reconstructing Insert queries with indirection

Hi All,
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. The function processIndirection(), which deparses the
indirections, can not handle more than one field in FieldStore node.

7344 /*
7345 * Print the field name. There should only be one target
field in
7346 * stored rules. There could be more than that in
executable
7347 * target lists, but this function cannot be used for that
case.
7348 */
7349 Assert(list_length(fstore->fieldnums) == 1);
7350 fieldname = get_relid_attribute_name(typrelid,
7351
linitial_int(fstore->fieldnums));
7352 if (printit)
7353 appendStringInfo(buf, ".%s",
quote_identifier(fieldname));

Why is this restriction here?

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.

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

Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)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

On Wed, Mar 21, 2012 at 10:58 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)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