Better handling of UPDATE multiple-assignment row expressions

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: pgsql-hackers(at)postgreSQL(dot)org
Subject: Better handling of UPDATE multiple-assignment row expressions
Date: 2016-11-21 15:26:48
Message-ID: 8542.1479742008@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

While fooling around trying to document the behavior of *-expansion,
I got annoyed about the fact that it doesn't work in the context of
UPDATE tab SET (c1,c2,c3) = (x,y,z) ...
The righthand side of this is a row-expression according to the SQL
standard, and "foo.*" would be expanded in any other row expression,
but we don't do that here. Another oddity is that this ought to be
an abbreviated form of
UPDATE tab SET (c1,c2,c3) = ROW(x,y,z) ...
but we don't accept that.

It seemed like fixing this might be a good lazy-Sunday project ... so
here is a patch.

The reason this case doesn't work like other row-expressions is
that gram.y bursts the construct into
UPDATE tab SET c1 = x, c2 = y, c3 = z ...
before we ever do parse analysis. gram.y has no idea what "foo.*"
might expand to, so there's no way that that approach can work if
we want to fix this; the conversion has to be postponed to parse
analysis. The reason we didn't do it like that originally is
explained in gram.y:

* Ideally, we'd accept any row-valued a_expr as RHS of a multiple_set_clause.
* However, per SQL spec the row-constructor case must allow DEFAULT as a row
* member, and it's pretty unclear how to do that (unless perhaps we allow
* DEFAULT in any a_expr and let parse analysis sort it out later?).

But it turns out that the idea suggested in the parenthetical comment
is pretty easy to do. The attached patch allows DEFAULT in any a_expr
and then makes transformExpr() throw an error for it if it wasn't handled
by earlier processing. That allows improved error reporting: instead of
getting "syntax error" when DEFAULT appears someplace you can't put it,
you get a more specific error. For instance, this:

regression=# UPDATE update_test SET (a,b) = (default, default+1);
ERROR: syntax error at or near "+"
LINE 1: UPDATE update_test SET (a,b) = (default, default+1);
^

changes to:

regression=# UPDATE update_test SET (a,b) = (default, default+1);
ERROR: DEFAULT is not allowed in this context
LINE 1: UPDATE update_test SET (a,b) = (default, default+1);
^

As stated in the comment I just quoted, ideally we'd allow any suitable
composite-valued a_expr on the RHS of this kind of SET clause. The
attached patch doesn't really move the goal posts on that: you can still
only use a RowExpr or a sub-SELECT. But the RowExpr is now processed in
standard fashion, and we've at least gotten that restriction out of
gram.y and pushed it down one more level.

Anyway, the user-visible effects of this are:

* You can now write ROW explicitly in this construct, which you should
be able to per SQL spec.

* Expansion of "foo.*" in the row-expression now works as expected
(see regression test changes).

* Some error conditions now give errors more specific than "syntax error",
specifically misplacement of DEFAULT and attempting to use an unsupported
kind of expression on the RHS of this kind of SET clause.

I couldn't find anything that seemed worth changing in the existing
docs for any of these points, although if we don't fix the second point,
it'd require adjustments to the new docs I proposed at
https://www.postgresql.org/message-id/16288.1479610770@sss.pgh.pa.us

Comments, better ideas? Anyone want to do a formal review of this?

regards, tom lane

Attachment Content-Type Size
update-multiassign-improvement.patch text/x-diff 30.7 KB

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2016-11-21 15:33:10 Re: [sqlsmith] Parallel worker crash on seqscan
Previous Message Robert Haas 2016-11-21 15:15:33 Re: condition variables