Size the index-fetch decode from the attribute prefix (#363) - #368
Conversation
…ompt#363) Holds row count, emitted width and plan shape fixed and varies only which column is referenced. a1's prefix decodes ~15 MB per group and stays under the fetch cache cap; a10's decodes ~156 MB and does not, so the early column should keep its index and the late one should not. A build that sizes the decode from rel->reltarget->width cannot tell the two apart -- it is identical for both -- so this fails on main. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ompt#363) The cost model sized the per-fetch decode from the columns the scan emits. The deferred index-fetch slot decodes the attribute prefix 0..max-referenced, because slot_getsomeattrs asks for a prefix and cannot ask for a set, so a query referencing a late column decodes every column before it. Both of the model's inputs understated it: decoded_width came from rel->reltarget->width, and nproj was the count of referenced columns rather than the length of the prefix. Measured on a ten-text-column table, same 300 fetched rows, same emitted width, same plan: max(a1) 975 ms against max(a10) 194,798 ms, the two sitting on opposite sides of the 32 MB fetch cache cap while the model computed the same decoded_width for both. columnar_scan_decode_shape returns the prefix length and its summed width, taking widths from pg_statistic where ANALYZE has run and the type average otherwise, as set_rel_width does -- the unreferenced columns in the prefix are not in reltarget at all. A whole-row reference widens the prefix to the whole tuple; a dropped column holds its place and decodes nothing. This matters more after commandprompt#365 than it did before: the penalty could not change a plan then, and now it can. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Gate: full matrix on PG18 + PG19
The one red is the pre-existing wide-table ANALYZE timing ratio (3,557 ms against a Every plan-shape check passes on both majors, including the ones that guard the The early-column check is the one that matters for over-firing: widening the model's |
jdatcmd
left a comment
There was a problem hiding this comment.
Approved. Premise verified in code, matrix run here.
The load-bearing claim is correct and I checked it rather than taking it.
columnar_slot_decode_upto builds needed as {0 .. natts-1} and its own comment
says "Decodes a prefix because that is what slot_getsomeattrs asks for". So sizing
the model from reltarget->width really was measuring the wrong thing, and the two
queries in your test really are indistinguishable to the old code.
Details I specifically looked for and found handled:
- Dropped columns occupy a place in the prefix and decode nothing (
continueon
!OidIsValid(typid)). - Whole-row references take the prefix to
rel->max_attr. - Widths come from
pg_statisticwith a type-average fallback, which is
necessary rather than incidental: the unreferenced columns in the prefix are not
inreltargetat all, so there is nowhere else to get them. nprojbecoming the prefix count rather than the referenced count is right for
the same reason, sincedecode_per_groupcharges per decoded column.
Matrix, run here
fix/363-prefix-width at cd0e140, full suite matrix:
PASS PG18 (112 suites)
FAIL PG19 native_cancel only
That red is not yours. It is cancel=85ms full=158ms against the old
cancel < full / 2 threshold, and your branch predates #373, which replaced
that threshold on main this evening because it has only 27 ms of headroom above a
50 ms floor and was failing PG17 two runs in three. Under #373's window the limit on
that run is 104 ms, so 85 ms passes. Merging onto current main resolves it; nothing
to change here.
One correction to the PR body
You listed the wide-table analyze_stats ANALYZE timing ratio as an expected red.
It passed on both majors here, and it also passed on all five majors in the full
15-19 matrix I ran at 488a2c0. So that red is specific to your container rather
than pre-existing in the code. Worth knowing before it gets treated as a known-bad
and discounted somewhere it matters.
On the result itself
975 ms against 194,798 ms on the same emitted width is the kind of thing a cost
model has no business being blind to, and after #362 the penalty is load-bearing for
plan choice rather than decorative, so this closes the gap I flagged on #367 as my
one reservation about not holding alpha for #363. Good that it is fixed rather than
documented.
Merging.
What
Closes #363. The index-fetch cost model sized the per-fetch decode from the columns
the scan emits. The deferred index-fetch slot decodes the attribute prefix
0..max-referenced—slot_getsomeattrsasks for a prefix and cannot ask for a set— so a query referencing a late column decodes every column before it.
Both of the model's inputs understated it:
decoded_widthcame fromrel->reltarget->width, which gates the cap-crossingbranch;
nprojwas the count of referenced columns rather than the length of the prefix.Measured on a ten-
text-column table, same 300 fetched rows, same emitted width,same plan, varying only which column is referenced:
200x, and
rel->reltarget->widthis identical for both — the two queries sit onopposite sides of the 32 MB fetch cache cap while the model computed the same
decoded_widthfor each.columnar_scan_decode_shape()returns the prefix length and its summed width.Why it matters more now than when it was filed
Before #365 the penalty could not change a plan at all, so a wrong input to it was
inert. It is load-bearing for plan choice now, which is jdatcmd's argument on #367 for
not deferring this past alpha, and I agree with it — I had it as post-alpha and was
wrong.
Design decisions worth a reviewer's eye
pg_statistic, falling back to the type average, the wayset_rel_widthdoes. The unreferenced columns in the prefix are not inreltargetat all, so there is nothing else to read them from.
pull_varattnosreports it as attribute 0; treating that as "no columns" would understate by the
entire table.
length, contributes no width.
Tests
test/analyze_stats.shgains a case that holds row count, emitted width and planshape fixed and varies only which column is referenced —
a1versusa10on aneleven-column table.
a1's prefix decodes ~15 MB per group and stays under the cap;a10's decodes ~156 MB and does not.Proven by removal. Same test file, PG18 assert:
max(a1)— must keep the indexmax(a10)— must lose it702d125Index Scan using o363_idIndex Scan using o363_idIndex Scan using o363_idCustom Scan (ColumnarScan)and every existing #355 and #362 check passes on both builds, so the change does not
over-fire into the cases #171/#159 protect: the clustered
ORDER BY, the selectivepoint lookup, and the early-column query all keep their indexes.
Gate
Five-major build: BUILD_OK, 0 warnings on PG15/16/17/18/19. Full matrix on
PG18 + PG19 running; I will post it as a comment.
The one expected red is the pre-existing wide-table ANALYZE timing ratio in
analyze_stats, which fails identically on clean main and is unrelated to theplanner (see #359 — ANALYZE never enters
columnar_fetch_row).🤖 Generated with Claude Code