Re: [HACKERS] identity columns - Mailing list pgsql-hackers

On 3/20/17 05:43, Vitaly Burovoy wrote:
>>> 1. The fact COPY ignores GENERATED ALWAYS constraint (treats as
>>> GENERATED BY DEFAULT) should be mentioned as well as rules.
> I think it would be better if that behavior is placed on both CREATE
> TABLE and COPY pages.

done

>>> 3. Strange error (not about absence of a column; but see pp.5 and 8):
>>> test=# ALTER TABLE idnt ALTER COLUMN o ADD GENERATED ALWAYS AS IDENTITY;
>>> ERROR:  identity column type must be smallint, integer, or bigint
>>
>> What's wrong with that?
> 
> The column mentioned there does not exist. Therefore the error should
> be about it, not about a type of absent column:

This was already fixed in the previous version.

> Since we extend the spec by "ADD GENERATED...", "SEQUENCE NAME" and
> allow more than one identity column, why can't we extend it by
> allowing "SET GENERATED" for non-identity columns and drop "ADD
> GENERATED..."?

SQL commands generally don't work that way.  Either they create or they
alter.  There are "OR REPLACE" options when you do both.  So I think it
is better to keep these two things separate.  Also, while you argue that
we *could* do it the way you propose, I don't really see an argument why
it would be better.

>>> 6. The docs mention a syntax:
>>> ALTER [ COLUMN ] <replaceable
>>> class="PARAMETER">column_name</replaceable> { SET GENERATED { ALWAYS |
>>> BY DEFAULT } | SET <replaceable>sequence_option</replaceable> | RESET
>>> } [...]
>>>
>>> but the command fails:
>>> test=# ALTER TABLE idnt ALTER COLUMN i RESET;
>>> ERROR:  syntax error at or near ";"
>>> LINE 1: ALTER TABLE idnt ALTER COLUMN i RESET;
>>
>> This works for me.  Check again please.
> 
> I'm sorry, it still does not work for me (whether you mix it up with "RESTART"):
> test=# ALTER TABLE idnt ALTER COLUMN i RESET;
> ERROR:  syntax error at or near ";"
> LINE 1: ALTER TABLE idnt ALTER COLUMN i RESET;

Oh I see, the documentation was wrong.  The correct syntax is RESTART
rather than RESET.  Fixed the documentation.

>>> 11. The last CF added partitioned tables which are similar to
>>> inherited, but slightly different.
>>
>> fixed
> 
> Something is left to be fixed:
> test=# CREATE TABLE measurement (
> test(#     i               int GENERATED BY DEFAULT AS IDENTITY,
> test(#     logdate         date not null,
> test(#     peaktemp        int,
> test(#     unitsales       int
> test(# ) PARTITION BY RANGE (logdate);
> CREATE TABLE
> test=# CREATE TABLE measurement_y2016m07
> test-#     PARTITION OF measurement (
> test(#     unitsales DEFAULT 0
> test(# ) FOR VALUES FROM ('2016-07-01') TO ('2016-08-01');
> CREATE TABLE
> test=# ALTER TABLE MEASUREMENT ALTER i RESTART 2;
> ERROR:  column "i" of relation "measurement_y2016m07" is not an identity column

fixed

> The patch should be rebased to the current master. It has easy
> conflicts in describe.c in the commit
> 395bfaae8e786eb17fc9dd79e4636f97c9d9b820.

done

> Please, don't include the file catversion.h in it because it is
> changed often and leads to error when applying.

OK

> 16. changing a type does not change an underlying sequence type (its limits):

It does change the type, but changing the type doesn't change the
limits.  That is a property of how ALTER SEQUENCE works, which was
separately discussed.

> 17. how to restart a sequence?
> test=# ALTER TABLE itest3 ALTER a SET RESTART 2;
> ERROR:  sequence option "restart" not supported here
> LINE 1: ALTER TABLE itest3 ALTER a SET RESTART 2;
> 
> Khm. The "RESTART" is one of official "sequence_options" (comparable
> to the "SEQUENCE NAME"), why it is not allowed?
> But there is another DDL which works OK, but not reflected in the docs:
> test=# ALTER TABLE itest3 ALTER a RESTART 2;
> ALTER TABLE

Yes, that is now fixed.  See #6 above.

> 18. If there is no sequence owned by a column, all DDLs for a sequence
> behind an identity column do not raise errors but do nothing:
> test=# CREATE TABLE itest3 (a int generated by default as identity
> (start with 7), b text);
> CREATE TABLE
> test=# ALTER SEQUENCE itest3_a_seq OWNED BY NONE;
> ALTER SEQUENCE

I have changed it to prohibit changing OWNED BY of an identity sequence
directly, so this can't happen anymore.

> 19. If anyone occasionally drops OWNED BY for the linked sequence
> there is no ways to restore it "as was":

fixed, see above

> 20. sequence does not follow the table owned by:

fixed

> 21. There are many places where error codes look strange:
> test=# ALTER TABLE itest3 ALTER b SET GENERATED BY DEFAULT;  --
> whether it is "internal_error" or user's error?
> ERROR:  XX000: column "b" of relation "itest3" is not an identity column

I added error codes to the places that were missing them.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers