On Mon, Sep 29, 2025 at 5:36 AM David Rowley <[email protected]> wrote: > > I do wonder what the exact reason was that AT_PASS_ALTER_TYPE and > AT_PASS_SET_EXPRESSION are separate passes. I'd have expected that's > maybe so that it's possible to alter the type of a column used within > a generated column, but looking at the following error message makes > me think I might not be correct in that thinking: > > create table test1 (a int, b int generated always as (a + 1) stored); > alter table test1 alter column a set data type bigint; > ERROR: cannot alter type of a column used by a generated column > DETAIL: Column "a" is used by generated column "b". > > There's probably some good explanation for the separate pass, but I'm > not sure of it for now. I'm including in the author and committer of > the patch which added this code to see if we can figure this out. >
I found the explanation link: https://www.postgresql.org/message-id/CAAJ_b96TYsRrqm%2BveXCBb6CJuHJh0opmAvnhw8iMvhCMFKO-Tg%40mail.gmail.com AT_PASS_SET_EXPRESSION should come after AT_PASS_ADD_COL, otherwise cases like below would fail. create table t1 (a int, b int generated always as (a + 1) stored); alter table t1 add column c int, alter column b set expression as (a + c); >AT_PASS_ALTER_TYPE and AT_PASS_ADD_COL cannot be together, the ALTER TYPE > cannot see that column. I guess this is the reason why we have two seperate PASS. please also check the attached patch. The idea is that if both generation expression and type are being changed, only call ATPostAlterTypeCleanup while the current pass is AT_PASS_SET_EXPRESSION.
From 44966ed78830d31ce6e3d64bf387dc380d12982e Mon Sep 17 00:00:00 2001 From: jian he <[email protected]> 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+tz45m...@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
