Skip site navigation (1) Skip section navigation (2)

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 10:01:19
Message-ID: 46249AEF.6040404@cybertec.at (view raw or flat)
Thread:
Lists: pgsql-hackerspgsql-patches
Zoltan Boszormenyi írta:
> 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
>

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: psql-serial-41.diff.gz
Description: application/x-tar (29.4 KB)

In response to

pgsql-hackers by date

Next:From: Heikki LinnakangasDate: 2007-04-17 10:25:29
Subject: Re: Hacking on PostgreSQL via GIT
Previous:From: Russell SmithDate: 2007-04-17 08:02:51
Subject: Re: Grantor name gets lost when grantor role dropped

pgsql-patches by date

Next:From: Heikki LinnakangasDate: 2007-04-17 11:36:26
Subject: Re: Heap page diagnostic/test functions (v2)
Previous:From: ITAGAKI TakahiroDate: 2007-04-17 07:27:49
Subject: Re: Dead Space Map version 3 (simplified)

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group