> On Oct 13, 2025, at 16:08, jian he <[email protected]> wrote:
>
> On Wed, Oct 1, 2025 at 11:04 AM jian he <[email protected]> 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/
