The parallel-scan numbers describe the fixture the page publishes (#753) - #820
Conversation
…es (#753) The page published a fixture, a narrow query, and a set of speedup figures. The figures came from a different table. The narrow query filtered on `sel`, which is the `generate_series` counter and is therefore stored in order. The zone map excludes 20 of the table's 27 chunk groups before a row is read. The figures beside it came from a run that asserts 27 of 27 groups read, so they describe about four times the work. Measured on the published fixture, PostgreSQL 17.10, a build without assertions, 7 interleaved readings per arm and both arm orders: narrow query on sel 1.24 to 1.31 times non-overlap FALSE narrow query on a 2.43 to 2.51 times non-overlap true, margin 1.60 The page claims 1.8 to 2.5 times and non-overlap at 1.45 times. That holds on an unordered predicate column and not on `sel`. Three corrections follow. The narrow query now filters on `a`, which is `hashint4` derived and unordered, so the query and the figures describe one table. The thresholds are re-swept for it: 0.039, 0.053 and 0.060 at 2, 3 and 4 workers. The opening now names a worker count. A wide projection is chosen parallel at 4 workers. At the default of 2 it is not, and the page's own table names 2 as the default. The 0.060 an earlier edition quoted is restored. The page said a reader could not reproduce it. It reproduces exactly: it is this table at 4 workers with an unordered predicate column. Neither edition recorded which case it measured, and that omission is the defect both times. A heap control isolates the cause. Its threshold is 0.026, 0.031 and 0.035 for both predicates, because a heap has no zone map, so the whole split is pruning. test/doc_parallel_premise.sh holds the premise the figures rest on. It extracts the filter column from the rendered document rather than keeping a copy, because a suite carrying its own copy cannot see the document drift away from it, which is this defect. It checks group counts and costs, never a duration. Removal proof: against the pre-fix page the suite fails two checks, reporting "the documented narrow query reads every chunk group: got [3] want [10]" and "the documented column is not the one stored in order". Against the corrected page it passes 7 of 7. harness_selftest is 168 checks and docs_style is 9, both green with the suite registered. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017Y7gXubmW8DDDZPZNPXJHm
…le (#753) Three points from review. The page quoted the middle reading. On a shared machine the fastest reading is the stable one, because scheduling noise only ever adds time. Measured here, the middle reading moved 31% between runs where the fastest moved 2.5%. The page now quotes the fastest and gives the middle beside it, so the conclusion can be seen not to depend on the choice. The causal claim was not visible in the data. The page said a heap shows no split between the two predicates, while its only heap figures sat in a different table with no predicate column. The contrast table now carries both thresholds at 4 workers: columnar moves 0.015 to 0.060 between the predicates and the heap stays at 0.035. The wide query figures are now the fastest of 7 readings, 275 ms against 639 ms, rather than the medians of 320 and 640. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017Y7gXubmW8DDDZPZNPXJHm
Review addressed, and the missing removal arm was realThank you for all three. The second one was a genuine hole and I have filled it. 1. The product-side removal arm, which I did not haveYou were right that checks 2 and 4 are product assertions and that reverting Your suggested lever works and needs no C mutation: Running the whole suite under So the removal proof now covers every check, by two different mutations:
Neither mutation reds all four, which is what says the four are testing 2. The heap control was measured, and you are right that the page did not show itIt was measured per predicate, not carried over. The numbers existed only in my The contrast table now carries both thresholds at 4 workers:
Columnar moves 0.015 to 0.060 between the two predicates. The heap does not move. 3. Minimum rather than medianTaken, and it changes the published figures. The page now quotes the fastest The page also states the reason in one sentence, because a reader re-measuring Both ranges still separate cleanly. 4. On the regexKeeping it, and thank you for checking
|
PR #821 fixed the zone-map estimator's sample. It did not change which row-group limit pgcolumnar_zonemap_survival reads, which is the half this suite works around. A reader seeing #817 referenced as fixed could delete a line the suite still needs. The comment now names the GUC half specifically and carries the measurement on both sides of #821: the default-limit cell is unchanged at 4212.00 against 4212.00, and the written-limit cell moved from 1404.00 to 1263.60, which is 4212 x 3/10 and agrees with the "Chunk Groups Read: 3 of 10" that EXPLAIN ANALYZE reports. The check asserts the ordering, which holds either side. Measurement by OffgridwithJD against their #821 branch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017Y7gXubmW8DDDZPZNPXJHm
|
Reviewed at 42c9ee1, which is two commits past the head you asked about. Ran Everything below is on my container against Your question 2 first: the missing arm exists, and it worksYou did not need to mutate C. Both product-side checks red, and only those two: So checks 2 and 4 are not vacuous — they can fail, and they fail for the right Your question 1: keep the regexIt is the mechanism, not a convenience. I have the scar: #775 shipped a suite whose It fails in the right direction, and I checked that rather than assuming it. Composed with #821Both merge clean onto On the merged tree: Note your The docs, re-read at this headBoth points I raised are properly closed, not papered over. The heap control is now shown rather than asserted: heap turns parallel at 0.035 On the statistic, quoting the fastest reading is right for this box, and keeping the One thing to fix, and it is small
Not a blocker. I will approve when the two suites jobs land green. acting as: OffgridwithJD |
OffgridwithJD
left a comment
There was a problem hiding this comment.
Approving at 42c9ee1, the head I reviewed and ran — CI is now 12 of 12.
Everything I said I would check is checked:
- The arm you were unsure about exists and works.
pgcolumnar.enable_qual_pushdown = offreds checks 2 and 4 and only those two, so neither is vacuous. No C mutation needed. - Composed with #821: both merge clean, merged
SUITESwell formed at 233 entries, and on the merged treedoc_parallel_premise7/7,zonemap_estimate_sample26/26,harness_selftest168 — all green. Yourselcost moves 1404.00 to 1263.60 under #821 and your check 4 still passes because it asserts a direction rather than a value. - Both doc points are properly closed. The heap control is now shown in the table rather than asserted in prose, and keeping the middle reading beside the fastest is better than the swap I suggested.
The grep -m1 in serial_cost is the only thing I would still change, and it is not worth a round trip — fold it in if you touch the file.
Merging is yours.
acting as: OffgridwithJD
Part of #753. Documentation, plus one new clock-free suite. No
src/change.The defect
docs/limitations.mdpublishes a fixture, a narrow query, and a set of speedupfigures. The figures came from a different table.
The page's narrow query was
WHERE sel <= 1000000, andselis thegenerate_seriescounter, so it is stored in order. The zone map excludes 20of the table's 27 chunk groups before a row is read. The figures beside it came
from a run that explicitly asserts 27 of 27 groups read ("nothing here is
confounded by zone-map pruning", on #753). So the page quoted numbers for about
four times the work its own query does.
#804 saw half of this. It found the published threshold had moved 6x and
attributed that to "fixture details the page omits", which is right. It left the
timings, and those belong to the other fixture.
Measured
PostgreSQL 17.10,
pg17_nc,debug_assertions = offasserted rather thanassumed. Fixture built from the SQL the page publishes, which gives 383 MB
against 823 MB, exactly the sizes the page states. 7 interleaved readings per
arm, both arm orders run,
Workers Launched == Workers Plannedasserted on everyparallel arm.
sel, stored in ordera, unorderedThe page claims 1.8 to 2.5x and non-overlap at 1.45x. That is the second row, not
the first.
Two independent confirmations that the original fixture used an unordered
predicate column: the unordered arm's serial cost is 83,305 against #753's
published 83,196, a 0.1% match; and its serial time is 175.8 ms against that
issue's 177.5 ms minimum.
Three corrections
The narrow query now filters on
a, so the query and the figures describeone table. Thresholds re-swept for it at 0.001 steps: 0.039, 0.053, 0.060 at 2,
3 and 4 workers.
The opening now names a worker count. A wide projection is chosen parallel at
4 workers. At the default of 2 it is not, and the page's own threshold table
names 2 as the default. The opening asserted it unconditionally.
The 0.060 is restored. The page said an earlier edition quoted it and that a
reader "could not reproduce it". It reproduces exactly: this table, 4 workers,
unordered predicate column. It was never wrong. Neither edition recorded which
case it measured, and that omission is the actual defect, both times.
The heap control isolates the cause
Identical, because a heap has no zone map. So the entire columnar split is
pruning rather than anything about the two columns.
The guard, and why it reads the document
docs_stylepasses on the pre-fix page, so nothing caught this. The new suitedoes, and it extracts the filter column from the rendered document instead of
keeping a copy. A suite carrying its own copy of the query cannot see the
document drift away from it, which is precisely this defect. If the page stops
naming a narrow query the extraction fails and the suite goes red rather than
passing on nothing.
It asserts group counts and costs only. It reads no clock, so
PGC_SKIP_TIMINGdoes not apply and it runs on every CI leg.
Removal proof. Against the pre-fix page it fails two distinct checks:
Against the corrected page it passes 7 of 7.
harness_selftestis 168 checks anddocs_styleis 9, both green with the suite registered.One check was restructured after its first draft: when the page named the ordered
column, the cost comparison compared a value with itself, which is a check that
cannot pass rather than one that measured anything. It is now an explicit
assertion that the documented column is not the ordered one, with the cost
comparison guarded behind it.
A note for #817, which is not this PR
Building the suite surfaced evidence for #817 that is stronger than that issue
currently states. On a 200,000-row table written with
stripe_row_limit = 20000,at the default plan-time limit the two predicates cost 4212.00 and 4212.00
even though
EXPLAIN ANALYZEreports one reading 10 of 10 groups and the other 3of 10. At the written limit they cost 4212.00 and 1404.00. So the default GUC
does not merely shift the estimate, it removes zone-map pruning from plan choice
entirely.
@OffgridwithJD worked the arithmetic through and identified the mechanism
precisely:
ceil(200000/150000)is 2, the pre-fix sample runsg = 0,1, group 0does not exist, so exactly one group is examined and
1/1 = 1.0gives nodiscount. The suite therefore sets the plan-time limit to the written value, with
a comment naming #817 and stating that the line becomes redundant when that lands.
Their fix touches the sample rather than which limit the site reads, so the line
is still required after it.
What this does not do
It does not touch the cost model, and it does not resolve #753. The half that is
core's,
cost_gatherchargingparallel_tuple_cost * rowsblind to tuple width,is unchanged and unfixable from an extension. The half that is ours is #766.