Whole row var issue
I had Claude code look for bugs in new Postgres 19 features. It
authored the test case added by the attached patch. The test fails for
me on master (but not on 18) as follows:
# SELECT x.m0, x::text FROM t_missing_wholerow x ORDER BY 1;
# - m0 | x
# -----+------------
# - 40 | (5,,40,41)
# - 40 | (6,,40,41)
# + m0 | x
# +----+----------
# + 40 | (5,,,41)
# + 40 | (6,,,41)
# (2 rows)
As you can see, when run on master the query shows m0 with the value
40 when it is read as a column (which is correct). However, the same
query also incorrectly shows that m0 contains NULL values in each
whole row value. Obviously, m0 should have the same value within each
row (in this case 40, which comes from "ADD COLUMN m0 int NOT NULL
DEFAULT 40"), regardless of how it is queried or how the underlying
value is represented on disk.
CC'ing David Rowley, because git bisect indicates this test case
starts failing at commit c456e3911.
--
Peter Geoghegan
Attachments:
0001-Add-a-test-for-a-missing-attribute-lost-inside-a-who.patchapplication/octet-stream; name=0001-Add-a-test-for-a-missing-attribute-lost-inside-a-who.patchDownload+59-1
On Thu, 6 Aug 2026 at 11:34, Peter Geoghegan <pg@bowt.ie> wrote:
I had Claude code look for bugs in new Postgres 19 features. It
authored the test case added by the attached patch. The test fails for
me on master (but not on 18) as follows:# SELECT x.m0, x::text FROM t_missing_wholerow x ORDER BY 1;
# - m0 | x
# -----+------------
# - 40 | (5,,40,41)
# - 40 | (6,,40,41)
# + m0 | x
# +----+----------
# + 40 | (5,,,41)
# + 40 | (6,,,41)
# (2 rows)
Thanks. Looks like the populate_isnull_array() tts_isnull population
writes back that the DEFAULT attribute is NULL for attributes greater
than what's in the tuple (which is valid as the array is always large
enough), but invalid as that might overwrite a tts_isnull value that
was set by the missing attribute code path in some a previous pass of
deformation for that tuple where we only deformed up to some previous
attribute.
Probably we can fix it by not doing the tts_isnull array population
when we've already got more slot->tts_nvalid attributes than what
appear in the tuple. I'll go and think about the best way to add that
check with the least amount of overhead...
Alternatively, we could rewrite the missing attributes starting at the
tuple's natts with each deform iteration, and that would put the extra
overhead just into the has-missing-attribute code path. It's probably
possible to form some wild case that ends up with some quadratic
overhead because only 1 extra attribute is being deformed with each
pass, but that plan probably is slow for other reasons anyway, so it
might be better doing it that way so as not to add the overhead to the
common path.
/me goes off to experiment.
David
On Wed, Aug 5, 2026 at 8:55 PM David Rowley <dgrowleyml@gmail.com> wrote:
Thanks. Looks like the populate_isnull_array() tts_isnull population
writes back that the DEFAULT attribute is NULL for attributes greater
than what's in the tuple (which is valid as the array is always large
enough), but invalid as that might overwrite a tts_isnull value that
was set by the missing attribute code path in some a previous pass of
deformation for that tuple where we only deformed up to some previous
attribute.
Incorrect results also seem possible with ALTER COLUMN ... SET NOT
NULL: old snapshots can sometimes see NULL values where they
shouldn't, due in part to a concurrent SET NOT NULL operation.
See the attached patch's isolation test, which fails for me on master.
I'm not sure if this is the same bug as the one you've diagnosed, or a
different one. But another git bisect also points to commit c456e3911,
so it's certainly an issue in the same area.
--
Peter Geoghegan
Attachments:
0001-Add-isolation-test-for-SET-NOT-NULL-with-an-older-sn.patchapplication/octet-stream; name=0001-Add-isolation-test-for-SET-NOT-NULL-with-an-older-sn.patchDownload+48-1
On Thu, 6 Aug 2026 at 12:54, David Rowley <dgrowleyml@gmail.com> wrote:
Alternatively, we could rewrite the missing attributes starting at the
tuple's natts with each deform iteration, and that would put the extra
overhead just into the has-missing-attribute code path. It's probably
possible to form some wild case that ends up with some quadratic
overhead because only 1 extra attribute is being deformed with each
pass, but that plan probably is slow for other reasons anyway, so it
might be better doing it that way so as not to add the overhead to the
common path.
I push this version instead. We already don't focus much on optimising
the case where we deform in multiple passes that much (e.g., calling
populate_isnull_array() from the start of the tuple again). IMO, it's
more reasonable to make deforming missing attributes in multiple
passes slightly slower at the expense of not adding any overhead to
the happy path. I spent too long on that happy path to want to add any
more instructions to it. I really doubt deforming multiple missing
attributes in separate deform passes is common. We'd have had a report
about this much sooner if it were.
David