Thread: Allow an alias for the target table in UPDATE/DELETE

I made a patch to allow an alias for target table in UPDATE/DELETE
This is a TODO item.
> o Allow an alias to be provided for the target table in UPDATE/DELETE
>   This is not SQL-spec but many DBMSs allow it.
Example:
  UPDATE accounts AS a SET a.abalance = a.abalance + 10 WHERE a.aid = 1;

I think that there are two viewpoints of this patch:
(1)I moved SET to reserved words to avoid shift/reduce conflicts.
It is because the parser confused by whether SET is a keyword or
an alias in SQL 'UPDATE tbl SET ...'.

(2)About processing when column identifier of SET clause is specified
like 'AAA.BBB'. 'AAA' is a composite column now. When an alias for
target table is supported, 'AAA' is a composite column or a table.
How do we distinguish these?

The syntax rule of the composite type is as follows:
-----------------------------------------------------------------------
SELECT item.name FROM on_hand WHERE item.price > 9.99;

This will not work since the name item is taken to be a table name,
not a field name, per SQL syntax rules. You must write it like this:

SELECT (item).name FROM on_hand WHERE (item).price > 9.99;
-----------------------------------------------------------------------
but...
-----------------------------------------------------------------------
We can update an individual subfield of a composite column:

UPDATE mytab SET complex_col.r = (complex_col).r + 1 WHERE ...;

Notice here that we don't need to (and indeed cannot) put parentheses
around the column name appearing just after SET, but we do need
parentheses when referencing the same column in the expression to
the right of the equal sign.
-----------------------------------------------------------------------

The syntax rule is different in SET clause and other clauses.
Incompatibility is caused if SET clause is changed to the same
rule as other clauses to allow an alias for target table.

To keep compatibility, I implemented the following rules:
Eat up a first element of column identifier when the first element
is a name or an alias for target table. And, make the second
element a column name.

Example:
  UPDATE tbl AS t SET t.col = 1;
  => 't' is eaten up, and 'col' becomes a column name.

--- Atsushi Ogawa

Atsushi Ogawa <atsushi.ogawa@gmail.com> writes:
> (2)About processing when column identifier of SET clause is specified
> like 'AAA.BBB'. 'AAA' is a composite column now. When an alias for
> target table is supported, 'AAA' is a composite column or a table.
> How do we distinguish these?

You don't, which is why you can't put an alias on a SET target.

Increasing the reserved-ness of SET isn't real attractive either.

            regards, tom lane

Thanks for comments. I modified the patch.

Tom Lane wrote:
> Atsushi Ogawa <atsushi.ogawa@gmail.com> writes:
> > (2)About processing when column identifier of SET clause is specified
> > like 'AAA.BBB'. 'AAA' is a composite column now. When an alias for
> > target table is supported, 'AAA' is a composite column or a table.
> > How do we distinguish these?
>
> You don't, which is why you can't put an alias on a SET target.

I stop applying an alias to a SET target.

> Increasing the reserved-ness of SET isn't real attractive either.

OK. I changed the syntax rule of an alias of UPDATE/DELETE target from
ColId to IDENT. This doesn't change reserved words though candidates
of that alias decreases.

--- Atsushi Ogawa

On Sat, 2005-12-03 at 10:42 +0900, Atsushi Ogawa wrote:
> Thanks for comments. I modified the patch.

Patch applied to HEAD.

From looking at SQL2003, it seems to me that this syntax is actually
specified by the standard:

<update statement: searched> ::=
    UPDATE <target table> [ [ AS ] <correlation name> ]
    SET <set clause list>
    [ WHERE <search condition> ]

<delete statement: searched> ::=
    DELETE FROM <target table> [ [ AS ] <correlation name> ]
    [ WHERE <search condition> ]

I think we ought to support using the alias in the SET clause,
particularly as the standard allows for it (AFAIK).

-Neil

Neil Conway <neilc@samurai.com> writes:
> From looking at SQL2003, it seems to me that this syntax is actually
> specified by the standard:

> <update statement: searched> ::=
>     UPDATE <target table> [ [ AS ] <correlation name> ]
>     SET <set clause list>
>     [ WHERE <search condition> ]

> <delete statement: searched> ::=
>     DELETE FROM <target table> [ [ AS ] <correlation name> ]
>     [ WHERE <search condition> ]

Interesting, because the AS clause is most definitely *not* in
SQL92 or SQL99.

I'm all for this change, in any case, but it's interesting to notice
that the SQL committee actually does respond to the masses ...

            regards, tom lane

Neil Conway <neilc@samurai.com> writes:
> Patch applied to HEAD.

This is wrong:

+relation_expr_opt_alias: relation_expr
+                {
+                    $$ = $1;
+                }
+            | relation_expr opt_as IDENT
+                {
+

Should be ColId, not IDENT, per existing alias_clause production.

Also, while I'm all for getting to 100 regression tests, this is a
mighty lame 100th entry.

            regards, tom lane

On Sun, 2006-01-22 at 00:55 -0500, Tom Lane wrote:
> This is wrong:
>
> +relation_expr_opt_alias: relation_expr
> +                {
> +                    $$ = $1;
> +                }
> +            | relation_expr opt_as IDENT
> +                {
> +
>
> Should be ColId, not IDENT, per existing alias_clause production.

That causes a reduce/reduce conflict:

state 557

  934 relation_expr_opt_alias: relation_expr .
  935                        | relation_expr . opt_as ColId

    AS  shift, and go to state 875

    $end      reduce using rule 934 (relation_expr_opt_alias)
    SET       reduce using rule 754 (opt_as)
    SET       [reduce using rule 934 (relation_expr_opt_alias)]
    USING     reduce using rule 934 (relation_expr_opt_alias)
    WHERE     reduce using rule 934 (relation_expr_opt_alias)
    ')'       reduce using rule 934 (relation_expr_opt_alias)
    ';'       reduce using rule 934 (relation_expr_opt_alias)
    $default  reduce using rule 754 (opt_as)

    opt_as  go to state 876

> Also, while I'm all for getting to 100 regression tests, this is a
> mighty lame 100th entry.

Why's that? We needed regression tests for the changes to DELETE (IMHO),
and I didn't see an existing test file where it would have made much
sense to add them. I don't think the barrier for adding a new regression
test should be particularly high, provided the test covers a clear set
of functionality (such as the "DELETE" statement).

-Neil

Neil Conway <neilc@samurai.com> writes:
> On Sun, 2006-01-22 at 00:55 -0500, Tom Lane wrote:
>> Should be ColId, not IDENT, per existing alias_clause production.

> That causes a reduce/reduce conflict:

The grammar change is the one marginally nontrivial part of the patch,
and you couldn't be bothered to get it right?  Try harder.

            regards, tom lane

On Sun, 2006-01-22 at 01:28 -0500, Tom Lane wrote:
> The grammar change is the one marginally nontrivial part of the patch,
> and you couldn't be bothered to get it right?  Try harder.

:-(

I believe the conflict results from the fact that ColId can include SET
(since it is an unreserved_keyword), but SET might also be the next
token in the UpdateStmt, and yacc is not capable of distinguishing
between these two cases. We could fix this by promoting SET to be a
func_name_keyword or reserved_keyword, but that seems unfortunate. Can
you see a better fix?

-Neil

Neil Conway <neilc@samurai.com> writes:
> Can you see a better fix?

I haven't done any experimentation, but my first instinct would be to
spell out the productions at greater length: instead of

    relation_expr opt_as ColId

try

    relation_expr ColId
    | relation_expr AS ColId

The normal game with bison is to postpone decisions (reductions) as
long as possible.  Shortcuts like opt_as lose that game because the
shift-versus-reduce decision has to be made with hardly any lookahead.

Or maybe some other hack is needed, but I seriously doubt it's
unfixable.

            regards, tom lane

On Sun, 2006-01-22 at 02:05 -0500, Neil Conway wrote:
> I believe the conflict results from the fact that ColId can include SET
> (since it is an unreserved_keyword), but SET might also be the next
> token in the UpdateStmt, and yacc is not capable of distinguishing
> between these two cases. We could fix this by promoting SET to be a
> func_name_keyword or reserved_keyword, but that seems unfortunate.

On thinking about this a bit more, an alternative fix would be to make
AS mandatory. That is unappealing because the SQL spec requires that it
be optional, as well as for consistency with aliases in SELECT's FROM
list.

Another possibility is to disallow SET here, but not in other places
where ColId is used. That is, some hack like:

ColId_no_set: IDENT | unreserved_keyword | col_name_keyword ;
ColId: ColId_no_set | SET ;

relation_expr_opt_alias: relation_expr
                    | relation_expr opt_as ColId_no_set

... along the corresponding changes to the other productions that deal
with keywords (removing SET from unreserved_keywords and adding it
manually as an alternative to type_name, function_name, etc.). Needless
to say, that is also very ugly.

-Neil

On Sun, 2006-01-22 at 02:23 -0500, Tom Lane wrote:
>     relation_expr opt_as ColId
>
> try
>
>     relation_expr ColId
>     | relation_expr AS ColId

Yeah, I already tried that without success. The no-AS-specified variant
is still ambiguous: given SET following relation_expr, the parser still
can't determine whether the SET is the table alias or is part of the
UpdateStmt.

-Neil

> Another possibility is to disallow SET here, but not in other places
> where ColId is used. That is, some hack like:

Quick point: If all you want to do if disallow SET here as ColID but
allow SET in other places with a ColId, you don't have to change
anything at all. Just make sure the rule for the want you prefer is
earlier in the file.

Shift/reduce and reduce/reduce errors still produce valid working
parsers, it's just that bison has to resolve an ambiguity by the
default (shift, otherwise earliest rule wins. maximum munch rule
really).

If you don't like relying on file order to resolve this, appropriate
use of %prec would have the same effect (just like for operator
precedence). The output file tell you which way bison went.

Have a nice day,
--
Martijn van Oosterhout   <kleptog@svana.org>   http://svana.org/kleptog/
> Patent. n. Genius is 5% inspiration and 95% perspiration. A patent is a
> tool for doing 5% of the work and then sitting around waiting for someone
> else to do the other 95% so you can sue them.

Martijn van Oosterhout wrote:

>Shift/reduce and reduce/reduce errors still produce valid working
>parsers, it's just that bison has to resolve an ambiguity by the
>default (shift, otherwise earliest rule wins. maximum munch rule
>really).
>
>If you don't like relying on file order to resolve this, appropriate
>use of %prec would have the same effect (just like for operator
>precedence). The output file tell you which way bison went.
>
>
>
>

If we allow shift/reduce or reduce/reduce conflicts, debugging future
development becomes more difficult. Right now we have the nice property
that if you see one of those you know you've done something wrong (and
using the expect directive isn't really a good answer, and only applies
to shift/reduce conflicts anyway).

cheers

andrew

On Sun, Jan 22, 2006 at 09:04:14AM -0500, Andrew Dunstan wrote:
> >If you don't like relying on file order to resolve this, appropriate
> >use of %prec would have the same effect (just like for operator
> >precedence). The output file tell you which way bison went.

> If we allow shift/reduce or reduce/reduce conflicts, debugging future
> development becomes more difficult. Right now we have the nice property
> that if you see one of those you know you've done something wrong (and
> using the expect directive isn't really a good answer, and only applies
> to shift/reduce conflicts anyway).

But that's the point of the %prec directive. To force bison to choose
one or the other, thus removing the warning... For an ambiguity that
only appears in one statement, it seems a better solution than upgrade
SET to a new class of identifier.

Have a nice day,
--
Martijn van Oosterhout   <kleptog@svana.org>   http://svana.org/kleptog/
> Patent. n. Genius is 5% inspiration and 95% perspiration. A patent is a
> tool for doing 5% of the work and then sitting around waiting for someone
> else to do the other 95% so you can sue them.

Martijn van Oosterhout wrote:

>On Sun, Jan 22, 2006 at 09:04:14AM -0500, Andrew Dunstan wrote:
>
>
>>>If you don't like relying on file order to resolve this, appropriate
>>>use of %prec would have the same effect (just like for operator
>>>precedence). The output file tell you which way bison went.
>>>
>>>
>
>
>
>>If we allow shift/reduce or reduce/reduce conflicts, debugging future
>>development becomes more difficult. Right now we have the nice property
>>that if you see one of those you know you've done something wrong (and
>>using the expect directive isn't really a good answer, and only applies
>>to shift/reduce conflicts anyway).
>>
>>
>
>But that's the point of the %prec directive. To force bison to choose
>one or the other, thus removing the warning... For an ambiguity that
>only appears in one statement, it seems a better solution than upgrade
>SET to a new class of identifier.
>
>
>

Quite so. We already use %prec extensively. All I was pointing out was
that using file order isn't an acceptable option.

Presumably the effect in this case would be to prevent anyone from using
SET as an alias unless there was a preceding AS.

cheers

andrew

Andrew Dunstan <andrew@dunslane.net> writes:
> Presumably the effect in this case would be to prevent anyone from using
> SET as an alias unless there was a preceding AS.

I fooled around with this and found three feasible alternatives.

The simplest thing is:

relation_expr_opt_alias: relation_expr
            | relation_expr IDENT
            | relation_expr AS ColId

which at least gets us to the point of being able to use anything you
normally can use as an alias in other contexts, so long as you write AS.
Given the large and ever-increasing list of unreserved_keywords, though,
I don't think this is sufficient.

Then there's the precedence way.  This seems to work:

431a432
> %nonassoc    SET
5883c5884
< relation_expr_opt_alias: relation_expr
---
> relation_expr_opt_alias: relation_expr        %prec UMINUS
5887c5888,5895
<             | relation_expr opt_as IDENT
---
>             | relation_expr ColId
>                 {
>                     Alias *alias = makeNode(Alias);
>                     alias->aliasname = $2;
>                     $1->alias = alias;
>                     $$ = $1;
>                 }
>             | relation_expr AS ColId

The effect of this, as Andrew says, is that in this particular context
you can't write SET as an alias unless you write AS first.  This is
probably not going to surprise anyone in the UPDATE case, since the
ambiguity inherent in writing
    UPDATE foo set SET ...
is pretty obvious.  However it might surprise someone in the DELETE
context.  Another thing that worries me is that assigning a precedence
to the SET keyword might someday bite us by masking some other
grammatical ambiguity.  (As far as I can tell by comparing y.output
files, it's not changing the behavior of any other part of the grammar
today, but ...)

The third alternative is to stop allowing SET as an unreserved_keyword.
I found that moving it up to func_name_keyword was enough to get rid
of the conflict, without using precedence.  This is somewhat attractive
on the grounds of future-proofing (we may have to make SET more reserved
someday anyway for something else); and you can argue that the principle
of least astonishment demands that SET should be either always OK as an
alias or always not OK.  On the other hand this would raise some
backwards-compatibility issues.  Is it likely that anyone out there is
using SET as a table or column name?

I could live with either choice #2 or choice #3.  Comments?

            regards, tom lane

Tom Lane wrote:

>The effect of this, as Andrew says, is that in this particular context
>you can't write SET as an alias unless you write AS first.  This is
>probably not going to surprise anyone in the UPDATE case, since the
>ambiguity inherent in writing
>    UPDATE foo set SET ...
>is pretty obvious.  However it might surprise someone in the DELETE
>context.
>
>

You probably avoid that if you have a separate rule for the DELETE case.
That raises this question: how far do we want to go in unfactoring the
grammar to handle such cases?

cheers

andrew

On Sun, 2006-01-22 at 13:26 -0500, Tom Lane wrote:
> The effect of this, as Andrew says, is that in this particular context
> you can't write SET as an alias unless you write AS first.

Did you actually test this?

neilc=# create table t1 (a int, b int);
CREATE TABLE
neilc=# update t1 set set a = 500 where set.a > 1000;
UPDATE 0

(Using essentially the patch you posted.)

> This is
> probably not going to surprise anyone in the UPDATE case, since the
> ambiguity inherent in writing
>     UPDATE foo set SET ...
> is pretty obvious.  However it might surprise someone in the DELETE
> context.

Well, if necessary we can just use an alternate production for the
DELETE case, as there is no SET ambiguity to worry about.

> The third alternative is to stop allowing SET as an unreserved_keyword.
> I found that moving it up to func_name_keyword was enough to get rid
> of the conflict, without using precedence.

I think we should avoid this if possible: it seems quite likely to me
that there are applications using SET as the name of a type, table or
column (making SET a func_name_keyword would disallow its use as a type
name, AFAICS). So I'm inclined to favor #2.

-Neil

Neil Conway <neilc@samurai.com> writes:
> Did you actually test this?

No, I was just looking at the y.output file to see what would happen.

> neilc=# update t1 set set a = 500 where set.a > 1000;
> UPDATE 0
> (Using essentially the patch you posted.)

[ scratches head... ]  That shouldn't have worked.  I'll have to look
again.

> Well, if necessary we can just use an alternate production for the
> DELETE case, as there is no SET ambiguity to worry about.

Yeah, I thought of that too and rejected it as being too much trouble
for too small a case.  I'm really considerably more worried about the
question of whether attaching a precedence to SET might cause trouble
later.  But it's only a hypothetical problem at this point.

> So I'm inclined to favor #2.

OK, motion carries.  I'll check what's happening in the case above
and commit if there's not something wrong.

            regards, tom lane

Neil Conway <neilc@samurai.com> writes:
> Did you actually test this?

> neilc=# create table t1 (a int, b int);
> CREATE TABLE
> neilc=# update t1 set set a = 500 where set.a > 1000;
> UPDATE 0

I get

regression=# update t1 set set a = 500 where set.a > 1000;
ERROR:  syntax error at or near "a" at character 19
LINE 1: update t1 set set a = 500 where set.a > 1000;
                          ^

I think possibly you put the %nonassoc entry in the wrong place --- it
has to go someplace before the UMINUS entry to get the desired effect.
My fault for putting up a non-context diff.

            regards, tom lane