Re: bug, ALTER TABLE call ATPostAlterTypeCleanup twice for the same relation

> On Sep 29, 2025, at 05:36, David Rowley <dgrowleyml(at)gmail(dot)com> wrote:
>
> On Sat, 27 Sept 2025 at 21:54, jian he <jian(dot)universality(at)gmail(dot)com> wrote:
>> 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);
>> + }
>> + }
>
>
> drop table if exists gtest25;
> CREATE TABLE gtest25 (a0 int, z int, a int, b int GENERATED ALWAYS AS
> (a * 2 + a0) STORED);
> alter table gtest25 add constraint check_z check (z > 0);
> alter table gtest25 add constraint cc check (b > 0);
>
> select oid, conname,conbin from pg_constraint where
> conrelid='gtest25'::regclass;
>
> alter table gtest25 alter column z set data type numeric, ALTER
> COLUMN b SET EXPRESSION AS (z + 0);
> select oid, conname,conbin from pg_constraint where
> conrelid='gtest25'::regclass;
>
> and that's certainly wrong as if I separate the ALTER TABLE into two
> separate commands, then "cc" does get recreated.
>
> 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.
>

Based on the comment of ATPostAlterTypeCleanup(), it says “cleanup after we’ve finished all the ALTER TYPE or SET EXPRESSION operations for a particular relation”, I tried this dirty fix:

```
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index fc89352b661..5c9c6d2a7db 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -5296,6 +5296,8 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
AlterTableUtilityContext *context)
{
ListCell *ltab;
+ bool needAlterTypeCleanup = false;
+ AlteredTableInfo *tabToCleanup = NULL;

/*
* We process all the tables "in parallel", one pass at a time. This is
@@ -5313,6 +5315,13 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
List *subcmds = tab->subcmds[pass];
ListCell *lcmd;

+ if (pass == AT_PASS_OLD_INDEX && needAlterTypeCleanup)
+ {
+ ATPostAlterTypeCleanup(wqueue, tabToCleanup, lockmode);
+ needAlterTypeCleanup = false;
+ tabToCleanup = NULL;
+ }
+
if (subcmds == NIL)
continue;

@@ -5334,7 +5343,10 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
* done only once if multiple columns of a table are altered).
*/
if (pass == AT_PASS_ALTER_TYPE || pass == AT_PASS_SET_EXPRESSION)
- ATPostAlterTypeCleanup(wqueue, tab, lockmode);
+ {
+ needAlterTypeCleanup = true;
+ tabToCleanup = tab;
+ }

if (tab->rel)
{
```

It postpones the cleanup to after all “alter type” and “set expression”. With this fix, David’s test case also passes:

```
evantest=# select oid, conname,conbin from pg_constraint where
evantest-# conrelid='gtest25'::regclass;
oid | conname | conbin
-------+---------+----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
32778 | check_z | {OPEXPR :opno 521 :opfuncid 147 :opresulttype 16 :opretset false :opcollid 0 :inputcollid 0 :args ({VAR :varno 1 :varattno 2 :vartype 23 :vartypmod -1 :varcollid 0 :varnullingrels (b) :varlevelsup 0 :varreturningtype 0 :varnosyn 1 :varattnosyn 2 :location -1} {CONST :consttype 23 :consttypmod -1 :constcollid 0 :constlen 4 :constbyval true :constisnull false :location -1 :constvalue 4 [ 0 0 0 0 0 0 0 0 ]}) :location -1}
32779 | cc | {OPEXPR :opno 521 :opfuncid 147 :opresulttype 16 :opretset false :opcollid 0 :inputcollid 0 :args ({VAR :varno 1 :varattno 4 :vartype 23 :vartypmod -1 :varcollid 0 :varnullingrels (b) :varlevelsup 0 :varreturningtype 0 :varnosyn 1 :varattnosyn 4 :location -1} {CONST :consttype 23 :consttypmod -1 :constcollid 0 :constlen 4 :constbyval true :constisnull false :location -1 :constvalue 4 [ 0 0 0 0 0 0 0 0 ]}) :location -1}
(2 rows)

evantest=# alter table gtest25 alter column z set data type numeric, ALTER
evantest-# COLUMN b SET EXPRESSION AS (z + 0);
ALTER TABLE
evantest=# select oid, conname,conbin from pg_constraint where
evantest-# conrelid='gtest25'::regclass;
oid | conname | conbin
-------+---------+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
32781 | check_z | {OPEXPR :opno 1756 :opfuncid 1720 :opresulttype 16 :opretset false :opcollid 0 :inputcollid 0 :args ({VAR :varno 1 :varattno 2 :vartype 1700 :vartypmod -1 :varcollid 0 :varnullingrels (b) :varlevelsup 0 :varreturningtype 0 :varnosyn 1 :varattnosyn 2 :location -1} {FUNCEXPR :funcid 1740 :funcresulttype 1700 :funcretset false :funcvariadic false :funcformat 2 :funccollid 0 :inputcollid 0 :args ({CONST :consttype 23 :consttypmod -1 :constcollid 0 :constlen 4 :constbyval true :constisnull false :location -1 :constvalue 4 [ 0 0 0 0 0 0 0 0 ]}) :location -1}) :location -1}
32782 | cc | {OPEXPR :opno 521 :opfuncid 147 :opresulttype 16 :opretset false :opcollid 0 :inputcollid 0 :args ({VAR :varno 1 :varattno 4 :vartype 23 :vartypmod -1 :varcollid 0 :varnullingrels (b) :varlevelsup 0 :varreturningtype 0 :varnosyn 1 :varattnosyn 4 :location -1} {CONST :consttype 23 :consttypmod -1 :constcollid 0 :constlen 4 :constbyval true :constisnull false :location -1 :constvalue 4 [ 0 0 0 0 0 0 0 0 ]}) :location -1}
(2 rows)

evantest=#
```

We can see both check_z and check_cc got rebuilt.

This fix uses the last “tab” with the cleanup function. In that way, TBH, I haven’t dig deeper enough to see if anything is missed in cleanup.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/