Bug: Whole-row var in indexes corrupts indexes after DDL
Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.
You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:
docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t253243psql -h localhost -U postgresBuilt from patchset v9 (message #9), September 20, 2026 at 12:14 PM.
Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:
git clone --branch t253243_9 https://github.com/hackorum-dev/postgres.gitIn a checkout you already have, add the fork once:
git remote add hackorum https://github.com/hackorum-dev/postgres.gitthen, for this patchset and every later one:
git fetch hackorum t253243_9 && git checkout t253243_9Patchset v9 (message #9) is on t253243_9
Hi,
The attached SQL script shows a clear index corruption case caused by
incomplete tracking of whole-row vars in index expressions and the
invalidation of those when the table's layout is modified by DDL. It
has similar effects as the bug reported by Ayush Tiwari and fixed with
a4639d64e2; the index wasn't built with new data after the table's
whole-row expression changed for existing rows through DDL.
I've attached a patch which adds explicit tracking for whole-row
expressions in pg_depend, and use it to make sure we're rebuilding
indexes and revalidating constraints where and when required, without
O(N) dependency scans.
(This was triggered by some traffic on the list and in the commit log
on the topic of whole-row support, apparently without noticing this
pre-existing issue that I've known existed but haven't had the time to
address or advocate for.)
There is one item that I haven't touched in this patch that might also
affect whole-row expressions, and that is column renaming:
Technically, changes in names can affect a function's output (see e.g.
row_to_json), but I haven't yet found an immutable function that has a
`record` argument and exposes column names in output, nor have I
worked up the courage to wire up index rebuilds into
ExecRenameStmt/renameatt().
Kind regards,
Matthias van de Meent
Databricks (https://www.databricks.com)
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]/messages/by-id/CACJufxHZsgn3zM5g-x7YmtFGzNDnRwR07S+GYfiUs+tZ45MDDw@mail.gmail.com. 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]/messages/by-id/e48a5d9a2d3d72985d61ee254314f5f5f5444a55.camel@cybertec.at, hash_record() indexes after
ADD or DROP COLUMN in 2023 [2]/messages/by-id/18244-8a7897de2acd5a08@postgresql.org, the work on whole-row dependencies for
DROP COLUMN and ALTER TYPE [3]/messages/by-id/CACJufxGA6KVQy7DbHGLVw9s9KKmpGyZt5ME6C7kEfjDpr2wZCw@mail.gmail.com, the virtual generated-column case [4]/messages/by-id/CAJTYsWVmfHyooKCj30suW8=Y6b8be7WcyYmuAKMD=2mTPFpDkg@mail.gmail.com,
and rebuilding whole-row CHECK constraints after SET EXPRESSION [5]/messages/by-id/CAJTYsWXOkyeDVbzymWc9sKrq7Y_MUv6XJXN4H9GfsBOPd3NJ+w@mail.gmail.com.
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]: /messages/by-id/CACJufxHZsgn3zM5g-x7YmtFGzNDnRwR07S+GYfiUs+tZ45MDDw@mail.gmail.com
[1]: /messages/by-id/e48a5d9a2d3d72985d61ee254314f5f5f5444a55.camel@cybertec.at
[2]: /messages/by-id/18244-8a7897de2acd5a08@postgresql.org
[3]: /messages/by-id/CACJufxGA6KVQy7DbHGLVw9s9KKmpGyZt5ME6C7kEfjDpr2wZCw@mail.gmail.com
[4]: /messages/by-id/CAJTYsWVmfHyooKCj30suW8=Y6b8be7WcyYmuAKMD=2mTPFpDkg@mail.gmail.com
[5]: /messages/by-id/CAJTYsWXOkyeDVbzymWc9sKrq7Y_MUv6XJXN4H9GfsBOPd3NJ+w@mail.gmail.com
On Sun, 23 Aug 2026 at 13:26, Andrey Borodin <x4mmm@yandex-team.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]/messages/by-id/CACJufxFD-LGP1Ccj58d4WgGAoq-+N1fwQkA=b5u5PQf_65Vo9A@mail.gmail.com 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 toERROR: 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]: /messages/by-id/CACJufxFD-LGP1Ccj58d4WgGAoq-+N1fwQkA=b5u5PQf_65Vo9A@mail.gmail.com
On Mon, 31 Aug 2026 at 14:39, Matthias van de Meent <boekewurm@gmail.com> wrote:
On Sun, 23 Aug 2026 at 13:26, Andrey Borodin <x4mmm@yandex-team.ru> wrote:
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.
v2 had the fixes for the issues you reported that were new and
specific to this patch. v3 is identical to v2 apart from
CompilerWarning fixes.
Kind regards,
Matthias van de Meent
Databricks (https://www.databricks.com)
Hi Matthias,
I found one remaining issue in the object-address changes in v2/v3.
pg_identify_object_as_address() does not produce a value that can be
passed back to pg_get_object_address():
WITH a AS (
SELECT *
FROM pg_identify_object_as_address(
'pg_class'::regclass, 'wr_multi'::regclass, -32768)
)
SELECT pg_get_object_address(type, object_names, object_args)
FROM a;
ERROR: unrecognized object type "table whole row"
The documentation describes these functions as inverses. I think we
either need to make "table whole row" an addressable object type, or use
another representation that pg_get_object_address() already understands.
This case would also make a useful addition to the object_address test.
The RTE_FUNCTION reproducer works for me with v2.
On Mon, Aug 31, 2026 at 2:39 PM Matthias van de Meent wrote:
I think the issue can and should be solved separately from this patch
Agreed. Jian's v6 in the linked thread [0]/messages/by-id/CACJufxFD-LGP1Ccj58d4WgGAoq-+N1fwQkA=b5u5PQf_65Vo9A@mail.gmail.com seems to provide the prerequisite
fix. The combined ADD COLUMN/DROP COLUMN reproducer still fails on v2
alone, so that fix needs to go in first or precede this patch in the
series.
I also agree that rejecting whole-row partition keys can be a separate
change, and that checking the relfilenode change is sufficient here. I
don't suspect a separate problem in the normal index rebuild machinery.
I think these attached reproducers were dropped somewhere along the way.
Oops, sorry, I missed the file. PFA. Nothing really useful though beyond
what you already fixed or must be fixed elsewhere.
RememberWholeRowDependentForRebuilding() is a void function, but:
+ return RememberAllDependentForRebuilding(tab, subtype, rel,
I think you can just call RememberAllDependentForRebuilding() without a
return.
Thank you!
Best regards, Andrey Borodin.
[0]: /messages/by-id/CACJufxFD-LGP1Ccj58d4WgGAoq-+N1fwQkA=b5u5PQf_65Vo9A@mail.gmail.com
Attachments:
Hi Matthias,
I tested v3 locally against current master.
For the original reproducer, I can reproduce the problem on unpatched
master. With v3 applied, the original case passes and the whole-row
dependent index is rebuilt as expected.
I also tested the combined ADD COLUMN / DROP COLUMN case discussed
earlier in the thread. With v3 alone, I can reproduce the known
```
ERROR: cache lookup failed for index ...
```
failure. With CF 6121 v6 applied before CF 7071 v3, the cache lookup
failure is gone.
I also verified the object-address round-trip case reported by Andrey:
```
WITH a AS (
SELECT *
FROM pg_identify_object_as_address(
'pg_class'::regclass, 'wr_multi'::regclass, -32768)
)
SELECT pg_get_object_address(type, object_names, object_args)
FROM a;
```
With CF 6121 v6 and CF 7071 v3 applied, this still fails with:
```
ERROR: unrecognized object type "table whole row"
```
So my current testing confirms that v3 fixes the original whole-row
index corruption case. The combined ALTER TABLE failure is resolved
when the prerequisite cleanup fix is applied, while the object-address
round-trip issue remains.
I'll continue with some additional testing around combinations of
row-shape-changing DDL and whole-row dependent objects and report
anything new I find.
Regards,
Clemenza
On Fri, 4 Sept 2026 at 20:16, Andrey Borodin <x4mmm@yandex-team.ru> wrote:
Hi Matthias,
I found one remaining issue in the object-address changes in v2/v3.
pg_identify_object_as_address() does not produce a value that can be
passed back to pg_get_object_address():WITH a AS (
SELECT *
FROM pg_identify_object_as_address(
'pg_class'::regclass, 'wr_multi'::regclass, -32768)
)
SELECT pg_get_object_address(type, object_names, object_args)
FROM a;ERROR: unrecognized object type "table whole row"
The documentation describes these functions as inverses.
I'm not convinced they are that. pg_identify_object_as_address() will
happily produce "<relkind> column" output, and with several relkinds
these don't survive a round trip (specifically known failing cases:
indexes, views; for specifics check objectaddress.c's ObjectTypeMap).
But regardless, I've added support for decoding "table whole row" and
"foreign table whole row", which were the only objects that also
supported "<relkind> column", so that the hole isn't any smaller than
that of direct attribute references.
I think we
either need to make "table whole row" an addressable object type, or use
another representation that pg_get_object_address() already understands.
This case would also make a useful addition to the object_address test.
I've added a OBJECT_WHOLE_ROW type, treated like OBJECT_ATTRIBUTE but
slightly differently, and wired it into some relevant places.
RememberWholeRowDependentForRebuilding() is a void function, but:
+ return RememberAllDependentForRebuilding(tab, subtype, rel,
I think you can just call RememberAllDependentForRebuilding() without a
return.
Adjusted.
----------------------------------------
Attached is v4, which I believe addresses the feedback received so
far. It does not fix the pre-existing ATPostAlterTypeCleanup issue.
Kind regards,
Matthias van de Meent
Databricks (https://www.databricks.com)
Hi Matthias,
I tested v4 on current master and added a few tests beyond the existing
relfilenode checks.
The results look good so far.
For whole-row expression indexes, I tested table-shape changes through
ADD COLUMN, DROP COLUMN, and ALTER COLUMN TYPE. In each case the index
was rebuilt, and a lookup using the rebuilt whole-row expression index
returned the expected row. For the ADD COLUMN case I also checked the
plan explicitly and confirmed that the query used the expression
index, rather than getting the correct result through a sequential
scan.
I also checked a negative case: an ordinary index on an unrelated
column was not rebuilt when other columns were added or dropped.
The new whole-row dependency is present in pg_depend with
WholeRowAttrNumber as expected.
I retested the object-address issue from the previous version. The
table whole-row case now survives the
pg_identify_object_as_address() -> pg_get_object_address() round trip.
I also tested the corresponding foreign-table whole-row case, and that
round trip succeeds as well. An ordinary table-column round trip still
works, so I did not see a regression in the existing attribute path.
I also exercised a whole-row Var through an RTE_FUNCTION and did not
see the failure discussed earlier.
So I have not found any new correctness issue in these tests.
One possible test-coverage suggestion: the current indexing regression
test checks that relfilenode changes after ADD COLUMN, ALTER TYPE, and
DROP COLUMN. It might be worth also checking the result of an actual
lookup through the rebuilt expression index, since stale index contents
were the user-visible failure mode of the original bug.
Likewise, since v4 adds OBJECT_WHOLE_ROW specifically to address the
object-address issue, the table whole-row round-trip case might be
worth adding to the object-address regression tests.
Regards,
Clemenza Zhang
On Wed, Sep 9, 2026 at 3:08 AM Matthias van de Meent
<boekewurm@gmail.com> wrote:
Show quoted text
On Fri, 4 Sept 2026 at 20:16, Andrey Borodin <x4mmm@yandex-team.ru> wrote:
Hi Matthias,
I found one remaining issue in the object-address changes in v2/v3.
pg_identify_object_as_address() does not produce a value that can be
passed back to pg_get_object_address():WITH a AS (
SELECT *
FROM pg_identify_object_as_address(
'pg_class'::regclass, 'wr_multi'::regclass, -32768)
)
SELECT pg_get_object_address(type, object_names, object_args)
FROM a;ERROR: unrecognized object type "table whole row"
The documentation describes these functions as inverses.
I'm not convinced they are that. pg_identify_object_as_address() will
happily produce "<relkind> column" output, and with several relkinds
these don't survive a round trip (specifically known failing cases:
indexes, views; for specifics check objectaddress.c's ObjectTypeMap).But regardless, I've added support for decoding "table whole row" and
"foreign table whole row", which were the only objects that also
supported "<relkind> column", so that the hole isn't any smaller than
that of direct attribute references.I think we
either need to make "table whole row" an addressable object type, or use
another representation that pg_get_object_address() already understands.
This case would also make a useful addition to the object_address test.I've added a OBJECT_WHOLE_ROW type, treated like OBJECT_ATTRIBUTE but
slightly differently, and wired it into some relevant places.RememberWholeRowDependentForRebuilding() is a void function, but:
+ return RememberAllDependentForRebuilding(tab, subtype, rel,
I think you can just call RememberAllDependentForRebuilding() without a
return.Adjusted.
----------------------------------------
Attached is v4, which I believe addresses the feedback received so
far. It does not fix the pre-existing ATPostAlterTypeCleanup issue.Kind regards,
Matthias van de Meent
Databricks (https://www.databricks.com)
Please avoid top-posting replies, it makes it difficult to get the
context of what you're replying to.
On Wed, 9 Sept 2026 at 07:23, Clemenza Zhang <zxlmgsps2@gmail.com> wrote:
So I have not found any new correctness issue in these tests.
One possible test-coverage suggestion: the current indexing regression
test checks that relfilenode changes after ADD COLUMN, ALTER TYPE, and
DROP COLUMN. It might be worth also checking the result of an actual
lookup through the rebuilt expression index, since stale index contents
were the user-visible failure mode of the original bug.
As I mentioned upthread, I think it's sufficient to check that the
index was rebuilt, because there are no signs that the index
insertion's expression evaluation code itself is broken for whole-row
expressions, and because index rebuilds use a tablescan for the new
index data and not the old data of the index being rebuilt.
Likewise, since v4 adds OBJECT_WHOLE_ROW specifically to address the
object-address issue, the table whole-row round-trip case might be
worth adding to the object-address regression tests.
Attached version 5 includes updated object_address.sql tests.
Kind regards,
Matthias van de Meent
Databricks (https://www.databricks.com)