> 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>
Now I fully understand what your concern is, so that I understand your solution
also. So, I take back my previous reply and re-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() will add 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 will be 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 can directly check if b has SET
EXPRESSION cmd, if yes, we don’t need to do anything; otherwise, we get the b’s
expression and 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/
