Re: Bug: Whole-row var in indexes corrupts indexes after DDL

On Sun, 23 Aug 2026 at 13:26, Andrey Borodin <x4mmm(at)yandex-team(dot)ru> wrote:
>
> Hi Matthias,
>
> Thanks for working on this! I took a look into v1.
>
> First, with v1 applied, a statement with shape changes in more than one
> ALTER TABLE pass fails:
>
> CREATE TABLE t(a int, b int);
> CREATE FUNCTION f(t) RETURNS t
> LANGUAGE sql IMMUTABLE RETURN $1;
> CREATE INDEX t_idx ON t ((f(t)));
> ALTER TABLE t ADD COLUMN c int, DROP COLUMN b;
>
> ERROR: cache lookup failed for index ...
>
> ATPostAlterTypeCleanup() runs after both AT_PASS_ADD_COL and
> AT_PASS_DROP. The first call drops the index, but its OID remains in
> tab->changedIndexOids, so the second call tries to process it again.
> This appears to be the same underlying issue that Jian He reported for
> ALTER TYPE combined with SET EXPRESSION [0]. The new cleanup calls make
> ADD COLUMN combined with DROP COLUMN hit it too.

I'm aware of the issue, and as you mention the issue is not exactly
new in this patch; [0] shows it is present in HEAD too. Whilst this
patch only exposes the user to more cases where it might appear, I
think the issue can and should be solved separately from this patch,
so that this patch can then be adapted to accomodate whichever changes
needed to include that same fix.

> Second, a whole-row partition expression can still leave a table
> mispartitioned. Its self-dependency is reversed, so looking for objects
> that depend on WholeRowAttrNumber does not find the partition key. In
> my test, adding a nullable column changed the partition key of an
> existing row from 1 to 0. The old row remained in partition 1, while a
> new row was routed to partition 0. Perhaps shape-changing DDL should be
> rejected when a partition key contains a whole-row reference.

Yes, I don't think partitioning's row routing can be solved with the
normal index/constraint rebuild system currently in place. I'll keep
it on the radar, but it's sufficiently different that I don't think it
needs to be fixed in the same patchset as one that fixes whole-row var
expressions in index- and constraint definitions.

> Rebuilding the expression would not move existing rows between
> partitions.

Correct. Alternatively we should reject whole-row expressions in
partition keys. I think rejecting them makes more sense, because it
simplifies DDL behaviour, and avoids issues with column ordering which
would then possibly affect row placement.

> There also seems to be a regression for whole-row Vars referencing an
> RTE_FUNCTION. find_expr_references_walker() now passes
> WholeRowAttrNumber to process_function_rte_ref(), but that function
> expects a positive output-column number. It therefore falls through to
>
> ERROR: column -32768 of relation "..." does not exist
>
> The new pseudo-subobject needs handling in the rest of the object
> address API too. pg_describe_object() reports it correctly, but
> pg_identify_object() and pg_identify_object_as_address() currently call
> it a "table column" and produce an empty identity.

Thanks for reporting, I've adjusted my patch accordingly.

> Some smaller points:
>
> * ATExecAddColumn() calls RememberWholeRowDependentForRebuilding()
> twice.
> * ATExecAlterColumnType() passes AT_AddColumn to that function.

Thanks, those were some copy-paste errors. Fixed in v2.

> I think the regression test should also compare the result of the
> original partial-index query using an index scan and a sequential scan.
> Checking only that relfilenode changes would not detect a rebuild that
> still produces an incorrect index.

While you are correct, I think noticing the rebuild should be
sufficient, given that this is an otherwise normal index rebuild
triggered by DDL, not unlike those of ALTER COLUMN TYPE. That is,
unless you suspect there are issues in index expression handling after
the DDL modifications themselves have been applied.

> The combined ALTER TABLE and
> partition-key cases above would also be useful. I attached the complete
> reproducers.

I think these attached reproducers were dropped somewhere along the way.

> For the record, an AI-assisted grep over the Hackorum archive puts the
> virtual generated-column case (a4639d64e2) mentioned in the opening
> message into a longer sequence: a unique whole-row index becoming
> inconsistent after DROP COLUMN in 2020 [1], hash_record() indexes after
> ADD or DROP COLUMN in 2023 [2], the work on whole-row dependencies for
> DROP COLUMN and ALTER TYPE [3], the virtual generated-column case [4],
> and rebuilding whole-row CHECK constraints after SET EXPRESSION [5].
> This seems like good evidence that the general dependency is worth
> representing, rather than fixing each new manifestation separately.

This evidence is exactly why I started this patch.

Kind regards,

Matthias van de Meent
Databricks (https://www.databricks.com)

[0]: https://postgr.es/m/CACJufxFD-LGP1Ccj58d4WgGAoq-+N1fwQkA=b5u5PQf_65Vo9A@mail.gmail.com