On Thu, Feb 01, 2018 at 10:24:48AM -0500, Peter Eisentraut wrote:
> On 12/30/17 18:26, Michael Paquier wrote:
>> A RangeVar is used for the proposed patch 0002. Still does it matter? In
>> the scenario proposed here, which is to fix ALTER TABLE ADD COLUMN
>> GENERATED, then the sequence gets created and visible only once the
>> transaction adding the column has been created.
>>
>> char identity; /* attidentity setting */
>> + RangeVar *identitySequence;
>> CollateClause *collClause; /* untransformed COLLATE spec, if any */
>> It mat be better to tell that this is used to create a sequence in the
>> same transaction as the one doing the parsing.
>
> I would like to get this into next week's minor release as a bug fix.
> Other than tweaking some of the comments, is there any more feedback on
> this?
Wrapping again my mind on this one... On top of the comment for
identitySequence, I think that it is important to mention that the
sequence name and a RangeVar are basically safe as they get created
hence they are not visible to other sessions yet. 0001 and 0002 should
be merged.
By the way, on HEAD with two sessions it is easy to bump into sequence
name conflicts with identity columns:
* Session 1:
begin;
create table itest13 (a int);
* Session 2:
create sequence itest13_b_seq;
* Session 1:
alter table itest13 add columns b int generated by default as identity; --blocks
* Session 2:
commit;
And then session 1 reports that:
ERROR: 23505: duplicate key value violates unique constraint "pg_type_typname_nsp_index"
DETAIL: Key (typname, typnamespace)=(itest13_b_seq, 2200) already exists.
SCHEMA NAME: pg_catalog
TABLE NAME: pg_type
CONSTRAINT NAME: pg_type_typname_nsp_index
LOCATION: _bt_check_unique, nbtinsert.c:434
We surely want to be smarter with the choice of the sequence name for
identity columns. Index names for primary keys are if I recall
correctly indexcmds.c and index.c.
--
Michael