Re: bug, ALTER TABLE call ATPostAlterTypeCleanup twice for the same relation - Mailing list pgsql-hackers

> On Oct 13, 2025, at 16:08, jian he <jian.universality@gmail.com> wrote:
>
> On Wed, Oct 1, 2025 at 11:04 AM jian he <jian.universality@gmail.com> wrote:
>>
>> we can simply change from
>>
>>            if (pass == AT_PASS_ALTER_TYPE || pass == AT_PASS_SET_EXPRESSION)
>>                ATPostAlterTypeCleanup(wqueue, tab, lockmode);
>>
>> to
>>            if (pass == AT_PASS_SET_EXPRESSION)
>>                ATPostAlterTypeCleanup(wqueue, tab, lockmode);
>>            else if (pass == AT_PASS_ALTER_TYPE &&
>>                     tab->subcmds[AT_PASS_SET_EXPRESSION] == NIL)
>>                ATPostAlterTypeCleanup(wqueue, tab, lockmode);
>
> That will not work if AT_PASS_ALTER_TYPE triggers the creation of a new
> AT_PASS_SET_EXPRESSION entry.
> For example, consider:
> CREATE TABLE gtest25 (a int, b int GENERATED ALWAYS AS (a * 2) STORED);
>
> If we alter the type of column "a", the column b’s generation
> expression would need
> to be rebuilt (which is currently not supported).  In that case, the current
> logic would not be able to handle it.
>
> I think the correct fix is within ATPostAlterTypeCleanup.
> at the end of ATPostAlterTypeCleanup, we already delete these changed
> objects via
> ``performMultipleDeletions(objects, DROP_RESTRICT, PERFORM_DELETION_INTERNAL);``
>
> we can just list_free these (the below) objects too:
>    tab->changedConstraintOids
>    tab->changedConstraintDefs
>    tab->changedIndexOids
>    tab->changedIndexDefs
>    tab->changedStatisticsOids
>    tab->changedStatisticsDefs
>
> since this information would become stale if those objects have
> already been dropped.
> <v5-0001-avoid-call-ATPostAlterTypeCleanup-twice.patch>

Now I fully understand what your concern is, so that I understand your solution also. So, I take back my previous reply
andre-reply. 

Your concern comes from an assumed solution to support altering a column’s type when it has a dependent generated
column.Your assumed solution is like: 

1. SQL: ALTER TABLE gtest25 ALTER COLUMN a TYPE bigint; # generated column depends on a
2. Before alter column a’s type, b’s expression is remembered
3. After alter column a’s type, ATPostAlterTypeClean() is called as there is no SET EXPRESSION. ATPostAlterTypeClean()
willadd a SET EXPRESSION subcmd to rebuild b 
4. In SET_EXPRESSION PASS, b is recreated
5. ATPostAlterTypeClean() is called again, if we don’t cleanup those tab->changedXXX lists, then remembered objects
willbe rebuilt again, which may lead to errors. What’s why your solution of cleaning up table->changedXXX works. 

In my opinion, in step 2, we don’t have follow the same mechanism to remember generated expressions as constraints. We
candirectly check if b has SET EXPRESSION cmd, if yes, we don’t need to do anything; otherwise, we get the b’s
expressionand inject a SET EXPRESSION subcmd for b. In this case, your worried problem will not exist. With this
approach,ATPostAlterTypeClean() will always be called only once, so that execution path is simpler. 

But anyway, we haven’t decided to support that yet. So for the current bug, your solution of:

>>            if (pass == AT_PASS_SET_EXPRESSION)
>>                ATPostAlterTypeCleanup(wqueue, tab, lockmode);
>>            else if (pass == AT_PASS_ALTER_TYPE &&
>>                     tab->subcmds[AT_PASS_SET_EXPRESSION] == NIL)
>>                ATPostAlterTypeCleanup(wqueue, tab, lockmode);

should work.

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