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

> On Oct 13, 2025, at 16:08, jian he <jian(dot)universality(at)gmail(dot)com> wrote:
>
> On Wed, Oct 1, 2025 at 11:04 AM jian he <jian(dot)universality(at)gmail(dot)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>

I think the correct solution should be my proposed v4 that has a fundamental difference from your current change is that:

* The current implementation as well as your current change, they do ATPostAlterTypeCleanup() in either ALTER_TYPE pass or SET_EXPRESSION pass. Say a table has columns a and b, and a constraint (a+b>5), then I alter both columns types in a single command, then it will alter a’s type, then rebuild the constant, and alter b’s type, then rebuild the constraint again. For the first rebuild, because c’s type has not been updated, the rebuild may potentially fail.

* My v4 defers ATPostAlterTypeCleanup() to a later pass, which allows all "set data type” and “set expression” to finish, then start to rebuild constraints. My v4 does the cleanup in next pass of SET_EXPRESSION, which might not be the best solution, instead, it’s just a quick PoC to demo my idea. Perhaps, we may add a new pass.

But to handle the complicated case that I described above, v4 is still not enough. All “changedXXXXX” stuffs should be moved to an upper level of tab. Still using the same example, the constrain depends on both columns an and b, so we should remember the constraint only once.

WDYT?

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