Re: [HACKERS] Re: IDENTITY/GENERATED v36 Re: Final version of IDENTITY/GENERATED patch - Mailing list pgsql-patches

From Zoltan Boszormenyi
Subject Re: [HACKERS] Re: IDENTITY/GENERATED v36 Re: Final version of IDENTITY/GENERATED patch
Date
Msg-id 46249AEF.6040404@cybertec.at
Whole thread Raw
In response to Re: [HACKERS] Re: IDENTITY/GENERATED v36 Re: Final version of IDENTITY/GENERATED patch  (Zoltan Boszormenyi <zb@cybertec.at>)
List pgsql-patches
Zoltan Boszormenyi írta:
> Tom Lane írta:
>> Andrew Dunstan <andrew@dunslane.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
>

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/


Attachment

pgsql-patches by date:

Previous
From: ITAGAKI Takahiro
Date:
Subject: Re: Dead Space Map version 3 (simplified)
Next
From: Heikki Linnakangas
Date:
Subject: Re: Heap page diagnostic/test functions (v2)