Re: IDENTITY/GENERATED columns - Mailing list pgsql-patches

Hi,

here's the next, changes are at the end.

Zoltan Boszormenyi írta:
> Hi,
>
> my last patch didn't make it to the -hackers list,
> here's a newer one. Let me list what this patch does now:
>
> - CREATE TABLE/ALTER TABLE ADD syntax support for
>     GENERATED { ALWAYS | BY DEFAULT } AS
>      { ( expr ) | IDENTITY ( seq_opts ) }
> - catalog indicators of the above properties
> - INSERT|COPY syntax support for OVERRIDING { SYSTEM | USER } VALUE
> - INSERT|COPY fails for non-owner when using OVERRIDING SYSTEM VALUE
>     on tables with GENERATED ALWAYS columns
> - UPDATE fails when using other than DEFAULT literal for
>     GENERATED ALWAYS columns
> - GRANT {INSERT|UPDATE|ALL} ON table automatically
>     gives UPDATE permission for the supporting sequence
>     (missing: ALTER TABLE ADD should give permission
>      for the new sequence)
> - ALTER TABLE tab ALTER col { SET seq_opts | RESTART [WITH] N }
>     syntax support to alter the supporting sequence
> - ALTER TABLE tab RENAME also renames the supporting sequence
>     on both table and column renaming
> - ALTER TABLE SET/DROP default is disallowed
> - pg_dump support for exporting the above properties including
>     sequence parameters. Data dump uses OVERRIDING SYSTEM VALUE
>     when the table has GENERATED ALWAYS columns
> - test cases for some operations
> - documented
>
> With this version, I mostly covered these TODO entries:
>
>
>     * Add support for SQL-standard GENERATED/IDENTITY columns (almost
>       done :-) )
>     * %Disallow changing DEFAULT expression of a SERIAL column? (done)
>     * %Disallow ALTER SEQUENCE changes for SERIAL sequences because
>       pg_dump does not dump the changes
>       (pg_dump dumps the sequence options)
>     * Add DEFAULT .. AS OWNER so permission checks are done as the table
>       owner
>       This would be useful for SERIAL nextval() calls and CHECK
> constraints.
>       (GRANT TABLE grants UPDATE on the supporting sequence,
>        ALTER TABLE ADD COLUMN will be implemented)
>     * %Have ALTER TABLE RENAME rename SERIAL sequence names (done)
>     * Allow SERIAL sequences to inherit permissions from the base table?
>       (not needed because INSERT is a fastpath, permissions added
>        with GRANT TABLE, and [will be] extended on ALTER TABLE ADD
> COLUMN)
>
> I am considering using ALTER TABLE tab ALTER col TYPE
> to add (and drop) GENERATED ALWAYS and GENERATED AS IDENTITY
> properties to existing columns. As it stands now, ALTER TYPE
> doesn't change these column attributes.
>
> Please, review.
>
> Best regards,
> Zoltán Böszörményi


Changes from previous version:

- ALTER TABLE ADD col type GENERATED AS IDENTITY
  adds permission over the newly created sequence
  to those who already have INSERT or UPDATE on the table.
- ALTER TABLE SET/DROP DEFAULT also disallowed on
  GENERATED ALWAYS columns
- some codepaths were consolidated to avoid source bloat

A remaining larger problem is that
GENERATED ALWAYS AS (expr) should allow column
references from the same table, etc. just like CHECK.
For the sake of generality, DEFAULT must have
the same features, too. E.g. evaluating DEFAULT
value should be done as a second phase, after
evaluating other columns with given constants or
from DEFAULT that doesn't depend on other columns.
Circular dependencies must be avoided, etc.

I talked about implementin setting and dropping
GENERATED / IDENTITY properties. I cooked up this syntax:

ALTER TABLE tab ALTER col SET GENERATED ALWAYS AS ( expr )
    Behaves the same as "SET DEFAULT expr" but sets attforceddef.
ALTER TABLE tab ALTER col SET GENERATED ALWAYS AS IDENTITY [( seq_opts )]
    It should create supporting sequence iff the column
    isn't already an IDENTITY column.
ALTER TABLE tab ALTER col DROP IDENTITY
    It should be spelled loud instead of DROP DEFAULT.

That leaves ALTER TABLE ALTER TYPE alone.

Then a short check showed that changing such attributes
on a column isn't defined by SQL2003. The nice thing about
not implementing the above is that it avoids the debate
whether SET GENERATED ALWAYS AS [IDENTITY]
should also rewrite the records.

My answer in the debate would be that
GENERATED ALWAYS AS (expr)
should naturally do that but it isn't clear at all for
GENERATED ALWAYS AS IDENTITY

Closing words: I think it is now ready for acceptance,
it does all what I wanted it to do and it conforms to
SQL2003. Maybe I introduced some memory leaks
but I tried to avoid it by carefully inspecting what other
functions do. And some documentation details can
also be improved. But I think the code is ready.


Please, review.

Thanks and best regards,
Zoltán Böszörményi