fix: expire must not drop live rows or leave them visible to index-only scans

[OffgridwithJD]

Summary

• pgcolumnar.expire decided a row group was fully past retention from the zone-map maximum alone. That maximum ignores NULLs, so a group of expired timestamps plus NULL retention values was retired and the NULL rows disappeared.
• Retiring a live group also left the visibility-map bits VACUUM had set. An index-only scan then returned the expired keys from the index without fetching (the group is gone, so a fetch would have correctly failed).
• Keep any group whose retention column has a NULL. Clear the VM range covering a group before retiring it.

Test plan

• test/ttl_expire.sh on PostgreSQL 18 in cusor-2604 (NULL retention rows kept; index-only scan returns 0 after expire)
• Existing expire fixture still retires fully-expired groups and keeps straddling groups

Made with Cursor

[OffgridwithJD]

Premise verified — this is silent data loss, and worse than the summary says

Measured on pg18a. One row group holding 900 rows whose timestamps are all 400
days old, of which 90 have a NULL ts, retention 90 days. The fixture is gated:
it refuses to report a verdict unless there is exactly one group (the bug
needs the expired values and the NULLs in the same group), 900 rows, and 90
NULLs.

                        main        #869
premise: groups=1 rows=900 nulls=90 zone_map null_count=90   (both arms)
expire dropped          1 group     0 groups
rows after              0           900
NULL rows after         0           90

On main, pgcolumnar.expire deleted every row in the table, including all 90
whose retention was unknown. docs/sql-reference.md promises the opposite:

A group is kept whole or dropped whole. ... Retention is therefore approximate
at the group boundary, and it errs toward keeping data.

A NULL retention is not "expired"; it is "unknown". Deleting it errs the other
way, and nothing tells the user it happened. I would put the row counts in the
PR body — "the NULL rows disappeared" understates a table going to zero.

The fix is not vacuous — control run

A guard that returns early can pass a keep-the-rows test by never expiring
anything. Same fixture with no NULLs:

[#869 control] premise: groups=1 rows=900 nulls=0
[#869 control] expire dropped 1 group(s); rows after = 0

So a fully-expired group with no NULLs is still retired. That is your unchecked
box — "Existing expire fixture still retires fully-expired groups" — and it holds
on the NULL-free side at least.

z->nullCount is real, which is what the guard depends on

Worth recording because the guard is worthless if the field is not maintained:
columnar_write_state.c:1335 sets z->nullCount = group->rowCount - col->valueCount,
columnar_metadata.c:2394 persists it, :2847 reads it back. My run confirms it
end to end — zone_map.null_count was 90 for the group in question.

What I did not verify

The visibility-map half. I did not construct an index-only scan returning ghost
keys, so PgColumnarVMClearForRowRange is unmeasured by me. The block arithmetic
reads correctly (b0..b1 inclusive over
rowNumber / COLUMNAR_VALID_ITEMPOINTER_OFFSETS), and clearing before
PgColumnarRetireGroup rather than after is the right order, but that is reading,
not running.

One thing to change: the PR carries no docs

Four files, none of them docs/ or CHANGELOG.md. The rule here is that a PR
ships its documentation, and this one changes documented behaviour:
docs/sql-reference.md describes expire's approximation as a boundary effect,
where a straddling group is kept until every row in it has expired. The NULL rule
is a different and stronger case — a single NULL pins its whole group forever.
Those rows never expire, no matter how old the rest of the group gets, because the
condition never stops being true.

That is the safe direction and I would not change it, but a user reading the
current page cannot predict it. It needs a sentence, and the CHANGELOG needs the
data-loss note.

Reviewed as OffgridwithJD. Not approving — same account as the author.