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

Lists: pgsql-patches
From: Atsushi Ogawa <atsushi(dot)ogawa(at)gmail(dot)com>
To: pgsql-patches(at)postgresql(dot)org
Subject: Allow an alias for the target table in UPDATE/DELETE
Date: 2005-12-01 14:23:11
Message-ID: 613787150512010623l16c528f3x@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-patches

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

Attachment Content-Type Size
allow_alias_update.patch application/octet-stream 8.8 KB

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Atsushi Ogawa <atsushi(dot)ogawa(at)gmail(dot)com>
Cc: pgsql-patches(at)postgresql(dot)org
Subject: Re: Allow an alias for the target table in UPDATE/DELETE
Date: 2005-12-01 16:05:12
Message-ID: 11388.1133453112@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-patches

Atsushi Ogawa <atsushi(dot)ogawa(at)gmail(dot)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


From: Atsushi Ogawa <atsushi(dot)ogawa(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: pgsql-patches(at)postgresql(dot)org
Subject: Re: Allow an alias for the target table in UPDATE/DELETE
Date: 2005-12-03 01:42:50
Message-ID: 613787150512021742p1d2a4eeby@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-patches

Thanks for comments. I modified the patch.

Tom Lane wrote:
> Atsushi Ogawa <atsushi(dot)ogawa(at)gmail(dot)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

Attachment Content-Type Size
allow_alias_update.patch application/octet-stream 6.6 KB

From: Neil Conway <neilc(at)samurai(dot)com>
To: Atsushi Ogawa <atsushi(dot)ogawa(at)gmail(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, pgsql-patches(at)postgresql(dot)org
Subject: Re: Allow an alias for the target table in UPDATE/DELETE
Date: 2006-01-22 05:29:37
Message-ID: 1137907777.8798.7.camel@localhost.localdomain
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-patches

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


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Neil Conway <neilc(at)samurai(dot)com>
Cc: Atsushi Ogawa <atsushi(dot)ogawa(at)gmail(dot)com>, pgsql-patches(at)postgresql(dot)org
Subject: Re: Allow an alias for the target table in UPDATE/DELETE
Date: 2006-01-22 05:48:06
Message-ID: 4602.1137908886@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-patches

Neil Conway <neilc(at)samurai(dot)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


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Neil Conway <neilc(at)samurai(dot)com>
Cc: Atsushi Ogawa <atsushi(dot)ogawa(at)gmail(dot)com>, pgsql-patches(at)postgresql(dot)org
Subject: Re: Allow an alias for the target table in UPDATE/DELETE
Date: 2006-01-22 05:55:18
Message-ID: 4664.1137909318@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-patches

Neil Conway <neilc(at)samurai(dot)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


From: Neil Conway <neilc(at)samurai(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Atsushi Ogawa <atsushi(dot)ogawa(at)gmail(dot)com>, pgsql-patches(at)postgresql(dot)org
Subject: Re: Allow an alias for the target table in UPDATE/DELETE
Date: 2006-01-22 06:21:59
Message-ID: 1137910919.8798.25.camel@localhost.localdomain
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-patches

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


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Neil Conway <neilc(at)samurai(dot)com>
Cc: Atsushi Ogawa <atsushi(dot)ogawa(at)gmail(dot)com>, pgsql-patches(at)postgresql(dot)org
Subject: Re: Allow an alias for the target table in UPDATE/DELETE
Date: 2006-01-22 06:28:31
Message-ID: 4957.1137911311@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-patches

Neil Conway <neilc(at)samurai(dot)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


From: Neil Conway <neilc(at)samurai(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Atsushi Ogawa <atsushi(dot)ogawa(at)gmail(dot)com>, pgsql-patches(at)postgresql(dot)org
Subject: Re: Allow an alias for the target table in UPDATE/DELETE
Date: 2006-01-22 07:05:14
Message-ID: 1137913514.8798.31.camel@localhost.localdomain
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-patches

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


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Neil Conway <neilc(at)samurai(dot)com>
Cc: Atsushi Ogawa <atsushi(dot)ogawa(at)gmail(dot)com>, pgsql-patches(at)postgresql(dot)org
Subject: Re: Allow an alias for the target table in UPDATE/DELETE
Date: 2006-01-22 07:23:46
Message-ID: 5264.1137914626@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-patches

Neil Conway <neilc(at)samurai(dot)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


From: Neil Conway <neilc(at)samurai(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Atsushi Ogawa <atsushi(dot)ogawa(at)gmail(dot)com>, pgsql-patches(at)postgresql(dot)org
Subject: Re: Allow an alias for the target table in UPDATE/DELETE
Date: 2006-01-22 07:26:05
Message-ID: 1137914765.8798.38.camel@localhost.localdomain
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-patches

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


From: Neil Conway <neilc(at)samurai(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Atsushi Ogawa <atsushi(dot)ogawa(at)gmail(dot)com>, pgsql-patches(at)postgresql(dot)org
Subject: Re: Allow an alias for the target table in UPDATE/DELETE
Date: 2006-01-22 07:43:31
Message-ID: 1137915811.8798.41.camel@localhost.localdomain
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-patches

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