From 6743c5ae64e3c957ef8bd4f8a0daa2ba9959b0b3 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut Date: Fri, 9 Feb 2024 07:57:31 +0100 Subject: [PATCH] Fix propagation of persistence to sequences in ALTER TABLE / ADD COLUMN Fix for 344d62fb9a9: That commit introduced unlogged sequences and made it so that identity/serial sequences automatically get the persistence level of their owning table. But this works only for CREATE TABLE and not for ALTER TABLE / ADD COLUMN. The latter would always create the sequence as logged (default), independent of the persistence setting of the table. This is fixed here. Note: It is allowed to change the persistence of identity sequences directly using ALTER SEQUENCE. So mistakes in existing databases can be fixed manually. Reviewed-by: Ashutosh Bapat Discussion: https://www.postgresql.org/message-id/flat/c4b6e2ed-bcdf-4ea7-965f-e49761094827%40eisentraut.org --- src/backend/parser/parse_utilcmd.c | 11 +++- src/test/regress/expected/identity.out | 72 ++++++++++++++++++++++++++ src/test/regress/sql/identity.sql | 18 +++++++ 3 files changed, 100 insertions(+), 1 deletion(-) diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c index 56ac4f516ea..c7efd8d8cee 100644 --- a/src/backend/parser/parse_utilcmd.c +++ b/src/backend/parser/parse_utilcmd.c @@ -459,7 +459,16 @@ generateSerialExtraStmts(CreateStmtContext *cxt, ColumnDef *column, seqstmt = makeNode(CreateSeqStmt); seqstmt->for_identity = for_identity; seqstmt->sequence = makeRangeVar(snamespace, sname, -1); - seqstmt->sequence->relpersistence = cxt->relation->relpersistence; + + /* + * Copy the persistence of the table. For CREATE TABLE, we get the + * persistence from cxt->relation, which comes from the CreateStmt in + * progress. For ALTER TABLE, the parser won't set + * cxt->relation->relpersistence, but we have cxt->rel as the existing + * table, so we copy the persistence from there. + */ + seqstmt->sequence->relpersistence = cxt->rel ? cxt->rel->rd_rel->relpersistence : cxt->relation->relpersistence; + seqstmt->options = seqoptions; /* diff --git a/src/test/regress/expected/identity.out b/src/test/regress/expected/identity.out index 08f95674ca8..f357b9b63b1 100644 --- a/src/test/regress/expected/identity.out +++ b/src/test/regress/expected/identity.out @@ -365,6 +365,78 @@ SELECT seqtypid::regtype FROM pg_sequence WHERE seqrelid = 'itest3_a_seq'::regcl ALTER TABLE itest3 ALTER COLUMN a TYPE text; -- error ERROR: identity column type must be smallint, integer, or bigint +-- check that unlogged propagates to sequence +CREATE UNLOGGED TABLE itest17 (a int NOT NULL, b text); +ALTER TABLE itest17 ALTER COLUMN a ADD GENERATED ALWAYS AS IDENTITY; +ALTER TABLE itest17 ADD COLUMN c int GENERATED ALWAYS AS IDENTITY; +\d itest17 + Unlogged table "public.itest17" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+------------------------------ + a | integer | | not null | generated always as identity + b | text | | | + c | integer | | not null | generated always as identity + +\d itest17_a_seq + Unlogged sequence "public.itest17_a_seq" + Type | Start | Minimum | Maximum | Increment | Cycles? | Cache +---------+-------+---------+------------+-----------+---------+------- + integer | 1 | 1 | 2147483647 | 1 | no | 1 +Sequence for identity column: public.itest17.a + +\d itest17_c_seq + Unlogged sequence "public.itest17_c_seq" + Type | Start | Minimum | Maximum | Increment | Cycles? | Cache +---------+-------+---------+------------+-----------+---------+------- + integer | 1 | 1 | 2147483647 | 1 | no | 1 +Sequence for identity column: public.itest17.c + +CREATE TABLE itest18 (a int NOT NULL, b text); +ALTER TABLE itest18 SET UNLOGGED, ALTER COLUMN a ADD GENERATED ALWAYS AS IDENTITY; +\d itest18 + Unlogged table "public.itest18" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+------------------------------ + a | integer | | not null | generated always as identity + b | text | | | + +\d itest18_a_seq + Unlogged sequence "public.itest18_a_seq" + Type | Start | Minimum | Maximum | Increment | Cycles? | Cache +---------+-------+---------+------------+-----------+---------+------- + integer | 1 | 1 | 2147483647 | 1 | no | 1 +Sequence for identity column: public.itest18.a + +ALTER TABLE itest18 SET LOGGED; +\d itest18 + Table "public.itest18" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+------------------------------ + a | integer | | not null | generated always as identity + b | text | | | + +\d itest18_a_seq + Sequence "public.itest18_a_seq" + Type | Start | Minimum | Maximum | Increment | Cycles? | Cache +---------+-------+---------+------------+-----------+---------+------- + integer | 1 | 1 | 2147483647 | 1 | no | 1 +Sequence for identity column: public.itest18.a + +ALTER TABLE itest18 SET UNLOGGED; +\d itest18 + Unlogged table "public.itest18" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+------------------------------ + a | integer | | not null | generated always as identity + b | text | | | + +\d itest18_a_seq + Unlogged sequence "public.itest18_a_seq" + Type | Start | Minimum | Maximum | Increment | Cycles? | Cache +---------+-------+---------+------------+-----------+---------+------- + integer | 1 | 1 | 2147483647 | 1 | no | 1 +Sequence for identity column: public.itest18.a + -- kinda silly to change property in the same command, but it should work ALTER TABLE itest3 ADD COLUMN c int GENERATED BY DEFAULT AS IDENTITY, diff --git a/src/test/regress/sql/identity.sql b/src/test/regress/sql/identity.sql index 9f35214751c..7b0800226ca 100644 --- a/src/test/regress/sql/identity.sql +++ b/src/test/regress/sql/identity.sql @@ -214,6 +214,24 @@ SELECT seqtypid::regtype FROM pg_sequence WHERE seqrelid = 'itest3_a_seq'::regcl ALTER TABLE itest3 ALTER COLUMN a TYPE text; -- error +-- check that unlogged propagates to sequence +CREATE UNLOGGED TABLE itest17 (a int NOT NULL, b text); +ALTER TABLE itest17 ALTER COLUMN a ADD GENERATED ALWAYS AS IDENTITY; +ALTER TABLE itest17 ADD COLUMN c int GENERATED ALWAYS AS IDENTITY; +\d itest17 +\d itest17_a_seq +\d itest17_c_seq +CREATE TABLE itest18 (a int NOT NULL, b text); +ALTER TABLE itest18 SET UNLOGGED, ALTER COLUMN a ADD GENERATED ALWAYS AS IDENTITY; +\d itest18 +\d itest18_a_seq +ALTER TABLE itest18 SET LOGGED; +\d itest18 +\d itest18_a_seq +ALTER TABLE itest18 SET UNLOGGED; +\d itest18 +\d itest18_a_seq + -- kinda silly to change property in the same command, but it should work ALTER TABLE itest3 ADD COLUMN c int GENERATED BY DEFAULT AS IDENTITY, -- 2.39.5