From 44966ed78830d31ce6e3d64bf387dc380d12982e Mon Sep 17 00:00:00 2001 From: jian he Date: Mon, 29 Sep 2025 15:39:54 +0800 Subject: [PATCH v3 1/1] avoid call ATPostAlterTypeCleanup twice discussion: https://postgr.es/m/CACJufxHZsgn3zM5g-x7YmtFGzNDnRwR07S+GYfiUs+tZ45MDDw@mail.gmail.com --- src/backend/commands/tablecmds.c | 27 ++++++++++++++++++- .../regress/expected/generated_stored.out | 12 +++++++++ .../regress/expected/generated_virtual.out | 12 +++++++++ src/test/regress/sql/generated_stored.sql | 6 +++++ src/test/regress/sql/generated_virtual.sql | 6 +++++ 5 files changed, 62 insertions(+), 1 deletion(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index fc89352b661..4443b69c156 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -5296,6 +5296,20 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode, AlterTableUtilityContext *context) { ListCell *ltab; + List *relids = NIL; + + /* + * Collect relation that both type and generation expression are changed. + */ + foreach(ltab, *wqueue) + { + AlteredTableInfo *tab = (AlteredTableInfo *) lfirst(ltab); + List *subcmds1 = tab->subcmds[AT_PASS_SET_EXPRESSION]; + List *subcmds2 = tab->subcmds[AT_PASS_ALTER_TYPE]; + + if (subcmds1 != NIL && subcmds2 != NIL) + relids = lappend_oid(relids, tab->relid); + } /* * We process all the tables "in parallel", one pass at a time. This is @@ -5333,7 +5347,16 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode, * (this is not done in ATExecAlterColumnType since it should be * done only once if multiple columns of a table are altered). */ - if (pass == AT_PASS_ALTER_TYPE || pass == AT_PASS_SET_EXPRESSION) + if (list_member_oid(relids, tab->relid)) + { + /* + * If both type and generation expression being changed, do the + * cleanup at AT_PASS_SET_EXPRESSION + */ + if (pass == AT_PASS_SET_EXPRESSION) + ATPostAlterTypeCleanup(wqueue, tab, lockmode); + } + else if (pass == AT_PASS_ALTER_TYPE || pass == AT_PASS_SET_EXPRESSION) ATPostAlterTypeCleanup(wqueue, tab, lockmode); if (tab->rel) @@ -15578,6 +15601,8 @@ ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode) free_object_addresses(objects); + + /* * 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 adac2cedfb2..cf9c2836daa 100644 --- a/src/test/regress/expected/generated_stored.out +++ b/src/test/regress/expected/generated_stored.out @@ -1120,6 +1120,18 @@ SELECT * FROM gtest25 ORDER BY a; Indexes: "gtest25_pkey" PRIMARY KEY, btree (a) +ALTER TABLE gtest25 ADD COLUMN z INT DEFAULT 11; +ALTER TABLE gtest25 ADD CONSTRAINT cc CHECK(b > 9) NOT VALID; +ALTER TABLE gtest25 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 + 0); +ALTER TABLE gtest25 ALTER COLUMN b SET DATA TYPE numeric, ALTER COLUMN b SET EXPRESSION AS (a * 3); +SELECT * FROM gtest25 ORDER BY a; + a | b | c | x | d | y | z +---+----+----+-----+-----+-----+---- + 3 | 9 | 42 | 168 | 101 | 404 | 11 + 4 | 12 | 42 | 168 | 101 | 404 | 11 +(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 c861bd36c5a..e97ea287b73 100644 --- a/src/test/regress/expected/generated_virtual.out +++ b/src/test/regress/expected/generated_virtual.out @@ -1082,6 +1082,18 @@ SELECT * FROM gtest25 ORDER BY a; Indexes: "gtest25_pkey" PRIMARY KEY, btree (a) +ALTER TABLE gtest25 ADD COLUMN z INT DEFAULT 11; +-- ALTER TABLE gtest25 ADD CONSTRAINT cc CHECK(b > 9) NOT VALID; +-- ALTER TABLE gtest25 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 + 0); +ALTER TABLE gtest25 ALTER COLUMN b SET DATA TYPE numeric, ALTER COLUMN b SET EXPRESSION AS (a * 3); +SELECT * FROM gtest25 ORDER BY a; + a | b | c | x | d | y | z +---+----+----+-----+-----+-----+---- + 3 | 9 | 42 | 168 | 101 | 404 | 11 + 4 | 12 | 42 | 168 | 101 | 404 | 11 +(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..6d5359fdcee 100644 --- a/src/test/regress/sql/generated_stored.sql +++ b/src/test/regress/sql/generated_stored.sql @@ -516,6 +516,12 @@ 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; +ALTER TABLE gtest25 ADD CONSTRAINT cc CHECK(b > 9) NOT VALID; +ALTER TABLE gtest25 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 + 0); +ALTER TABLE gtest25 ALTER COLUMN b SET DATA TYPE numeric, ALTER COLUMN b SET EXPRESSION AS (a * 3); +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..9a94a2e311c 100644 --- a/src/test/regress/sql/generated_virtual.sql +++ b/src/test/regress/sql/generated_virtual.sql @@ -559,6 +559,12 @@ 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; +-- ALTER TABLE gtest25 ADD CONSTRAINT cc CHECK(b > 9) NOT VALID; +-- ALTER TABLE gtest25 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 + 0); +ALTER TABLE gtest25 ALTER COLUMN b SET DATA TYPE numeric, ALTER COLUMN b SET EXPRESSION AS (a * 3); +SELECT * FROM gtest25 ORDER BY a; -- ALTER TABLE ... ALTER COLUMN CREATE TABLE gtest27 ( -- 2.34.1