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
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers pgsql-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 Content-Type Size
psql-serial-41.diff.gz application/x-tar 29.4 KB

In response to

Browse pgsql-hackers by date

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

Browse pgsql-patches by date

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