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

On 8/3/2026 1:00 AM, Chao Li wrote:
>
>
>> On Aug 2, 2026, at 19:52, jian he <jian(dot)universality(at)gmail(dot)com> wrote:
>>
>> Hi.
>>
>> ATPostAlterTypeCleanup->performMultipleDeletions drops the
>> objects referenced by changedConstraintOids, changedIndexOids, and similar
>> fields of the AlteredTableInfo, it makes sense to set those pointers
>> to NULL afterward.
>> The attached patch implements this approach.
>>
>> Another reason why I prefer this approach:
>> Imagine some other random ALTER TABLE command that also needs to
>> rebuild whole-row dependent objects for the table,
>> then ATPostAlterTypeCleanup would also need to be called for that
>> AlterTablePass.
>> Ideally, we should be able to call ATPostAlterTypeCleanup for any kind
>> of AlterTablePass.
>>
>> The commit message is below:
>> ---------------------------------
>> ATPostAlterTypeCleanup() is called twice when a single ALTER TABLE contains both
>> ALTER COLUMN SET DATA TYPE and ALTER COLUMN SET EXPRESSION.
>> The first call drops the objects listed in tab->changedConstraintOids,
>> tab->changedIndexOids and tab->changedStatisticsOids via
>> performMultipleDeletions(), but left those lists untouched. The second call
>> would drop the same OIDs again, failing with errors like
>> "cache lookup failed".
>>
>> Fix by resetting the changed-object lists (and the replica identity and CLUSTER
>> index markings, which would otherwise queue duplicate subcommands) at the end of
>> ATPostAlterTypeCleanup(), so the second invocation only processes objects
>> registered by the SET EXPRESSION pass.
>> --------------------------------
>>
>>
>>
>> --
>> jian
>> https://www.enterprisedb.com/
>> <v6-0001-Fix-ALTER-TABLE-when-ALTER-TYPE-and-SET-EXPRESSION-are-used-toget.patch>
>
> Since my last round of review of this patch, I have done a lot of work on ALTER TABLE and thus gained a better understanding of the ALTER TABLE infra. I now agree that your approach (as in v6) is correct. The ALTER TYPE pass remembers and drops its dependencies, and SET EXPRESSION remembers and drops any additional ones.
>
> A small comment on the commit message:
> ```
> The first call drops the objects … but left those lists untouched.
> ```
>
> I think “left” should be “leaves” to keep the tense consistent with “drops”.
>
> Best regards,
> --
> Chao Li (Evan)
> HighGo Software Co., Ltd.
> https://www.highgo.com/

The fix for the double ATPostAlterTypeCleanup() call is right in what it
does, but rebased onto current master the list reset is incomplete, and
it appears the gap reintroduces CVE-2026-6469.

Since v6 was written, 6713a6e04cb added a third parallel list,
tab->changedStatisticsOwners. The patch resets
changedStatisticsOids/Defs but not changedStatisticsOwners. So on the
second call (the SET EXPRESSION pass) the new statistics objects are
paired with the previous leftover owners and the rebuild goes through
CreateStatistics() with check_rights disabled, silently installing the
wrong owner.

Reproducer -- a statistics object silently changes owner, and an
unrelated role can then drop the table owner's object:

CREATE ROLE r_owner;
CREATE ROLE r_other;
GRANT CREATE, USAGE ON SCHEMA public TO r_other;
CREATE TABLE t (k int, c int, b int GENERATED ALWAYS AS (k*2) STORED);
ALTER TABLE t OWNER TO r_owner;
CREATE STATISTICS s_c ON k, c FROM t; -- rebuilt in the ALTER TYPE pass
ALTER STATISTICS s_c OWNER TO r_other;
CREATE STATISTICS s_b ON k, b FROM t; -- rebuilt in the SET
EXPRESSION pass
ALTER STATISTICS s_b OWNER TO r_owner;

SET ROLE r_owner;
ALTER TABLE t ALTER COLUMN c TYPE bigint, ALTER COLUMN b SET
EXPRESSION AS (k*3);
RESET ROLE;

SELECT stxname, stxowner::regrole FROM pg_statistic_ext WHERE
stxrelid='t'::regclass;
-- HEAD: ERROR: cache lookup failed for statistics object ...
(s_b still r_owner)
-- patched: succeeds, and s_b is now owned by r_other

SET ROLE r_other;
DROP STATISTICS s_b; -- HEAD/fixed: ERROR: must be owner of
statistics object s_b
-- patched: succeeds -- r_other drops
r_owner's object
RESET ROLE;

The fix is to reset that list too, in the same block:

list_free(tab->changedStatisticsOwners);
tab->changedStatisticsOwners = NIL;

I applied exactly that on top of v6 and re-ran the reproducer: s_b keeps
its owner (r_owner), r_other's DROP is again refused with "must be
owner", and the original combined-ALTER case still succeeds.

It might be worth extending the owner-preservation test that came with
6713a6e04cb (stats_ext.sql) with a two-statistics-object ALTER TYPE +
SET EXPRESSION case, since the existing test exercises only a single
ALTER TYPE pass and wouldn't catch this.

With that list added, the rest of the approach looks correct to me.

--
Bryan Green
EDB: https://www.enterprisedb.com