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

Hi,

Zoltan Boszormenyi írta:
> Zoltan Boszormenyi írta:
>> Tom Lane írta:
>>> Zoltan Boszormenyi <zb(at)cybertec(dot)at> writes:
>>>
>>>> So, I should allow DROP DEFAULT, implement
>>>> SET DEFAULT GENERATED ALWAYS AS
>>>> and modify the catalog so the GENERATED property
>>>> is part of pg_attrdef.
>>>>
>>>
>>> Sounds good.
>>>
>>
>> Finally here it is.
>>
>>>> What about IDENTITY?
>>>> Should it also be part of pg_attrdef? There are two ways
>>>> to implement it: have or don't have a notion of it.
>>>> The latter would treat GENERATED BY DEFAULT AS IDENTITY
>>>> the same as SERIAL.
>>>>
>>>
>>> Is there any good reason to distinguish the two?
>>>
>>
>> Actually, I needed to have a flag for IDENTITY
>> but not for the reason above. I need it to distinguish
>> between GENERATED ALWAYS AS IDENTITY
>> and GENERATED ALWAYS AS ( expr ).
>>
>> Changes:
>> - Rewritten the GENERATED/IDENTITY flags to be part of the default
>> pg_attrdef
>> This made the patch MUCH smaller.
>> - SERIALs are now the same as INTEGER GENERATED BY DEFAULT AS IDENTITY
>> - Allow DROP DEFAULT on GENERATED/IDENTITY columns
>> - Implemented SET GENERATED ALWAYS AS
>> - Modified syntax of SET GENERATED {ALWAYS | BY DEFAULT} AS IDENTITY
>> so it reads as SET IDENTITY GENERATED {ALWAYS | BY DEFAULT}
>> so compiling gram.y/gram.c doesn't give me errors.
>> This DDL statement isn't part of SQL:2003 so it might be accepted
>> as a PostgreSQL extension.
>> - Modified behaviour of SET IDENTITY to also restore the DEFAULT
>> expression. Someone might have done did a DROP DEFAULT before
>> but kept the OWNED sequence.
>> - Fixed behaviour of GENERATED columns regarding
>> INSERT ... OVERRIDING SYSTEM VALUE and
>> only those GENERATED columns get UPDATEd that
>> are either explicitly modified with SET column = DEFAULT
>> or one of their referenced columns are modified.
>> - Testcase and documentation is modified to reflect the above.
>
> - Also allowed UPDATE on IDENTITY columns.
>
>>
>> Please, review.
>

I just realized that by treating SERIAL the same as
IDENTITY GENERATED BY DEFAULT, I incidentally
broke the possibility of multiple SERIALs in the same table.
I rewrote the patch so instead of two BOOL flags,
I now have only one CHAR flag:
- ' ' says it's a simple DEFAULT expression
- 'i' says it's GENERATED ALWAYS AS IDENTITY
- 'g' says it's GENERATED ALWAYS AS ( expr )

Apart from making the patch a bit smaller again, checking only
for 'i' still allows multiple SERIALs in the same table but lets
disallowing multiple GENERATED ALWAYS AS IDENTITY.
Thinking a bit about it, is it desired to disallow multiple GENERATED
ALWAYS AS IDENTITY fields? It's just a twisted SERIAL anyway.
And it was said many times that it's not an advantage to blindly
follow the standard.

Also, DROP IDENTITY is equivalent with SET DEFAULT
nextval('owned_sequence') iff the field has an OWNED
sequence and it was GENERATED ALWAYS AS IDENTITY before.
Considering that SERIAL is a macro, and SET/DROP DEFAULT is allowed
on IDENTITY/GENERATED columns per Tom's request,
should I keep this statement?

Also, the current grammar is made to give a syntax error
if you say "colname type GENERATED BY DEFAULT AS ( expr )".
But it makes the grammar unbalanced, and gives me:

bison -y -d gram.y
conflicts: 2 shift/reduce

Is there a good solution to this?

I post the new patch after someone answers those questions for me.

--
----------------------------------
Zoltán Böszörményi
Cybertec Geschwinde & Schönig GmbH
http://www.postgresql.at/