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@cybertec.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/