Re: [HACKERS] Re: IDENTITY/GENERATED v36 Re: [PATCHES] Final version of IDENTITY/GENERATED patch
Tom Lane írta:
Andrew Dunstan [EMAIL PROTECTED] writes:
Zoltan Boszormenyi wrote:
Thanks. This idea solved one of the two shift/reduce conflicts.
But the other one can only be solved if I put GENERATED
into the reserved_keyword set. But the standard spec says
it's unreserved. Now what should I do with it?
Yeah, I had a brief look. It's a bit ugly - the remaining conflict is in
the b_expr rules. We do have the filtered_base_yylex() gadget - not
sure if that can disambiguate for us.
The problem comes from cases like
colname coltype DEFAULT 5! GENERATED ...
Since b_expr allows postfix operators, it takes one more token of
lookahead than we have to tell if the default expression is 5!
or 5!GENERATED
There are basically two ways to fix this:
1. Collapse GENERATED ALWAYS and GENERATED BY into single tokens
using filtered_base_yylex.
2. Stop allowing postfix operators in b_expr.
I find #1 a bit icky --- not only does every case added to
filtered_base_yylex slow down parsing a little more, but combined
tokens create rough spots in the parser's behavior. As an example,
both NULLS and FIRST are allegedly unreserved words, so this should
work:
regression=# create table nulls (x int);
CREATE TABLE
regression=# select first.* from nulls first;
ERROR: syntax error at or near first
LINE 1: select first.* from nulls first;
^
regression=#
#2 actually seems like a viable alternative: postfix operators aren't
really in common use, and doing this would not only fix GENERATED but
let us de-reserve a few keywords that are currently reserved. In a
non-exhaustive check I found that COLLATE, DEFERRABLE, and INITIALLY
could become unreserved_keyword if we take out this production:
*** 7429,7436
{ $$ = (Node *) makeA_Expr(AEXPR_OP, $2, $1, $3, @2); }
| qual_Op b_expr%prec Op
{ $$ = (Node *) makeA_Expr(AEXPR_OP, $1, NULL, $2, @1); }
- | b_expr qual_Op%prec POSTFIXOP
- { $$ = (Node *) makeA_Expr(AEXPR_OP, $2, $1, NULL, @2); }
| b_expr IS DISTINCT FROM b_expr%prec IS
{
$$ = (Node *) makeSimpleA_Expr(AEXPR_DISTINCT, =, $1, $5,
@2);
--- 7550,7555
(Hmm, actually I'm wondering why COLLATE is a keyword at all right
now... but the other two trace directly to the what-comes-after-DEFAULT
issue.)
regards, tom lane
I looked at it a bit and tried to handle DEFAULT differently from
other column constaints. Basically I did what the standard says:
column definition ::=
column name [ data type or domain name ]
[ default clause | identity column specification | generation
clause ]
[ column constraint definition... ]
[ collate clause ]
So DEFAULT and GENERATED {BY DEFAULT | ALWAYS } AS
{ IDENTITY| GENERATED} is made mutually exclusive in the grammar
and these must come before any other column constraints.
This have one possible problem and one fix. The problem is that
the DEFAULT clause cannot be mixed with other constraints any more,
hence breaking some regression tests and lazy deployment.
BTW the PostgreSQL documentation already says that DEFAULT
must come after the type specification.
The other is that specifying DEFAULT as a named constraint isn't allowed
any more. See the confusion below. PostgreSQL happily accepts
but forgets about the name I gave to the DEFAULT clause.
db=# create table aaa (id integer not null constraint my_default default
5, t text);
CREATE TABLE
db=# \d aaa
Tábla public.aaa
Oszlop | Típus | Módosító
+-+
id | integer | not null default 5
t | text|
db=# alter table aaa drop constraint my_default ;
ERROR: constraint my_default does not exist
--
--
Zoltán Böszörményi
Cybertec Geschwinde Schönig GmbH
http://www.postgresql.at/
---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] Re: IDENTITY/GENERATED v36 Re: [PATCHES] Final version of IDENTITY/GENERATED patch
Zoltan Boszormenyi írta:
Tom Lane írta:
Andrew Dunstan [EMAIL PROTECTED] writes:
Zoltan Boszormenyi wrote:
Thanks. This idea solved one of the two shift/reduce conflicts.
But the other one can only be solved if I put GENERATED
into the reserved_keyword set. But the standard spec says
it's unreserved. Now what should I do with it?
Yeah, I had a brief look. It's a bit ugly - the remaining conflict
is in the b_expr rules. We do have the filtered_base_yylex() gadget
- not sure if that can disambiguate for us.
The problem comes from cases like
colname coltype DEFAULT 5! GENERATED ...
Since b_expr allows postfix operators, it takes one more token of
lookahead than we have to tell if the default expression is 5!
or 5!GENERATED
There are basically two ways to fix this:
1. Collapse GENERATED ALWAYS and GENERATED BY into single tokens
using filtered_base_yylex.
2. Stop allowing postfix operators in b_expr.
I find #1 a bit icky --- not only does every case added to
filtered_base_yylex slow down parsing a little more, but combined
tokens create rough spots in the parser's behavior. As an example,
both NULLS and FIRST are allegedly unreserved words, so this should
work:
regression=# create table nulls (x int);
CREATE TABLE
regression=# select first.* from nulls first;
ERROR: syntax error at or near first
LINE 1: select first.* from nulls first;
^
regression=#
#2 actually seems like a viable alternative: postfix operators aren't
really in common use, and doing this would not only fix GENERATED but
let us de-reserve a few keywords that are currently reserved. In a
non-exhaustive check I found that COLLATE, DEFERRABLE, and INITIALLY
could become unreserved_keyword if we take out this production:
*** 7429,7436
{ $$ = (Node *) makeA_Expr(AEXPR_OP, $2, $1, $3,
@2); }
| qual_Op b_expr%prec Op
{ $$ = (Node *) makeA_Expr(AEXPR_OP, $1, NULL, $2,
@1); }
- | b_expr qual_Op%prec POSTFIXOP
- { $$ = (Node *) makeA_Expr(AEXPR_OP, $2, $1, NULL,
@2); }
| b_expr IS DISTINCT FROM b_expr%prec IS
{
$$ = (Node *) makeSimpleA_Expr(AEXPR_DISTINCT,
=, $1, $5, @2);
--- 7550,7555
(Hmm, actually I'm wondering why COLLATE is a keyword at all right
now... but the other two trace directly to the what-comes-after-DEFAULT
issue.)
regards, tom lane
I looked at it a bit and tried to handle DEFAULT differently from
other column constaints. Basically I did what the standard says:
column definition ::=
column name [ data type or domain name ]
[ default clause | identity column specification |
generation clause ]
[ column constraint definition... ]
[ collate clause ]
So DEFAULT and GENERATED {BY DEFAULT | ALWAYS } AS
{ IDENTITY| GENERATED} is made mutually exclusive in the grammar
and these must come before any other column constraints.
This have one possible problem and one fix. The problem is that
the DEFAULT clause cannot be mixed with other constraints any more,
hence breaking some regression tests and lazy deployment.
BTW the PostgreSQL documentation already says that DEFAULT
must come after the type specification.
The other is that specifying DEFAULT as a named constraint isn't allowed
any more. See the confusion below. PostgreSQL happily accepts
but forgets about the name I gave to the DEFAULT clause.
db=# create table aaa (id integer not null constraint my_default
default 5, t text);
CREATE TABLE
db=# \d aaa
Tábla public.aaa
Oszlop | Típus | Módosító
+-+
id | integer | not null default 5
t | text|
db=# alter table aaa drop constraint my_default ;
ERROR: constraint my_default does not exist
Here's what it looks like now. Another side effect is that
the check for conflicting DEFAULT clauses can now be
deleted from analyze.c.
--
--
Zoltán Böszörményi
Cybertec Geschwinde Schönig GmbH
http://www.postgresql.at/
psql-serial-41.diff.gz
Description: Unix tar archive
---(end of broadcast)---
TIP 5: don't forget to increase your free space map settings
Re: [HACKERS] Re: IDENTITY/GENERATED v36 Re: [PATCHES] Final version of IDENTITY/GENERATED patch
Andrew Dunstan wrote:
Tom Lane wrote:
The problem comes from cases like
colname coltype DEFAULT 5! GENERATED ...
Since b_expr allows postfix operators, it takes one more token of
lookahead than we have to tell if the default expression is 5!
or 5!GENERATED
There are basically two ways to fix this:
1. Collapse GENERATED ALWAYS and GENERATED BY into single tokens
using filtered_base_yylex.
2. Stop allowing postfix operators in b_expr.
I can't think of any good reason why we need postfix operators at all.
Plenty of languages do quite happily without them, and where they make
some sense (e.g. in C) they do so because of their side effect, which
doesn't seem relevant here.
So I vote for #2 unless it will break too much legacy stuff. You should
always be able to replace operand postop with a function call anyway -
it's arguably just syntactic sugar.
Is it not enough to enclose the expression in parentheses? ISTM that
this rule covers this:
c_expr:
| '(' a_expr ')' opt_indirection
BTW I just noticed this bug in the comment above a_expr:
* Note that '(' a_expr ')' is a b_expr, so an unrestricted expression can
* always be used by surrounding it with parens.
It is wrong because it's not a b_expr, but a c_expr.
--
Alvaro Herrerahttp://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] Re: IDENTITY/GENERATED v36 Re: [PATCHES] Final version of IDENTITY/GENERATED patch
Alvaro Herrera [EMAIL PROTECTED] writes:
BTW I just noticed this bug in the comment above a_expr:
* Note that '(' a_expr ')' is a b_expr, so an unrestricted expression can
* always be used by surrounding it with parens.
It is wrong because it's not a b_expr, but a c_expr.
Well, it's right in context because the comment is discussing the
difference between a_expr and b_expr. Also, a c_expr is-a b_expr.
If anyone seriously wants to propose removing postfix ops from b_expr,
we'd better take it up on someplace more widely read than -patches.
regards, tom lane
---(end of broadcast)---
TIP 4: Have you searched our list archives?
http://archives.postgresql.org
Re: [HACKERS] Re: IDENTITY/GENERATED v36 Re: [PATCHES] Final version of IDENTITY/GENERATED patch
Tom Lane wrote: The problem comes from cases like colname coltype DEFAULT 5! GENERATED ... Since b_expr allows postfix operators, it takes one more token of lookahead than we have to tell if the default expression is 5! or 5!GENERATED There are basically two ways to fix this: 1. Collapse GENERATED ALWAYS and GENERATED BY into single tokens using filtered_base_yylex. 2. Stop allowing postfix operators in b_expr. I can't think of any good reason why we need postfix operators at all. Plenty of languages do quite happily without them, and where they make some sense (e.g. in C) they do so because of their side effect, which doesn't seem relevant here. So I vote for #2 unless it will break too much legacy stuff. You should always be able to replace operand postop with a function call anyway - it's arguably just syntactic sugar. 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: [HACKERS] Re: IDENTITY/GENERATED v36 Re: [PATCHES] Final version of IDENTITY/GENERATED patch
Zoltan Boszormenyi írta: Andrew Dunstan írta: Florian G. Pflug wrote: bison -y -d gram.y conflicts: 2 shift/reduce I'ts been quite a time since I last used bison, but as far as I remember, you can tell it to write a rather details log about it's analysis of the grammar. That log should include more detailed information about those conflicts - maybe that helps to figure out their exact cause, and to find a workaround. You can almost always get rid of shift/reduce conflicts by unwinding some of the productions - resist the temptation to factor the grammar. The effect of this is to eliminate places where the parser has to decide between shifting and reducing. (This is why, for example, almost all the drop foo if exists variants require separate productions rather than using opt_if_exists.) cheers andrew Thanks. This idea solved one of the two shift/reduce conflicts. But the other one can only be solved if I put GENERATED into the reserved_keyword set. But the standard spec says it's unreserved. Now what should I do with it? What should I do? Send it. :-) Someone more familiar with bison can hopefully fix it... Please, review. Best regards, Zoltán -- -- Zoltán Böszörményi Cybertec Geschwinde Schönig GmbH http://www.postgresql.at/ psql-serial-40.diff.gz Description: Unix tar archive ---(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
