Hi. ATPostAlterTypeCleanup->performMultipleDeletions drops the objects referenced by changedConstraintOids, changedIndexOids, and similar fields of the AlteredTableInfo, it makes sense to set those pointers to NULL afterward. The attached patch implements this approach.
Another reason why I prefer this approach: Imagine some other random ALTER TABLE command that also needs to rebuild whole-row dependent objects for the table, then ATPostAlterTypeCleanup would also need to be called for that AlterTablePass. Ideally, we should be able to call ATPostAlterTypeCleanup for any kind of AlterTablePass. The commit message is below: --------------------------------- ATPostAlterTypeCleanup() is called twice when a single ALTER TABLE contains both ALTER COLUMN SET DATA TYPE and ALTER COLUMN SET EXPRESSION. The first call drops the objects listed in tab->changedConstraintOids, tab->changedIndexOids and tab->changedStatisticsOids via performMultipleDeletions(), but left those lists untouched. The second call would drop the same OIDs again, failing with errors like "cache lookup failed". Fix by resetting the changed-object lists (and the replica identity and CLUSTER index markings, which would otherwise queue duplicate subcommands) at the end of ATPostAlterTypeCleanup(), so the second invocation only processes objects registered by the SET EXPRESSION pass. -------------------------------- -- jian https://www.enterprisedb.com/
From c99314a787c9b70b36064f0b827b9cb195489e9d Mon Sep 17 00:00:00 2001 From: jian he <[email protected]> Date: Sun, 2 Aug 2026 19:44:44 +0800 Subject: [PATCH v6 1/1] Fix ALTER TABLE when ALTER TYPE and SET EXPRESSION are used together ATPostAlterTypeCleanup() is called twice when a single ALTER TABLE contains both ALTER COLUMN SET DATA TYPE and ALTER COLUMN SET EXPRESSION. The first call drops the objects listed in tab->changedConstraintOids, tab->changedIndexOids and tab->changedStatisticsOids via performMultipleDeletions(), but left those lists untouched. The second call would drop the same OIDs again, failing with errors like "cache lookup failed". Fix by resetting the changed-object lists (and the replica identity and CLUSTER index markings, which would otherwise queue duplicate subcommands) at the end of ATPostAlterTypeCleanup(), so the second invocation only processes objects registered by the SET EXPRESSION pass. Oversight in 5d06e99a3, so backpatch to v17. Author: Jian He <[email protected]> Discussion: https://postgr.es/m/cacjufxhzsgn3zm5g-x7ymtfgzndnrwr07s+gyfius+tz45m...@mail.gmail.com --- src/backend/commands/tablecmds.c | 21 ++++++++++++ .../regress/expected/generated_stored.out | 34 +++++++++++++++++++ .../regress/expected/generated_virtual.out | 34 +++++++++++++++++++ src/test/regress/sql/generated_stored.sql | 15 ++++++++ src/test/regress/sql/generated_virtual.sql | 14 ++++++++ 5 files changed, 118 insertions(+) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 6d4c457b820..18ad78c582b 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -16315,6 +16315,27 @@ ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode) free_object_addresses(objects); + /* + * This function may be called a second time (once after the ALTER TYPE + * pass, again after the SET EXPRESSION pass). Reset the lists so objects + * already dropped and re-queued here aren't processed again. + */ + list_free(tab->changedConstraintOids); + list_free(tab->changedConstraintDefs); + list_free(tab->changedIndexOids); + list_free(tab->changedIndexDefs); + list_free(tab->changedStatisticsOids); + list_free(tab->changedStatisticsDefs); + + tab->changedConstraintOids = NIL; + tab->changedConstraintDefs = NIL; + tab->changedIndexOids = NIL; + tab->changedIndexDefs = NIL; + tab->changedStatisticsOids = NIL; + tab->changedStatisticsDefs = NIL; + tab->replicaIdentityIndex = NULL; + tab->clusterOnIndex = NULL; + /* * The objects will get recreated during subsequent passes over the work * queue. diff --git a/src/test/regress/expected/generated_stored.out b/src/test/regress/expected/generated_stored.out index 6a8b5113e73..8d4819970d0 100644 --- a/src/test/regress/expected/generated_stored.out +++ b/src/test/regress/expected/generated_stored.out @@ -1235,6 +1235,40 @@ SELECT * FROM gtest25 ORDER BY a; Indexes: "gtest25_pkey" PRIMARY KEY, btree (a) +ALTER TABLE gtest25 + ADD COLUMN z INT DEFAULT 11, + ADD CONSTRAINT cc CHECK(b > 9) NOT VALID, + ADD CONSTRAINT check_z CHECK(z > 9) NOT VALID; +ALTER TABLE gtest25 + ALTER COLUMN z SET DATA TYPE numeric, + ALTER COLUMN b SET EXPRESSION AS (z + 1); -- ok +ALTER TABLE gtest25 + ALTER COLUMN b SET DATA TYPE numeric, + ALTER COLUMN b SET EXPRESSION AS (z + 2); -- ok +SELECT * FROM gtest25 ORDER BY a; + a | b | c | x | d | y | z +---+----+----+-----+-----+-----+---- + 3 | 13 | 42 | 168 | 101 | 404 | 11 + 4 | 13 | 42 | 168 | 101 | 404 | 11 +(2 rows) + +\d gtest25 + Table "generated_stored_tests.gtest25" + Column | Type | Collation | Nullable | Default +--------+------------------+-----------+----------+------------------------------------------------------ + a | integer | | not null | + b | numeric | | | generated always as (z + 2::numeric) stored + c | integer | | | 42 + x | integer | | | generated always as (c * 4) stored + d | double precision | | | 101 + y | double precision | | | generated always as (d * 4::double precision) stored + z | numeric | | | 11 +Indexes: + "gtest25_pkey" PRIMARY KEY, btree (a) +Check constraints: + "cc" CHECK (b > 9::numeric) NOT VALID + "check_z" CHECK (z > 9::numeric) NOT VALID + -- 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 6ee029796f1..ca995b296da 100644 --- a/src/test/regress/expected/generated_virtual.out +++ b/src/test/regress/expected/generated_virtual.out @@ -1184,6 +1184,40 @@ SELECT * FROM gtest25 ORDER BY a; Indexes: "gtest25_pkey" PRIMARY KEY, btree (a) +ALTER TABLE gtest25 + ADD COLUMN z INT DEFAULT 11, + ADD CONSTRAINT cc CHECK(b > 9) NOT VALID, + ADD CONSTRAINT check_z CHECK(z > 9) NOT VALID; +ALTER TABLE gtest25 + ALTER COLUMN z SET DATA TYPE numeric, + ALTER COLUMN b SET EXPRESSION AS (z + 1); -- ok +ALTER TABLE gtest25 + ALTER COLUMN b SET DATA TYPE numeric, + ALTER COLUMN b SET EXPRESSION AS (z + 2); -- ok +SELECT * FROM gtest25 ORDER BY a; + a | b | c | x | d | y | z +---+----+----+-----+-----+-----+---- + 3 | 13 | 42 | 168 | 101 | 404 | 11 + 4 | 13 | 42 | 168 | 101 | 404 | 11 +(2 rows) + +\d gtest25 + Table "generated_virtual_tests.gtest25" + Column | Type | Collation | Nullable | Default +--------+------------------+-----------+----------+----------------------------------------------- + a | integer | | not null | + b | numeric | | | generated always as (z + 2::numeric) + c | integer | | | 42 + x | integer | | | generated always as (c * 4) + d | double precision | | | 101 + y | double precision | | | generated always as (d * 4::double precision) + z | numeric | | | 11 +Indexes: + "gtest25_pkey" PRIMARY KEY, btree (a) +Check constraints: + "cc" CHECK (b > 9::numeric) NOT VALID + "check_z" CHECK (z > 9::numeric) NOT VALID + -- 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 b349a16ddf3..09406f88316 100644 --- a/src/test/regress/sql/generated_stored.sql +++ b/src/test/regress/sql/generated_stored.sql @@ -594,6 +594,21 @@ 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 COLUMN z INT DEFAULT 11, + ADD CONSTRAINT cc CHECK(b > 9) NOT VALID, + ADD CONSTRAINT check_z CHECK(z > 9) NOT VALID; + +ALTER TABLE gtest25 + ALTER COLUMN z SET DATA TYPE numeric, + ALTER COLUMN b SET EXPRESSION AS (z + 1); -- ok + +ALTER TABLE gtest25 + ALTER COLUMN b SET DATA TYPE numeric, + ALTER COLUMN b SET EXPRESSION AS (z + 2); -- ok +SELECT * FROM gtest25 ORDER BY a; +\d gtest25 + -- 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 ed9d50fe784..7d9fc835d67 100644 --- a/src/test/regress/sql/generated_virtual.sql +++ b/src/test/regress/sql/generated_virtual.sql @@ -609,6 +609,20 @@ 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 COLUMN z INT DEFAULT 11, + ADD CONSTRAINT cc CHECK(b > 9) NOT VALID, + ADD CONSTRAINT check_z CHECK(z > 9) NOT VALID; + +ALTER TABLE gtest25 + ALTER COLUMN z SET DATA TYPE numeric, + ALTER COLUMN b SET EXPRESSION AS (z + 1); -- ok + +ALTER TABLE gtest25 + ALTER COLUMN b SET DATA TYPE numeric, + ALTER COLUMN b SET EXPRESSION AS (z + 2); -- ok +SELECT * FROM gtest25 ORDER BY a; +\d gtest25 -- ALTER TABLE ... ALTER COLUMN CREATE TABLE gtest27 ( -- 2.34.1
