Support UPDATE table SET(*)=...

Hi

On Wed, Oct 15, 2014 at 11:02 AM, Atri Sharma <atri.jiit@gmail.com> wrote:

Please find attached a patch which implements support for UPDATE table1
SET(*)=...

I presume you haven't read Tom Lane's proposal and discussion about
multiple column assignment in UPDATE:
/messages/by-id/1783.1399054541@sss.pgh.pa.us
(Assigning all columns was also discussed there)

And there's a WIP patch:
/messages/by-id/20930.1402931841@sss.pgh.pa.us

Regards,
Marti

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Wednesday, October 15, 2014, Marti Raudsepp <marti@juffo.org> wrote:

Hi

On Wed, Oct 15, 2014 at 11:02 AM, Atri Sharma <atri.jiit@gmail.com
<javascript:;>> wrote:

Please find attached a patch which implements support for UPDATE table1
SET(*)=...

I presume you haven't read Tom Lane's proposal and discussion about
multiple column assignment in UPDATE:
/messages/by-id/1783.1399054541@sss.pgh.pa.us
(Assigning all columns was also discussed there)

And there's a WIP patch:
/messages/by-id/20930.1402931841@sss.pgh.pa.us

Thanks for the links, but this patch only targets SET(*) case, which, if I
understand correctly, the patch you mentioned doesn't directly handle (If I
understand correctly, the target of the two patches is different).

Regards,

Atri

--
Regards,

Atri
*l'apprenant*

On Wed, Oct 15, 2014 at 2:18 PM, Atri Sharma <atri.jiit@gmail.com> wrote:

On Wednesday, October 15, 2014, Marti Raudsepp <marti@juffo.org> wrote:

Hi

On Wed, Oct 15, 2014 at 11:02 AM, Atri Sharma <atri.jiit@gmail.com>
wrote:

Please find attached a patch which implements support for UPDATE table1
SET(*)=...

I presume you haven't read Tom Lane's proposal and discussion about
multiple column assignment in UPDATE:
/messages/by-id/1783.1399054541@sss.pgh.pa.us
(Assigning all columns was also discussed there)

And there's a WIP patch:
/messages/by-id/20930.1402931841@sss.pgh.pa.us

Thanks for the links, but this patch only targets SET(*) case, which, if I
understand correctly, the patch you mentioned doesn't directly handle (If I
understand correctly, the target of the two patches is different).

Digging more, I figured that the patch I posted builds on top of Tom's
patch, since it did not add whole row cases.

Regards,

Atri

On Wed, Oct 15, 2014 at 3:48 AM, Atri Sharma <atri.jiit@gmail.com> wrote:

On Wednesday, October 15, 2014, Marti Raudsepp <marti@juffo.org> wrote:

Hi

On Wed, Oct 15, 2014 at 11:02 AM, Atri Sharma <atri.jiit@gmail.com> wrote:

Please find attached a patch which implements support for UPDATE table1
SET(*)=...

I presume you haven't read Tom Lane's proposal and discussion about
multiple column assignment in UPDATE:
/messages/by-id/1783.1399054541@sss.pgh.pa.us
(Assigning all columns was also discussed there)

And there's a WIP patch:
/messages/by-id/20930.1402931841@sss.pgh.pa.us

Thanks for the links, but this patch only targets SET(*) case, which, if I
understand correctly, the patch you mentioned doesn't directly handle (If I
understand correctly, the target of the two patches is different).

Yeah -- in fact, there was some discussion about this exact case.
This patch solves a very important problem: when doing record
operations to move data between databases with identical schema
there's currently no way to 'update' in a generic way without building
out the entire field list via complicated and nasty dynamic SQL. I'm
not sure about the proposed syntax though; it seems a little weird to
me. Any particular reason why you couldn't have just done:

UPDATE table1 SET * = a,b,c, ...

also,

UPDATE table1 t SET t = (SELECT (a,b,c)::t FROM...);

seems cleaner than the proposed syntax for row assignment. Tom
objected though IIRC.

merlin

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Fri, Oct 17, 2014 at 7:45 PM, Merlin Moncure <mmoncure@gmail.com> wrote:

On Wed, Oct 15, 2014 at 3:48 AM, Atri Sharma <atri.jiit@gmail.com> wrote:

On Wednesday, October 15, 2014, Marti Raudsepp <marti@juffo.org> wrote:

Hi

On Wed, Oct 15, 2014 at 11:02 AM, Atri Sharma <atri.jiit@gmail.com>

wrote:

Please find attached a patch which implements support for UPDATE

table1

SET(*)=...

I presume you haven't read Tom Lane's proposal and discussion about
multiple column assignment in UPDATE:
/messages/by-id/1783.1399054541@sss.pgh.pa.us
(Assigning all columns was also discussed there)

And there's a WIP patch:
/messages/by-id/20930.1402931841@sss.pgh.pa.us

Thanks for the links, but this patch only targets SET(*) case, which, if

I

understand correctly, the patch you mentioned doesn't directly handle

(If I

understand correctly, the target of the two patches is different).

Yeah -- in fact, there was some discussion about this exact case.
This patch solves a very important problem: when doing record
operations to move data between databases with identical schema
there's currently no way to 'update' in a generic way without building
out the entire field list via complicated and nasty dynamic SQL.

Thanks!

I'm
not sure about the proposed syntax though; it seems a little weird to
me. Any particular reason why you couldn't have just done:

UPDATE table1 SET * = a,b,c, ...

also,

UPDATE table1 t SET t = (SELECT (a,b,c)::t FROM...);

I honestly have not spent a lot of time thinking about the exact syntax
that may be acceptable. If we have arguments for or against a specific
syntax, I will be glad to incorporate them.

On 10/17/14 4:15 PM, Merlin Moncure wrote:

Any particular reason why you couldn't have just done:

UPDATE table1 SET * = a,b,c, ...

That just looks wrong to me. I'd prefer (*) = .. over that any day.

UPDATE table1 t SET t = (SELECT (a,b,c)::t FROM...);

seems cleaner than the proposed syntax for row assignment. Tom
objected though IIRC.

I don't know about Tom, but I didn't like that:
/messages/by-id/5364C982.7060003@joh.to

.marko

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Fri, Oct 17, 2014 at 9:55 AM, Marko Tiikkaja <marko@joh.to> wrote:

On 10/17/14 4:15 PM, Merlin Moncure wrote:

Any particular reason why you couldn't have just done:

UPDATE table1 SET * = a,b,c, ...

That just looks wrong to me. I'd prefer (*) = .. over that any day.

UPDATE table1 t SET t = (SELECT (a,b,c)::t FROM...);

seems cleaner than the proposed syntax for row assignment. Tom
objected though IIRC.

I don't know about Tom, but I didn't like that:
/messages/by-id/5364C982.7060003@joh.to

Hm, I didn't understand your objection:

<quoting>
So e.g.:
UPDATE foo f SET f = ..;

would resolve to the table, despite there being a column called "f"?
That would break backwards compatibility.
</quoting>

That's not correct: it should work exactly as 'select' does; given a
conflict resolve the field name, so no backwards compatibility issue.

merlin

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Merlin Moncure <mmoncure@gmail.com> writes:

On Wed, Oct 15, 2014 at 3:48 AM, Atri Sharma <atri.jiit@gmail.com> wrote:

Thanks for the links, but this patch only targets SET(*) case, which, if I
understand correctly, the patch you mentioned doesn't directly handle (If I
understand correctly, the target of the two patches is different).

Yeah -- in fact, there was some discussion about this exact case.
This patch solves a very important problem: when doing record
operations to move data between databases with identical schema
there's currently no way to 'update' in a generic way without building
out the entire field list via complicated and nasty dynamic SQL. I'm
not sure about the proposed syntax though; it seems a little weird to
me. Any particular reason why you couldn't have just done:

UPDATE table1 SET * = a,b,c, ...

also,

UPDATE table1 t SET t = (SELECT (a,b,c)::t FROM...);

seems cleaner than the proposed syntax for row assignment. Tom
objected though IIRC.

That last proposal is no good because it would be ambiguous if the
table contains a column by that name. The "(*)" idea actually seems
not bad, since it's shorthand for a parenthesized column list.

I'm not sure about the patch itself though --- in particular, it
doesn't seem to me that it should be touching transformTargetList,
since that doesn't have anything to do with expansion of multiassignments
today. Probably this is a symptom of having chosen a poor representation
of the construct in the raw grammar output. However, I've not exactly
wrapped my head around what that representation is ... the lack of any
comments explaining it doesn't help.

A larger question is whether it's appropriate to do the *-expansion
at parse time, or whether we'd need to postpone it to later in order
to handle reasonable use-cases. Since we expand "SELECT *" at parse
time (and are mandated to do so by the SQL spec, I believe), it seems
consistent to do this at parse time as well; but perhaps there is a
counter argument.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 10/17/14 5:03 PM, Merlin Moncure wrote:

Hm, I didn't understand your objection:

<quoting>
So e.g.:
UPDATE foo f SET f = ..;

would resolve to the table, despite there being a column called "f"?
That would break backwards compatibility.
</quoting>

That's not correct: it should work exactly as 'select' does; given a
conflict resolve the field name, so no backwards compatibility issue.

local:marko=# show server_version;
server_version
----------------
9.1.13
(1 row)

local:marko=#* create table foo(f int);
CREATE TABLE
local:marko=#* update foo f set f=1;
UPDATE 0

This query would change meaning with your suggestion.

I'm not saying it would be a massive problem in practice, but I think we
should first consider options which don't break backwards compatibility,
even if some consider them "less clean".

.marko

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Merlin Moncure <mmoncure@gmail.com> writes:

On Fri, Oct 17, 2014 at 9:55 AM, Marko Tiikkaja <marko@joh.to> wrote:

I don't know about Tom, but I didn't like that:
/messages/by-id/5364C982.7060003@joh.to

Hm, I didn't understand your objection:

<quoting>
So e.g.:
UPDATE foo f SET f = ..;

would resolve to the table, despite there being a column called "f"?
That would break backwards compatibility.
</quoting>

That's not correct: it should work exactly as 'select' does; given a
conflict resolve the field name, so no backwards compatibility issue.

The point is that it's fairly messy (and bug-prone) to have a syntax
where we have to make an arbitrary choice between two reasonable
interpretations.

If you look back at the whole thread Marko's above-cited message is in,
we'd considered a bunch of different possible syntaxes for this, and
none of them had much support. The "(*)" idea actually is starting to
look pretty good to me.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Fri, Oct 17, 2014 at 10:10 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Merlin Moncure <mmoncure@gmail.com> writes:

On Fri, Oct 17, 2014 at 9:55 AM, Marko Tiikkaja <marko@joh.to> wrote:

I don't know about Tom, but I didn't like that:
/messages/by-id/5364C982.7060003@joh.to

Hm, I didn't understand your objection:

<quoting>
So e.g.:
UPDATE foo f SET f = ..;

would resolve to the table, despite there being a column called "f"?
That would break backwards compatibility.
</quoting>

That's not correct: it should work exactly as 'select' does; given a
conflict resolve the field name, so no backwards compatibility issue.

The point is that it's fairly messy (and bug-prone) to have a syntax
where we have to make an arbitrary choice between two reasonable
interpretations.

If you look back at the whole thread Marko's above-cited message is in,
we'd considered a bunch of different possible syntaxes for this, and
none of them had much support. The "(*)" idea actually is starting to
look pretty good to me.

Hm, I'll take it then.

merlin

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Marko Tiikkaja <marko@joh.to> writes:

local:marko=#* create table foo(f int);
CREATE TABLE
local:marko=#* update foo f set f=1;
UPDATE 0

This query would change meaning with your suggestion.

I think it wouldn't; Merlin is proposing that f would be taken as the
field name. A more realistic objection goes like this:

create table foo(f int, g int);
update foo x set x = (1,2); -- works
alter table foo add column x int;
update foo x set x = (1,2,3); -- no longer works

It's not a real good thing if a column addition or renaming can
so fundamentally change the nature of a query.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Fri, Oct 17, 2014 at 10:16 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Marko Tiikkaja <marko@joh.to> writes:

local:marko=#* create table foo(f int);
CREATE TABLE
local:marko=#* update foo f set f=1;
UPDATE 0

This query would change meaning with your suggestion.

I think it wouldn't; Merlin is proposing that f would be taken as the
field name. A more realistic objection goes like this:

create table foo(f int, g int);
update foo x set x = (1,2); -- works
alter table foo add column x int;
update foo x set x = (1,2,3); -- no longer works

It's not a real good thing if a column addition or renaming can
so fundamentally change the nature of a query.

That's exactly how SELECT works. I also dispute that the user should
be surprised in such cases.

merlin

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Merlin Moncure <mmoncure@gmail.com> writes:

On Fri, Oct 17, 2014 at 10:16 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

I think it wouldn't; Merlin is proposing that f would be taken as the
field name. A more realistic objection goes like this:

create table foo(f int, g int);
update foo x set x = (1,2); -- works
alter table foo add column x int;
update foo x set x = (1,2,3); -- no longer works

It's not a real good thing if a column addition or renaming can
so fundamentally change the nature of a query.

That's exactly how SELECT works. I also dispute that the user should
be surprised in such cases.

Well, the reason we have a problem in SELECT is that we support an
unholy miscegenation of SQL-spec and PostQUEL syntax: the idea that
"SELECT foo FROM foo" could represent a whole-row selection is nowhere
to be found in the SQL standard, for good reason IMO. But we've never
had the courage to break cleanly with this Berkeley leftover and
insist that people spell it SQL-style as "SELECT ROW(foo.*) FROM foo".
I'd just as soon not introduce the same kind of ambiguity into UPDATE
if we have a reasonable alternative.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Fri, Oct 17, 2014 at 10:30 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Merlin Moncure <mmoncure@gmail.com> writes:

On Fri, Oct 17, 2014 at 10:16 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

I think it wouldn't; Merlin is proposing that f would be taken as the
field name. A more realistic objection goes like this:

create table foo(f int, g int);
update foo x set x = (1,2); -- works
alter table foo add column x int;
update foo x set x = (1,2,3); -- no longer works

It's not a real good thing if a column addition or renaming can
so fundamentally change the nature of a query.

That's exactly how SELECT works. I also dispute that the user should
be surprised in such cases.

Well, the reason we have a problem in SELECT is that we support an
unholy miscegenation of SQL-spec and PostQUEL syntax: the idea that
"SELECT foo FROM foo" could represent a whole-row selection is nowhere
to be found in the SQL standard, for good reason IMO. But we've never
had the courage to break cleanly with this Berkeley leftover and
insist that people spell it SQL-style as "SELECT ROW(foo.*) FROM foo".
I'd just as soon not introduce the same kind of ambiguity into UPDATE
if we have a reasonable alternative.

Ah, interesting point (I happen to like the terse syntax and use it
often). This is for posterity anyways since you guys seem to like
Atri's proposal, which surprised me. However, I think you're over
simplifying things here. Syntax aside: I think
SELECT f FROM foo f;
and a hypothetical
SELECT row(f.*) FROM foo f;

give different semantics. The former gives an object of type 'f' and
the latter gives type 'row'. To get parity, you'd have to add an
extra cast which means you'd have to play tricky games to avoid extra
performance overhead besides being significantly more verbose. I'm
aware some of the other QUELisms are pretty dodgy and have burned us
in the past (like that whole function as record reference thing) but
pulling a record as a field in select is pretty nice. It's also
widely used and quite useful in json serialization.

merlin

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Oct 17, 2014 6:16 PM, "Tom Lane" <tgl@sss.pgh.pa.us> wrote:

A more realistic objection goes like this:

create table foo(f int, g int);
update foo x set x = (1,2); -- works
alter table foo add column x int;
update foo x set x = (1,2,3); -- no longer works

It's not a real good thing if a column addition or renaming can
so fundamentally change the nature of a query.

I think a significant use case for this feature is when you already have a
row-value and want to persist it in the database, like you can do with
INSERT:
insert into foo select * from populate_record_json(null::foo, '{...}');

In this case the opposite is true: requiring explicit column names would
break the query if you add columns to the table. The fact that you can't
reasonably use populate_record/_json with UPDATE is a significant omission.
IMO this really speaks for supporting shorthand whole-row assignment,
whatever the syntax.

Regards,
Marti

On Fri, Oct 17, 2014 at 10:47 AM, Merlin Moncure <mmoncure@gmail.com> wrote:

On Fri, Oct 17, 2014 at 10:30 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Merlin Moncure <mmoncure@gmail.com> writes:

On Fri, Oct 17, 2014 at 10:16 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

I think it wouldn't; Merlin is proposing that f would be taken as the
field name. A more realistic objection goes like this:

create table foo(f int, g int);
update foo x set x = (1,2); -- works
alter table foo add column x int;
update foo x set x = (1,2,3); -- no longer works

It's not a real good thing if a column addition or renaming can
so fundamentally change the nature of a query.

That's exactly how SELECT works. I also dispute that the user should
be surprised in such cases.

Well, the reason we have a problem in SELECT is that we support an
unholy miscegenation of SQL-spec and PostQUEL syntax: the idea that
"SELECT foo FROM foo" could represent a whole-row selection is nowhere
to be found in the SQL standard, for good reason IMO. But we've never
had the courage to break cleanly with this Berkeley leftover and
insist that people spell it SQL-style as "SELECT ROW(foo.*) FROM foo".
I'd just as soon not introduce the same kind of ambiguity into UPDATE
if we have a reasonable alternative.

Ah, interesting point (I happen to like the terse syntax and use it
often). This is for posterity anyways since you guys seem to like
Atri's proposal, which surprised me. However, I think you're over
simplifying things here. Syntax aside: I think
SELECT f FROM foo f;
and a hypothetical
SELECT row(f.*) FROM foo f;

give different semantics. The former gives an object of type 'f' and
the latter gives type 'row'. To get parity, you'd have to add an
extra cast which means you'd have to play tricky games to avoid extra
performance overhead besides being significantly more verbose. I'm
aware some of the other QUELisms are pretty dodgy and have burned us
in the past (like that whole function as record reference thing) but
pulling a record as a field in select is pretty nice. It's also
widely used and quite useful in json serialization.

Been thinking about this in the back of my mind the last couple of
days. There are some things you can do with the QUEL 'table as
column' in select syntax that are impossible otherwise, at least
today, and its usage is going to proliferate because of that. Row
construction via () or row() needs to be avoided whenever the column
names are important and there is no handy type to cast to. For
example, especially during json serialization it's convenient to do
things like:

select
array_agg((select q from (select a, b) q) order by ...)
from foo;

...where a,b are fields of foo. FWICT, this is the only way to do
json serialization of arbitrary runtime row constructions in a way
that does not anonymize the type. Until I figured out this trick I
used to create lots of composite types that served no purpose other
than to give me a type to cast to which is understandably annoying.

if:
select (x).* from (select (1, 2) as x) q;

worked and properly expanded x to given names should they exist and:
SELECT row(f.*) FROM foo f;

worked and did same, and:
SELECT (row(f.*)).* FROM foo f;

was as reasonably performant and gave the same results as:
SELECT (f).* FROM foo f;

...then IMSNHO you'd have a reasonable path to deprecating the QUEL
inspired syntax. Food for thought.

merlin

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Hi Atri,

Sorry for the delay. With pgconf.eu and all, it's been very hard to
find the time to look at this.

On 10/15/14, 10:02 AM, Atri Sharma wrote:

Please find attached a patch which implements support for UPDATE table1
SET(*)=...
The patch supports both UPDATE table SET(*)=(a,b,c) and UPDATE table1
SET(*)=(SELECT a,b,c FROM...).
It solves the problem of doing UPDATE from a record variable of the same
type as the table e.g. update foo set (*) = (select foorec.*) where ...;

Excellent! This is a very welcome change.

I've had a few looks at this patch and I have a few comments:

1) This doesn't work for the zero-column table case at all:
CREATE TABLE foo();
UPDATE foo SET (*) = (SELECT);
ERROR: number of columns does not match number of values
2) What's the purpose of the second condition here?
if (!(origTarget) || !(origTarget->name))
3) The extra parentheses around everything make this code for some
reason very hard to read.
4) transformTargetList() is a mess right now. If this is the
approach we want to take, the common code should probably be refactored
into a function. But the usage of List as a somehow magical way to
represent the SET (*) case makes me feel weird inside.
5) The complete lack of regression tests make it hard to poke around
the code to try and figure out what each line/condition is trying to do.

I feel like I understand what this code is doing and some details feel a
bit icky, but I'm not the right person to comment on whether the broad
strokes are on the right canvas or not. Maybe someone else wants to
take a closer look before Atri spends too much time on this approach?
After all, this patch has a very distinctive WIP feel to it, so I guess
feedback on the general approach is what's being sought after here, and
in that area I consider my skills and knowledge lacking.

The design is simple. It basically expands the * in transformation stage,
does the necessary type checking and adds it to the parse tree. This allows
for normal execution for the rest of the stages.

I can't poke any big holes into this approach (disregarding the details
of this implementation), but perhaps someone else can?

.marko

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Marko Tiikkaja <marko@joh.to> writes:

On 10/15/14, 10:02 AM, Atri Sharma wrote:

Please find attached a patch which implements support for UPDATE table1
SET(*)=...

I've had a few looks at this patch and I have a few comments:

1) This doesn't work for the zero-column table case at all:
CREATE TABLE foo();
UPDATE foo SET (*) = (SELECT);
ERROR: number of columns does not match number of values
2) What's the purpose of the second condition here?
if (!(origTarget) || !(origTarget->name))
3) The extra parentheses around everything make this code for some
reason very hard to read.
4) transformTargetList() is a mess right now. If this is the
approach we want to take, the common code should probably be refactored
into a function. But the usage of List as a somehow magical way to
represent the SET (*) case makes me feel weird inside.
5) The complete lack of regression tests make it hard to poke around
the code to try and figure out what each line/condition is trying to do.

I feel like I understand what this code is doing and some details feel a
bit icky, but I'm not the right person to comment on whether the broad
strokes are on the right canvas or not. Maybe someone else wants to
take a closer look before Atri spends too much time on this approach?

FWIW, I opined upthread that transformTargetList was not the place to
be handling this; it should be done in the same place(s) that support
the existing MultiAssignRef feature, and transformTargetList is not
that.

I think what's likely missing here is a clear design for the raw parse
tree representation (what's returned by the bison grammar). The patch
seems to be trying to skate by without creating any new parse node types
or fields, but that may well be a bad idea. At the very least there
needs to be some commentary added to parsenodes.h explaining what the
representation actually is (cf commentary there for MultiAssignRef).

Also, I think it's a mistake not to be following the MultiAssignRef
model for the case of "(*) = ctext_row". We optimize the ROW-source
case at the grammar stage when there's a fixed number of target columns,
because that's a very easy transformation --- but you can't do it like
that when there's not. It's possible that that optimization should be
delayed till later even in the existing case; in general, optimizing
in gram.y is a bad habit that's better avoided ...

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers