Re: [HACKERS] Re: IDENTITY/GENERATED v36 Re: Final version of IDENTITY/GENERATED patch

From: Zoltan Boszormenyi <zb(at)cybertec(dot)at>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Andrew Dunstan <andrew(at)dunslane(dot)net>, "Florian G(dot) Pflug" <fgp(at)phlo(dot)org>, Bruce Momjian <bruce(at)momjian(dot)us>, List pgsql-patches <pgsql-patches(at)postgresql(dot)org>
Subject: Re: [HACKERS] Re: IDENTITY/GENERATED v36 Re: Final version of IDENTITY/GENERATED patch
Date: 2007-04-17 07:15:34
Message-ID: 46247416.5020200@cybertec.at
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers pgsql-patches

Tom Lane írta:
> Andrew Dunstan <andrew(at)dunslane(dot)net> 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/

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message ITAGAKI Takahiro 2007-04-17 07:27:49 Re: Dead Space Map version 3 (simplified)
Previous Message Islam Hegazy 2007-04-17 07:08:37 modifying the table function

Browse pgsql-patches by date

  From Date Subject
Next Message ITAGAKI Takahiro 2007-04-17 07:27:49 Re: Dead Space Map version 3 (simplified)
Previous Message Nikolay Samokhvalov 2007-04-17 05:26:12 Re: xpath_array with namespaces support