Review: read only the columns a query references (#338) [already merged as #339]

[ChronicallyJD]

Read this first. This change is already in main (merged as #339, commit 00a0955). I self-merged it under the standing autonomous authority before you asked for a review, and that was the wrong call for a change this size. This PR exists so the diff is actually reviewable: its base is pinned at 423fb7e, the commit immediately before the merge, so what you see below is exactly the change and nothing else.

Two things follow from that. It cannot be "merged" to land the code — the code has landed. And the review is currently non-blocking, which is not what a review should be. If you want it out of main until you have signed off, say so and I will revert the merge and we re-land it through a normal PR. I have not done that unilaterally because reverting a public main is not mine to decide after the fact.

---

What

Closes #338. The reader read and decoded every column of every row group it visited, regardless of how few columns the query needed. The projection was computed correctly by columnar_projected_columns, threaded down through ColumnarBeginRead, copied into the read state at columnar_reader.c:334 — and then never read again. That assignment was the field's only occurrence.

Measured

The TSBS shape from #289, 12 columns, 4M rows, 294 MB. Same data, same build, enable_column_projection off vs on:

	buffers	time
filtered avg over one metric, off	38,288	1343.6 ms
filtered avg over one metric, on	4,462	226.2 ms

8.6x less I/O, 5.9x faster, results identical (401054|95.0028955075411 both ways).

What it looked like before, on a 12-column 351 MB table (44,962 buffers):

query	columns needed	buffers	% of table
count(*)	0	3	0%
sum(a)	1	45,094	100%
count(*), avg(b) WHERE a>90	2	45,094	100%
sum of all 12	12	45,092	100%

One column cost the same bytes as twelve. Cross-checked against width: the identical two-column query read 100% of a 2-column table (7,620 buffers) and 100% of a 12-column one (45,097).

max() alone stays at 300 buffers either way — that is the zone-map fast path, correctly untouched.

How

columnar_native_load_group now fetches the chunk metadata before the data read (it is a catalog read and touches no data pages), then reads only the byte ranges the projected columns occupy, coalescing ranges that are file-adjacent. Chunks are written column-major, so a projection is a small number of contiguous runs rather than natts scattered reads. The group buffer is still allocated at full size so the existing base = nativeBuffer + (pageOffset - fileOffset) arithmetic is unchanged; palloc does not touch the pages it returns, so unread regions cost no resident memory.

The decode loop skips unwanted chunks too — that mattered as much as the I/O, since it previously ran the full decode over every column.

Blast radius is small by construction: a NULL projection means "all columns", and that is what every caller outside the custom scan and the two vectorized-aggregate paths passes — vacuum, parquet export, arrow, the tableam seqscan all keep the whole-group path byte for byte.

pgcolumnar.enable_column_projection (default on) is both the operational escape hatch and the A/B oracle the tests compare against.

Two things that fell out, neither of them the happy path

A corrupt row_group.byte_length stopped being detected. corruption.sh inflates it and requires a clean error; the whole-group read produced one incidentally, by reading a length that ran past the end of the relation. A projected read never uses that length to size anything, so the corruption went unnoticed.

Weakening the corruption test to match the new path would have been the wrong fix. Instead the reader now checks the invariant directly: chunks must exactly tile their group, no gap at the start and none at the end. That was verified to hold before being relied on — across plain inserts, ADD COLUMN, stored generated columns, updates and deletes, compact, vacuum_sorted, block-compressed columns, and VACUUM FULL. It is the better check anyway: it names the inconsistency rather than surfacing as a short read. Enforced on the projected path only, so enable_column_projection=off stays a way back.

cancel_decode broke, and was right to. Its scan filtered on one column of eight, so with projection it no longer ran long enough to cancel — its own premise check caught it. The query now names every column, so it again decodes the whole relation as the test intends, and keeps testing cancellation on the default path instead of being pinned to the old behavior.

Tests

test/column_projection.sh, 35 checks. Projection on equals projection off, and equals a heap oracle holding identical data, across: single and multi column targetlists, a qual on a column absent from the targetlist, count(*), SELECT *, whole-row Var, ctid, varlena, an all-NULL column, min/max over mixed types, a join, deletes, updates, and parallel scan.

The ADD COLUMN case is covered specifically, on both sides of the boundary: an unmaterialised column and a column predating an ADD COLUMN both leave the validity pointer NULL but mean different things, and conflating them would hand back a default in place of stored data.

The win is asserted as a buffer count, not a timing — exact, stable on a shared runner, and it fails loudly if projection silently stops applying.

Removal proofs

• Forcing allColumnsWanted true fails the buffer assertion: on: 2517, off: 2517.
• Disabling the tiling check fails corruption.sh's byte_length assertion.
• Removing the explicit-NULL guard for unwanted columns fails nothing. It is defensive, not load-bearing — nothing above the scan can read a column outside the projection — and it is documented as defensive rather than implied to be proven. It is kept because the cost is one array test and the failure it forecloses is silent and data-shaped.

One discarded proof worth naming: flipping the GUC's default proves nothing, because the suite sets the GUC explicitly on both arms. The default never reaches the code under test.

Gate

Full 15-19 matrix.

Design note: design/COLUMN_PROJECTION.md.

🤖 Generated with Claude Code

https://claude.ai/code/session_011miCFRSatixeNRw3w5yNq8

[ChronicallyJD]

CI status on this PR: one red, and it is not this change

suites (PG 18) is failing. The failure is recluster_extent, filed as #342. Everything else is green, including column_projection, corruption, and cancel_decode -- the three suites this PR actually affects.

I need to correct something I said when I merged #339. I reported that CI as 11/11 green. That reading was true of run 30772271903, but gh pr checks 339 now reports the same job id as this PR's failure, because #339 and #340 share a head commit. So there are two CI runs of the identical tree: 30772271903 passed PG18, 30772737889 failed it. The tree is not deterministically green and I should not have described it as such without qualification.

Why I do not think it is this change, and what would prove me wrong

recluster_extent and the extent marking both predate #338. The recluster path calls ColumnarBeginRead with a NULL projection (columnar_vacuum.c), which by construction keeps the old whole-group read, so this PR should not be able to affect it. The failure surfaced here only because opening this PR triggered a fresh CI run of an already-merged tree.

That is an argument, not a proof, and it is being checked rather than assumed.

What is established so far

• CI: base 1500000 rows, 50000 inserted during the rewrite; sorted 1520000, appended 30000. So 20,000 concurrently-inserted rows were counted as sorted.
• The insert is a single transaction, so a snapshot sees all 50,000 rows or none. Forcing it to commit entirely before the rewrite gives 1550000 / 0 -- all-or-nothing, which is not what CI showed.
• With stripe_row_limit => 20000, a 20000/30000 split is consistent with exactly one of the insert's groups landing at or below the mark, which is the precise failure mode record_online_sorted_extent exists to prevent.
• The suite's premise check is provably too weak: with the insert forced entirely before the rewrite, the premise still passes while the property fails. That is a test defect regardless of what else is true.

So it is currently unresolved whether #342 is a real defect in the extent watermark or a race in my test. I am not guessing, and I would rather this PR sit with a visible red than have me relabel a red as noise.

Reviewing the projection change itself is unaffected -- its own suites are green and its behavior is independent of the recluster path.

[jdatcmd]

Superseded — this review branch's work landed in main as #339 (feat: read only the columns a query references, #338). Closing; not merging.