Re: [PATCHES] Allow an alias for the target table in UPDATE/DELETE
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. signature.asc Description: Digital signature
Re: [PATCHES] Allow an alias for the target table in UPDATE/DELETE
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 ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] Allow an alias for the target table in UPDATE/DELETE
Andrew Dunstan [EMAIL PROTECTED] 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
---(end of broadcast)---
TIP 6: explain analyze is your friend
Re: [PATCHES] Allow an alias for the target table in UPDATE/DELETE
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 ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] Allow an alias for the target table in UPDATE/DELETE
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 ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] Allow an alias for the target table in UPDATE/DELETE
Neil Conway [EMAIL PROTECTED] 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 ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] Allow an alias for the target table in UPDATE/DELETE
Neil Conway [EMAIL PROTECTED] 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 ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] Allow an alias for the target table in UPDATE/DELETE
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 ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] Allow an alias for the target table in UPDATE/DELETE
Neil Conway [EMAIL PROTECTED] 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 ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] Allow an alias for the target table in UPDATE/DELETE
Neil Conway [EMAIL PROTECTED] 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
---(end of broadcast)---
TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] Allow an alias for the target table in UPDATE/DELETE
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
---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] Allow an alias for the target table in UPDATE/DELETE
Neil Conway [EMAIL PROTECTED] 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 ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] Allow an alias for the target table in UPDATE/DELETE
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 ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] Allow an alias for the target table in UPDATE/DELETE
Neil Conway [EMAIL PROTECTED] 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 ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] Allow an alias for the target table in UPDATE/DELETE
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 ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] Allow an alias for the target table in UPDATE/DELETE
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 ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] Allow an alias for the target table in UPDATE/DELETE
Thanks for comments. I modified the patch.Tom Lane wrote: Atsushi Ogawa [EMAIL PROTECTED] 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 fromColId to IDENT. This doesn't change reserved words though candidates of that alias decreases.--- Atsushi Ogawa allow_alias_update.patch Description: Binary data ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
[PATCHES] 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 allow_alias_update.patch Description: Binary data ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
