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

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.

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.
Rebuilding the expression would not move existing rows between
partitions.

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.

Some smaller points:

* ATExecAddColumn() calls RememberWholeRowDependentForRebuilding()
twice.
* ATExecAlterColumnType() passes AT_AddColumn to that function.

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. The combined ALTER TABLE and
partition-key cases above would also be useful. I attached the complete
reproducers.

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.

Thank you!

Best regards, Andrey Borodin.

[0] https://postgr.es/m/CACJufxHZsgn3zM5g-x7YmtFGzNDnRwR07S+GYfiUs+tZ45MDDw@mail.gmail.com
[1] https://postgr.es/m/e48a5d9a2d3d72985d61ee254314f5f5f5444a55.camel@cybertec.at
[2] https://postgr.es/m/18244-8a7897de2acd5a08@postgresql.org
[3] https://postgr.es/m/CACJufxGA6KVQy7DbHGLVw9s9KKmpGyZt5ME6C7kEfjDpr2wZCw@mail.gmail.com
[4] https://postgr.es/m/CAJTYsWVmfHyooKCj30suW8=Y6b8be7WcyYmuAKMD=2mTPFpDkg@mail.gmail.com
[5] https://postgr.es/m/CAJTYsWXOkyeDVbzymWc9sKrq7Y_MUv6XJXN4H9GfsBOPd3NJ+w@mail.gmail.com