docs: record the #405 late-materialization plan and its retraction - #601
Conversation
The multi-agent investigation of #405 item 1 (position-level late materialization, Abadi) produced a measurement-gated plan, a Step 1 gate that reported 41-58 percent recoverable, and then a retraction of that gate: the measurement compared count(*) on the batch-fold path against sum(payload) on the row path, two different execution paths. The error was caught by implementing Step 2 and finding its work-done counter read 0. The corrected like-with-like measurement (row path only, selectivity 1 to 99 percent) shows payload cost already scales 4.5x with selectivity, because #452 Phase 1a already defers payload materialization to surviving rows on the row path. The batch-fold path, the only place lacking position-level deferral, is byval-only and never carries payload aggregates, so there is nothing to defer there. Item 1 closes as done-by-#452; ChronicallyJD confirmed the same conclusion independently from the source on the issue. The pre-retraction plan is kept below the retraction for its verified architecture facts (the no-table-AM-change plumbing verdict, the two decode granularities, the #452 1a row-path deferral mechanism), marked superseded, not as a recommendation to implement. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ChronicallyJD
left a comment
There was a problem hiding this comment.
The retraction's central architecture fact is refuted; the doc needs it corrected before it becomes the durable record
The row-path facts in this doc verify cleanly against main — I re-checked every
file:line citation at 744b7ca and re-ran the shapes at 99c6a58. The plumbing
verdict, the #452 1a row-path deferral, the byval-only fold gate, and the
decode-is-sequential kill shot all hold. What does not hold is the retraction's
premise, and it is the TL;DR:
pgcolumnar_native_batch_fold— the only place lacking position-level
late-mat — is taken only for aggregates with no payload column
(count(*)). [...] So there is nothing to defer on the fold path: it never
carries payload.
That is an int8 artifact, not an architecture fact. The classifier maps
sum(int8) to COLUMNAR_AGG_SUM_INT8 (returns numeric) and
pgcolumnar_batch_agg_ok rejects it — alongside SUM_NUMERIC, AVG_INT8,
AVG_NUMERIC, and min/max. But it accepts SUM_INT (int2/int4),
SUM_FLOAT (float4/float8), AVG_INT, AVG_FLOAT, and COUNT_COL — all of
which carry payload through the fold gather. The Step-2 counter read 0 because
the fixture's payload was int8, and "every sum(pN) shape reports Batch Fold: no"
was a true observation whose scope was the fixture's type, not the fold path.
Measured on main @ 99c6a58, pg18a assert build (.so md5 b148f00d31d933909f50a06b436022f7), enable_ungrouped_vector_agg = on, serial
| shape | Columnar Batch Fold |
|---|---|
count(*), count(i4) |
yes |
sum(i2), sum(i4), sum(f4), sum(f8), avg(i4), avg(f8) |
yes — payload-carrying |
sum(i8), sum(numeric), sum(i4),sum(i8) mixed, min/max |
no |
Correctness control: the fold and row arms agree on sum(i4), sum(f8),
count(*) over the same fixture (GUC toggled, same session).
The like-with-like Step-1 measurement the int8 fixture could not run
4M rows, q int4 uniform 1..100 scattered (27/27 chunk groups read, 0 vectors
skipped — asserted), 8 float8 payload columns, PG18 non-assert
(.so md5 fd007021170a1a4d13bece2bae653108), fold pinned in every timed run's own
EXPLAIN, arms interleaved, median of 7. Selectivity premise measured: 40,000 and
3,960,000 of 4,000,000.
| arm | median ms |
|---|---|
fold count(*) @1% |
76 |
fold sum(p1..p8) @1% |
2,998 |
fold sum(p1..p8) @50% |
3,091 |
fold sum(p1..p8) @99% |
3,147 |
fold sum(p1..p8) @1%, constant payload |
419 |
fold count(*) @1%, constant payload |
89 |
row path sum(p1..p8) @1% / @99% |
3,126 / 3,834 |
Survivors rise 99x, fold time rises 5%: payload cost on the fold path is flat
in selectivity, i.e. paid for every non-skipped row before the key check. The
gap the pre-retraction plan targeted exists, on exactly the shapes above.
Decomposed at 1%: payload cost = 2,922 ms, of which ~2,579 ms is whole-vector
decode (your kill shot 1 stands — the reorder cannot recover it) and ~330 ms is
the gather (~10.3 ns per row·col — reproducing your 11.7 ns fetch_att figure on
the valid comparison). So on this worst-case-for-decode payload the Step-2
reorder is worth ~11% of the wide query. On the constant payload, where decode is
near-free, the gather is essentially the whole payload cost: 419 ms → ~92 ms
derived ceiling, ~4.6x on that query shape. (Derived, not measured — the true
number needs the reorder built. And your own comp-vs-rand int8 numbers put int8
decode small, so the recoverable share on int8-shaped payloads likely sits
toward the high end. Not measured here; float8 is the one class I measured.)
What I'd change in the doc
- Replace "taken only for aggregates with no payload column" and "it never
carries payload" with the census: fold accepts count(*)/count(col) and
sum/avg over int2/int4/float4/float8; it rejects sum/avg over int8/numeric,
min/max, and any mixed list containing one ineligible aggregate. State that
the Step-2 counter read 0 because the fixture payload was int8. - Rescope the TL;DR: "no recoverable gap on the row path; on the
(default-off) fold path the gap is real, flat in selectivity, and bounded by
the gather share of payload cost — measured ~11% of the wide query on
incompressible float8 @1%, ceiling ~4.6x on highly compressible payload." - Keep everything else — the plumbing verdict, the byval-only gate (so this
still does nothing for q24), kill shots 1/3/4/5, and the cost-model
requirement, which this measurement re-confirms: recoverable ≈ gather x
(1 − selectivity), so at high survival there is nothing to win.
Two scope notes for prioritization, not objections: enable_ungrouped_vector_agg
defaults to off, so no default configuration reaches this gap; and whether
the corrected numbers justify rebuilding Step 2 is a scheduling call I am not
making here — the doc just must not record "there is no gap" when the gap is
measured.
Requesting changes because this PR's entire content is the record, and the
record's headline is the one refuted sentence. Happy to re-review the same day
it's updated; full scripts and raw outputs are on the bench under
/home/jd/i405/ if you want to re-run any arm.
Re-review: the doc is unchanged, so the one blocking edit still stands — not addressed yet, and I think it just hasn't reached the top of your queue behind M2/M3.Head is still Fresh proof on current main, now that #602 has landedThe reason this is worth pinning: post-#602, the A payload column, summed, and the fold ran. So "the fold path never carries The edit still owed (unchanged from my first review)
Happy to hand you ready-to-paste replacement paragraphs, or open a tiny PR |
The PR #601 review refuted the retraction's central sentence: "the fold path never carries payload" was an int8 artifact. The fold classifier rejects sum/avg over int8 (numeric transition type) and numeric, which is why the Step-2 work-done counter read 0 on an int8-payload fixture and why "every sum(pN) shape reports Batch Fold: no" was true only of that fixture. The classifier accepts sum/avg over int2, int4, float4 and float8, all payload-carrying. Verified in code and empirically on two independent lanes before amending (reviewer on PG18 assert; author on PG17 non-assert via the post-#602 ANALYZE line). The doc now carries both corrections legibly: no recoverable gap on the ROW path (#452 Phase 1a defers there, unchanged), and on the default-off fold path the gap the pre-retraction plan targeted is real, flat in selectivity, decomposed by the reviewer's measurement into whole-vector decode (unrecoverable, kill shot 1 stands) and the gather (~11% of a wide float8 query at 1% selectivity, ~4.6x derived ceiling on compressible payload). Whether to rebuild Step 2 is recorded as a scheduling call on the issue, bounded by the GUC defaulting off and by the mandatory cost model. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Amended in the pushed revision. Before adopting your census I re-verified it independently: The doc now carries both corrections legibly, each caught by the discipline the other skipped: the original gate fell to a work-done counter, and the retraction built on that counter fell to your census — the counter read 0 because the fixture was int8-shaped, a true observation scoped to the fixture, not the path. Your three asks are in as written: the census replaces the refuted sentence with the int8-artifact explanation; the TL;DR is rescoped to "no gap on the ROW path; the fold-path gap is real, flat in selectivity, ~11% measured on incompressible float8 @1%, ~4.6x derived ceiling"; and the plumbing verdict, byval gate (q24 unaffected), surviving kill shots, and the now re-confirmed cost-model requirement all stay. The rebuild-Step-2 scheduling call is recorded as open on the issue rather than decided in the doc, with your two bounding facts beside it (the GUC defaults off; recoverable ≈ gather × (1 − selectivity)). Ready for re-review. |
ChronicallyJD
left a comment
There was a problem hiding this comment.
Re-review: all three asks addressed, census matches the measurements. Approve.
Verified 5f23188 against the tree:
- The refuted section is gone: "Why the batch-fold path cannot help" / "it never
carries payload" is replaced by The fold-path census, which states what
batch_agg_okactually does — acceptscount(*)/count(col)andsum/avg
over int2/int4/float4/float8, rejects int8/numeric, min/max, and mixed lists —
and names the int8 fixture as why the Step-2 counter read 0. That census
matches what I measured (PG18 assert) and what you re-ran (PG17 non-assert):
sum(int4)/sum(float8)/avg(int4)fold,sum(int8)/maxdo not, on the
post-#602 line that reports the fold that RAN. - The TL;DR is rescoped to "row path done-by-#452; fold-path gap is real,
default-off, bounded by the gather share, cost-model mandatory, byval-only so
q24 is untouched" — and keeps both corrections legible, which is the right call
for a doc whose value is the record of how a twice-reversed conclusion settled. - Everything I asked to keep is kept: plumbing verdict, kill shots, the row-path
deferral facts.
Keeping the pre-retraction plan below as the starting point for Step 2/4 if the
scheduling call is ever taken is the right disposition. Nothing further from me.
Approving.
Records the final disposition of the three ranked items in #405, re-measured against current main so the issue can be closed: - Item 1 (position-list intersection): MEASURED again on main 977bb44. Two queries returning the same 2,000 surviving rows show 25x the payload-decode cost (28.9 ms vs 726.7 ms) when survivors are spread one-per-vector, with 1,998,000 rows removed by filter -- decode tracks surviving VECTORS not positions. We prune vectors, not positions; Abadi's full bit-string intersection stays retracted (a trade, not a free win, PR #601); the worthwhile subset shipped as the fold-path payload deferral (#617). - Item 2 (branch predication): N/A -- the tree builds no packed selection vector to predicate (the selection loops were deleted in #200). Recorded. - Item 3 (vector length): reframed. COLUMNAR_NATIVE_VECTOR_LENGTH is vestigial (write-only catalog stamp); the execution stride is a literal and the runtime knob is chunk_group_row_limit. Not a single tunable constant. Recorded. Docs only (design note); no code, no behaviour change. Recommends CLOSE. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TgAk1gqeME7DHpJw8xxybu
Adds
design/ISSUE_405_LATE_MATERIALIZATION.md, the plan-before-code document for #405 item 1 that the issue comments reference by path.The doc records, in order:
count(*)on the batch-fold path againstsum(payload)on the row path, two different execution paths. The error was caught by implementing Step 2 and finding its work-done counter read 0.The corrected like-with-like measurement (row path only, selectivity 1-99%) shows payload cost already scales 4.5x with selectivity because #452 Phase 1a defers payload materialization to surviving rows on the row path. The batch-fold path, the only place lacking position-level deferral, is byval-only and never carries payload aggregates, so there is nothing to defer there. Item 1 closes as done-by-#452; @ChronicallyJD confirmed the same conclusion independently from the source on the issue.
The pre-retraction plan is kept below the retraction, marked superseded, for its verified architecture facts: the no-table-AM-change plumbing verdict, the two decode granularities, and the #452 1a row-path deferral mechanism.
Docs only, no code change.
Closes nothing on its own; the #405 disposition is being settled on the issue.
🤖 Generated with Claude Code