Re: identity columns

From: Vitaly Burovoy <vitaly(dot)burovoy(at)gmail(dot)com>
To: Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>
Cc: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: identity columns
Date: 2017-03-21 20:11:07
Message-ID: CAKOSWNnMp0sKZzcX0zSKJOTSb1Fas02+xTW=kvGMu2NhRiOAbg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I haven't seen a patch (I'll do it later), just few notes:

On 3/21/17, Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com> wrote:
>>>> 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.

I've just checked and still get an error about a type, not about
absence of a column:
test=# CREATE TABLE itest3 (a int generated by default as identity, b text);
CREATE TABLE
test=# ALTER TABLE itest3 ALTER COLUMN o ADD GENERATED ALWAYS AS IDENTITY;
ERROR: identity column type must be smallint, integer, or bigint

>> 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.

I'd agree with you if we are talking about database's objects like
tables, functions, columns etc.

> 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.

My argument is consistency.
Since IDENTITY is a property of a column (similar to DEFAULT, NOT
NULL, attributes, STORAGE, etc.), it follows a different rule: it is
either set or not set. If it did not set before, the "SET" DDL "adds"
it, if that property already present, the DDL replaces it.
There is no "ADD" clause in DDLs like "...ALTER table ALTER column..."
(only "SET", "RESET" and "DROP")[2].
Your patch introduces the single DDL version with "...ALTER column
ADD..." for a property.

>> 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.

Are you about the thread[1]? If so, I'd say the current behavior is not good.
I sent an example with users' bad experience who will know nothing
about sequences (because they'll deal with identity columns).
Would it be better to change bounds of a sequence if they match the
bounds of an old type (to the bounds of a new type)?

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

[1] https://www.postgresql.org/message-id/flat/898deb94-5265-37cf-f199-4f79ef864536(at)2ndquadrant(dot)com
[2] https://www.postgresql.org/docs/current/static/sql-altertable.html
--
Best regards,
Vitaly Burovoy

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Dmitry Dolgov 2017-03-21 20:42:06 Re: [PATCH] Generic type subscripting
Previous Message Alvaro Herrera 2017-03-21 20:10:16 Re: brin autosummarization -- autovacuum "work items"