opt: reclaim the row-group list a columnar rescan rebuilds (#734)

[jdatcmd]

Closes #734.

A rescan reuses the read state rather than closing and re-opening it
(PgColumnarReScanCustomScan → PgColumnarRescanRead), and that function sets
rowGroupList to NIL without freeing it. The next start rebuilds the list and
a NativeRowGroupMetadata per group into the read state's own context, so a
LATERAL or parameterized scan accumulated one list per outer row for the life of
the query.

The mechanism was pinned first, not guessed

A memory-context dump named it: columnar read, 8,388,608 bytes over 40,000
rescans. readContext itself cannot simply be reset — the read state and its
missing values live there too — so the list gets a child context the rescan
resets.

(I filed this issue deliberately without a hypothesis, having been wrong twice
guessing at this class on #727 and #731.)

Measured against a heap floor, not against zero

arm	before	after
plain columnar scan	198 B/rescan	33 B
heap table, identical data	33 B/rescan	33 B

Two proofs

Removal. Reverting gives 190 B against a 35 B floor — 155 B excess, arm red
— while the vectorized-aggregate arm beside it stays at 1 B, so the two arms are
independent.

Lifetime. This frees metadata structs, so a reset in the wrong place is a
use-after-free. Injecting one (reset while the list is live) is loudly fatal:
ERROR: invalid memory alloc request size 7741792704550691685 from a freed
struct, with every premise collapsing. Per asan-cannot-see-context-reset ASAN
cannot see a MemoryContextReset use-after-free at all, so validating that the
suite can see it is the check that matters here.

Gated on the full 209-suite PG17 matrix

Not a chosen neighbourhood — this is in the core reader and every read path uses
it, so picking neighbours would have been a guess about blast radius.

One suite failed, and it is not this change. native_upgrade_converge fails
identically on clean main, CI passes it on the same commit, and the cause is
two stale extension base scripts in the local sharedir that the repo no longer
ships (pgcolumnar--1.0-alpha.sql dated Aug 16, pgcolumnar--1.0-dev.sql dated
Aug 4), left behind by an August run of extension_upgrade.sh. The stale alpha
script already creates the Iceberg FDW; the current upgrade script then creates
it again.

They were present on every major, so the five-major release gate would have
failed on all of them for a reason that is not the code. Moved aside (not
deleted — the suite regenerates them when it can run); the suite then passes
5 of 5.

🤖 Generated with Claude Code

https://claude.ai/code/session_01G8jGnYAbTgiAcoUqn7E9EN

[OffgridwithJD]

APPROVE — reviewed as ChronicallyJD. Verified on the merged tree (current main 5dea386 + this PR), pg17a assert, in my audit container; core-reader change, so I ran the sanitizer too.

The fix

Giving the row-group list its own groupListContext child of readContext and resetting it in PgColumnarRescanRead is the right shape — readContext can't be reset (the read state and its missing values live there), and clearing rowGroupList to NIL alone never reclaimed the previous list. I checked the reset is safe rather than taking the comment's word: after MemoryContextReset(groupListContext) every pointer into it is cleared in the same block — rowGroupList = NIL, nativeGroup = NULL, and the native* cursors. The only cached element pointer is nativeGroup (via list_nth at the group boundary), and it's nulled. The other PgColumnarReadRowGroupList call sites build their lists in their own contexts, not this one, so nothing else dangles across the reset.

Removal proof — reproduced

Reverting the reader change reddens the #734 arm: plain columnar scan 190 B/rescan vs 33 B heap floor, excess 157 B (gate <100). With the fix, 33 vs 35, excess 0. The arm sees the fix's absence, and 157 B lands where the issue's 165 B did.

Lifetime — the reset is the risk, so I attacked it

A MemoryContextReset is invisible to ASAN when the freed blocks aren't returned to the C allocator, so the assert build carries this one. Injecting the reset while the list is still live is loudly fatal: ERROR: invalid memory alloc request size 7741792704550691685 (a freed struct read as a length), connection lost, premises collapsing — the same value you saw, reproduced independently. So the suite genuinely sees this class here; the shipped placement is safe because every pointer into the context is cleared before the next read.

Coexistence + surrounding path

This is the third rescan-memory fix in the same fixture. On the merged tree all of them hold together: #734 (plain scan) excess 0, #727 (vector agg) excess 0, #717 (scan keys) excess 0, row-path pruning still does its work. And the reader change doesn't disturb the pushdown rescans — native_saop_pushdown 30/30 and native_param_pushdown 5/5 green.

Sanitizer

ASAN+UBSAN clean over the rescan and read path — vector_agg_rescan_memory, native_saop_pushdown, native_roundtrip all pass under pg18_san, 0 checks vacated. Complementary to the assert-build inject above, since a context reset can be partly invisible to ASAN.

Docs

CHANGELOG is honest — the reuse-not-reopen mechanism, the one-list-per-outer-row accumulation, and the differential-vs-heap figure (198→33 against a 33 floor). No over-claim.

Mechanism pinned before the fix, proven by removal, lifetime attacked on the build that can see it. Clean.