On Sat, Sep 27, 2025 at 5:54 PM jian he <[email protected]> wrote: > > While at it, maybe we can also polish the comment below in ATRewriteCatalogs. > /* > * After the ALTER TYPE or SET EXPRESSION pass, do cleanup work > * (this is not done in ATExecAlterColumnType since it should be > * done only once if multiple columns of a table are altered). > */ > but I didn't do it...
I add one sentence: Note: ATPostAlterTypeCleanup must be called only once per relation! I also polished the regress tests.
From a750440b9405a0fc4122583f11735295fe97e1e6 Mon Sep 17 00:00:00 2001 From: jian he <[email protected]> Date: Sat, 27 Sep 2025 22:17:04 +0800 Subject: [PATCH v2 1/1] ALTER TABLE each relation call ATPostAlterTypeCleanup only once bug demo: drop table if exists gtest25; CREATE TABLE gtest25 (a0 int, a int, b int GENERATED ALWAYS AS (a * 2 + a0) STORED); alter table gtest25 add constraint cc check (b > 0); alter table gtest25 alter column b set data type int8, ALTER COLUMN b SET EXPRESSION AS (a * 3 + a0); ERROR: cache lookup failed for constraint 18432 The reason it can only call once is: ATPostAlterTypeCleanup searches the system catalog for recreate objects such as STATISTICS, CONSTRAINTS. After that, the original object will be dropped via performMultipleDeletions. If ATPostAlterTypeCleanup is called a second time, the catalog cache lookup will fail because those objects were already removed during the first call. discussion: https://postgr.es/m/cacjufxhzsgn3zm5g-x7ymtfgzndnrwr07s+gyfius+tz45m...@mail.gmail.com --- src/backend/commands/tablecmds.c | 10 +++++++++- src/test/regress/expected/generated_stored.out | 9 +++++++++ src/test/regress/expected/generated_virtual.out | 9 +++++++++ src/test/regress/sql/generated_stored.sql | 3 +++ src/test/regress/sql/generated_virtual.sql | 3 +++ 5 files changed, 33 insertions(+), 1 deletion(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 3be2e051d32..cd5d1ca6251 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -5296,6 +5296,7 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode, AlterTableUtilityContext *context) { ListCell *ltab; + List *relids = NIL; /* * We process all the tables "in parallel", one pass at a time. This is @@ -5332,9 +5333,16 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode, * After the ALTER TYPE or SET EXPRESSION pass, do cleanup work * (this is not done in ATExecAlterColumnType since it should be * done only once if multiple columns of a table are altered). + * Note: ATPostAlterTypeCleanup must be called only once per relation! */ if (pass == AT_PASS_ALTER_TYPE || pass == AT_PASS_SET_EXPRESSION) - ATPostAlterTypeCleanup(wqueue, tab, lockmode); + { + if (!list_member_oid(relids, tab->relid)) + { + ATPostAlterTypeCleanup(wqueue, tab, lockmode); + relids = lappend_oid(relids, tab->relid); + } + } if (tab->rel) { diff --git a/src/test/regress/expected/generated_stored.out b/src/test/regress/expected/generated_stored.out index adac2cedfb2..cd1685fd90f 100644 --- a/src/test/regress/expected/generated_stored.out +++ b/src/test/regress/expected/generated_stored.out @@ -1120,6 +1120,15 @@ SELECT * FROM gtest25 ORDER BY a; Indexes: "gtest25_pkey" PRIMARY KEY, btree (a) +ALTER TABLE gtest25 ADD CONSTRAINT cc CHECK(b < 9) NOT VALID; +ALTER TABLE gtest25 ALTER COLUMN b SET DATA TYPE INT8, ALTER COLUMN b SET EXPRESSION AS (a * 4); --ok +SELECT * FROM gtest25 ORDER BY a; + a | b | c | x | d | y +---+----+----+-----+-----+----- + 3 | 12 | 42 | 168 | 101 | 404 + 4 | 16 | 42 | 168 | 101 | 404 +(2 rows) + -- ALTER TABLE ... ALTER COLUMN CREATE TABLE gtest27 ( a int, diff --git a/src/test/regress/expected/generated_virtual.out b/src/test/regress/expected/generated_virtual.out index d8645192351..db34851b041 100644 --- a/src/test/regress/expected/generated_virtual.out +++ b/src/test/regress/expected/generated_virtual.out @@ -1082,6 +1082,15 @@ SELECT * FROM gtest25 ORDER BY a; Indexes: "gtest25_pkey" PRIMARY KEY, btree (a) +ALTER TABLE gtest25 ADD CONSTRAINT cc CHECK(b < 9) NOT VALID; +ALTER TABLE gtest25 ALTER COLUMN b SET DATA TYPE INT8, ALTER COLUMN b SET EXPRESSION AS (a * 4); --ok +SELECT * FROM gtest25 ORDER BY a; + a | b | c | x | d | y +---+----+----+-----+-----+----- + 3 | 12 | 42 | 168 | 101 | 404 + 4 | 16 | 42 | 168 | 101 | 404 +(2 rows) + -- ALTER TABLE ... ALTER COLUMN CREATE TABLE gtest27 ( a int, diff --git a/src/test/regress/sql/generated_stored.sql b/src/test/regress/sql/generated_stored.sql index f56fde8d4e5..d4066cc9056 100644 --- a/src/test/regress/sql/generated_stored.sql +++ b/src/test/regress/sql/generated_stored.sql @@ -516,6 +516,9 @@ ALTER TABLE gtest25 ALTER COLUMN d SET DATA TYPE float8, ADD COLUMN y float8 GENERATED ALWAYS AS (d * 4) STORED; SELECT * FROM gtest25 ORDER BY a; \d gtest25 +ALTER TABLE gtest25 ADD CONSTRAINT cc CHECK(b < 9) NOT VALID; +ALTER TABLE gtest25 ALTER COLUMN b SET DATA TYPE INT8, ALTER COLUMN b SET EXPRESSION AS (a * 4); --ok +SELECT * FROM gtest25 ORDER BY a; -- ALTER TABLE ... ALTER COLUMN CREATE TABLE gtest27 ( diff --git a/src/test/regress/sql/generated_virtual.sql b/src/test/regress/sql/generated_virtual.sql index adfe88d74ae..250d3dd6a49 100644 --- a/src/test/regress/sql/generated_virtual.sql +++ b/src/test/regress/sql/generated_virtual.sql @@ -559,6 +559,9 @@ ALTER TABLE gtest25 ALTER COLUMN d SET DATA TYPE float8, ADD COLUMN y float8 GENERATED ALWAYS AS (d * 4) VIRTUAL; SELECT * FROM gtest25 ORDER BY a; \d gtest25 +ALTER TABLE gtest25 ADD CONSTRAINT cc CHECK(b < 9) NOT VALID; +ALTER TABLE gtest25 ALTER COLUMN b SET DATA TYPE INT8, ALTER COLUMN b SET EXPRESSION AS (a * 4); --ok +SELECT * FROM gtest25 ORDER BY a; -- ALTER TABLE ... ALTER COLUMN CREATE TABLE gtest27 ( -- 2.34.1
