On Thu, Apr 20, 2017 at 12:05 AM, Vitaly Burovoy <vitaly.buro...@gmail.com> wrote: >> I am still not fond of this change. There is precedent all over the >> place for having separate commands for creating a structure, changing a >> structure, and removing a structure. I don't understand what the >> problem with that is.
I agree. That's not intrinsically a problem. > OK. Let's go through it again. > IDENTITY is a property of a column. There are no syntax to change any > property of any DB object via the "ADD" syntax. > Yes, a structure (a sequence) is created. But in fact it cannot be > independent from the column at all (I remind you that according to the > standard it should be unnamed sequence and there are really no way to > do something with it but via the column's DDL). I agree that ADD is a little odd here, but it doesn't seem terrible. But why do we need it? Instead of: ADD GENERATED { ALWAYS | BY DEFAULT } AS IDENTITY SET GENERATED { ALWAYS | BY DEFAULT } DROP IDENTITY [ IF EXISTS ] Why not just: SET GENERATED { ALWAYS | BY DEFAULT } AS IDENTITY DROP IDENTITY [ IF EXISTS ] Surely the ALTER TABLE command can tell whether the column is already GENERATED, so the first form could make it generated if it's not and adjust the ALWAYS/BY DEFAULT property if it is. > It is even hard to detect which sequence (since they have names) is > owned by the column: > > postgres=# CREATE TABLE xxx(i int generated always as identity, j serial); > CREATE TABLE > postgres=# \d xxx* > Table "public.xxx" > Column | Type | Collation | Nullable | Default > --------+---------+-----------+----------+-------------------------------- > i | integer | | not null | generated always as identity > j | integer | | not null | nextval('xxx_j_seq'::regclass) > > Sequence "public.xxx_i_seq" > Column | Type | Value > ------------+---------+------- > last_value | bigint | 1 > log_cnt | bigint | 0 > is_called | boolean | f > > Sequence "public.xxx_j_seq" > Column | Type | Value > ------------+---------+------- > last_value | bigint | 1 > log_cnt | bigint | 0 > is_called | boolean | f > Owned by: public.xxx.j > > I can only guess that "public.xxx_i_seq" is owned by "public.xxx.i", > nothing proves that. > Whereas for regular sequence there are two evidences ("Default" and "Owned > by"). This seems like a psql deficiency that should be fixed. > Also the created sequence cannot be deleted (only with the column) or > left after the column is deleted. This does not seem like a problem. In fact I'd say that's exactly the desirable behavior. > The "[ NOT ] EXISTS" is a common Postgres' syntax extension for > creating/updating objects in many places. That's why I think it should > be used instead of introducing the new "ADD" syntax which contradicts > the users' current experience. As noted above, I don't understand why we need either ADD or IF NOT EXISTS. Why can't SET just, eh, set the property to the desired state? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers