docs: record #890's measurement in the roadmap, not the claim - #1080
Conversation
|
The conclusion is right and the narrow reading is exactly what I would want preserved. But the table has a provenance problem, and for a document whose whole purpose is to be the durable record that is the thing to fix before it lands. Not approving yet on that basis alone. Everything else here I would take as written. The
|
`design/ROADMAP.md:192` read as an outstanding task sourced to Zeng et al., VLDB 2024: block compression costs more CPU than it saves in I/O on fast local NVMe, so make it opt-in per storage tier. It was measured on this engine (#890) and did not reproduce. Left as written it would have regenerated the same issue against a different corpus in a year, which is the failure this amendment exists to prevent. jd ruled it: change nothing about the default, amend the roadmap. shape S = 1 - zstd/none C W verdict repetitive 0.750 1.001 0.998 NET WIN text_heavy 0.958 0.911 0.920 NET WIN realistic 0.127 1.001 0.996 UNDECIDED random_int 0.000 1.000 1.012 UNDECIDED incompressible -0.015 1.010 0.913 UNDECIDED No shape fires NET COST, and there is no storage tier to key a default off: a native table's blocks always live in the data directory, and object storage is an import/export surface rather than a location for native table storage. THE FIRST VERSION OF THIS ENTRY MIXED TWO RUNS IN ONE COLUMN. Found in review by @OffgridwithJD. The summary table it was transcribed from labelled two of its read costs `0.92 (rep)` and `0.92 (mix)` -- shape names from an EARLIER three-shape run, with a different row count and wall-clock rather than instruction counts. Transcribing it dropped the parentheses, which were the only marker that those two values came from somewhere else. Reconciled row by row against the five-shape run, three of five read costs were wrong: shape C as written C in the five-shape run repetitive 0.92 1.001 text_heavy -- 0.911 realistic 0.92 1.001 random_int 1.00 1.000 matched incompressible 1.01 1.010 matched Every `S` was correct and NO VERDICT CHANGES: the two NET WINs are decided on `S` alone, and the three UNDECIDED rows stay UNDECIDED because their `C` was already under the 1.10 threshold. The conclusion never depended on the wrong numbers, which is precisely why nothing would have caught them. `text_heavy`'s blank was the costly one. Its real `C` of 0.911 is the most favourable read cost in the set, so omitting it UNDERSTATED the case. THE CONDITIONS NOW TRAVEL WITH THE NUMBERS: rows, build, counter, and that the codec was asserted from `column_chunk.block_codec` rather than assumed from the GUC. Plus the limit that an instruction count cannot see an I/O saving at ANY working-set size, so `C` near 1.0 means "costs nothing to read" and never "saves nothing". #890 learned the header the hard way when a table went out from an assert build without naming it; the roadmap is where someone goes to reopen this, so it belongs there more than on the issue. THE NARROW READING IS RECORDED RATHER THAN THE HEADLINE. The shape the rule names as the DECIDER is `realistic`, and it comes out UNDECIDED, not NET WIN. The outcome is identical because an undecided measurement does not move a default, but "the premise is refuted" is true only of the two most compressible shapes. Two things that would otherwise have been lost with the entry. Dictionary encoding on float columns was a SEPARATE clause of the same bullet and nothing here measures it, so it is called out as still open. And the alpha4 item is marked as having cost nothing in written bytes, so it need not have been alpha work at all -- it was scheduled as alpha because the plan assumed it would change the writer, and that assumption was the part that needed checking first. Three defects found by the investigation are tracked separately and do NOT close with this: #1074, #1075, #1076. Docs only. `design/` is outside `docs_style.sh`'s scope; the CHANGELOG entry carries no em or en dash, which is the one rule that does police it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NhwXKAgSmYDUjteWkfajHK
e5963d9 to
73c9d65
Compare
|
You were right, and it was worse than the reconstruction you could see. I verified it against The mechanism, which is the part worth keepingThe summary table I transcribed labelled those two So the defect is not "I copied a wrong number", it is "I deleted the label that No verdict changes, which is why nothing caught it
And your point about the blank stands: Your second point, taken in fullThe entry now carries rows, build, counter, and that the codec was asserted from I also added the Same correction posted on #890The closing comment there carried the identical table, and a closed issue is where |
OffgridwithJD
left a comment
There was a problem hiding this comment.
APPROVE at 73c9d65. 14/14 green. I re-derived the corrected table rather than taking the fix on trust.
The five C values now match the five-shape run
shape C as written five-shape run
repetitive 1.001 1.001 ✓
text_heavy 0.911 0.911 ✓ (was blank)
realistic 1.001 1.001 ✓
random_int 1.000 1.000 ✓
incompressible 1.010 1.010 ✓
And "all five values from the SAME run" is now stated in the text, which is what stops the next transcription doing this again.
The narrative reconciles against its own columns
Checked rather than read past, because a table and its prose drifting apart is the same class of defect one level up:
- "
text_heavyis the only shape where compression measurably pays on the READ side" —Ccolumn is1.001, 0.911, 1.001, 1.000, 1.010.0.911is the only value below 1. Holds. - "on
incompressible, zstd is measurably cheaper to write (W = 0.913)" — matches theWcolumn, and the claim is not one of uniqueness, sotext_heavy's0.920does not contradict it. - "no shape fires NET COST" — the rule needs
W >= 1.25orC >= 1.10withS < 0.10. The largestWis1.012and the largestCis1.010. Holds with a wide margin.
The mechanism you found is the part worth keeping
The parentheses were the provenance marker and I deleted them in transcription.
That is a better account than "a wrong number was copied", and it generalises: 0.92 (rep) and 0.92 (mix) were a table telling its reader that column was mixed, and flattening it removed the only signal. A transcription that drops an annotation drops the thing that made the annotation necessary.
And no verdict moved, which is why nothing caught it. The two NET WINs are decided on S alone and the three UNDECIDEDs stay undecided at the real C. A wrong value that leaves the conclusion intact is invisible to anyone reading for the argument — which is everyone, including me on the first pass. I only found it because the blank in an otherwise complete column is a structural tell rather than a numerical one.
Your point back at me is the one I would not have reached: omitting a value that helps you is a failure direction nobody looks for. text_heavy's 0.911 was the strongest number against the premise and it was the one left out.
The conditions block
Everything I asked for and one thing I did not: the codec asserted from column_chunk.block_codec rather than the GUC, and PgColumnarScan asserted on every arm. Those are the two ways this measurement could have been of something else entirely.
The instruction-count limit landed in the right form — "C near 1.0 means costs nothing to read, never saves nothing" — and it belongs here rather than in the issue, because the ROADMAP is where someone goes to reopen this.
Correcting #890's closing comment too
Right call, and the reason is the sharper half: a closed issue is where a wrong table does the most damage, because nobody re-reads it and it is cited as settled. Checking that #1082's docs do not carry the same table, rather than assuming, is the check I would have asked for.
Land this one first. A closed #890 pointing at an unamended roadmap is the state the amendment exists to prevent.
…ntees TWO FIXES, one of them a hole in this PR that review did not look for. THE CHANGELOG ENTRY WAS NEVER COMMITTED. `f115d0b` contains only `test/native_fetch_projection.sh`. The entry was written, sat unstaged, and was lost: `git reset --soft` leaves the index alone, `git commit` commits the index, and the working-tree edit never entered it. I checked with `git diff --stat origin/main` AFTER committing, which compares the WORKING TREE to main rather than the commit to main, so it showed both files and read as confirmation. That check cannot see this class of mistake at all. The one that can is `git show <sha> --name-only`, which reads the commit. Checked the sibling branches by the same means: #1082 and #1083 both carry their CHANGELOG entries. Only this one was affected. AND THE THIRD PREMISE'S COMMENT OVERSTATED ITS JOB. It read "the whole arm rests on it". It does not. A broken prefix relationship cannot pass silently, because the other two arms already contradict each other under it: if `Cols(` matched the wide pattern then every narrow call would be counted twice, so `full >= cols`, and `cols >= 1` with `full == 0` is a contradiction. cols=1, prefix intact -> full=0 arm PASS cols=1, prefix broken -> full=1 arm RED cols=3, prefix broken -> full=3 arm RED The premise documents the assumption and names what a future rename would break. It is not the guarantee. Kept, with the comment now saying which it is. Correction from @OffgridwithJD's review. NO CHECK NAME MOVES, so no ledger key moves: the 17 names are identical to `f115d0b` by sorted diff. Suite re-run on PG17 non-assert after the change, 17 passed + 0 failed + 0 unrunnable + 0 skipped = 17. Merged `origin/main` to pick up #1080, and verified the roadmap amendment survives in the merged tree rather than assuming it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NhwXKAgSmYDUjteWkfajHK
The only conflict is CHANGELOG.md, and both sides add entries to `### Fixed`, so both are kept. #1080, #1081 and #1083 landed while this was open. Verified by count rather than by reading the diff: markers left 0 entries present exactly once #1074/#1076, #1075, #1077, #1080, #1081 bodiless headings in [Unreleased] 0 Nothing else moved. Per-file patch md5 of my seven files, merged result against the pre-merge branch, added and removed lines only: docs/administration.md 6a456b0edcac same docs/best-practices.md 0c84c03e77bd same docs/configuration.md 68b2705ebe8a same test/fsst_margin.sh 978e4e448429 same test/pytest/TESTS.md 787346d68405 same test/pytest/expected_tests.txt d1807c9dfd54 same test/pytest/test_compression_reaches_the_cascade.py 025da1ba7426 same main moved none of `expected_tests.txt`, `check_ledger.tsv` or `check_ledger_budget.txt` -- checked by md5 against 8e88f42 rather than assumed from the fact that the merges were docs, shell suites and one `src/` file. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NhwXKAgSmYDUjteWkfajHK
CHANGELOG.md only. #1081 and #1083 landed while this was open and both add to `### Fixed`, as this does, so all three entries are kept. Verified by count rather than by reading the diff: conflict markers left 0 each entry present exactly once #1077 sweep, #1075, #1080, #1081, and #1078's, which was already there bodiless headings in [Unreleased] 0 The suite file is untouched by the merge: its patch md5 against main is unchanged from before it, and the 17 check names are identical by sorted diff, so no ledger key moves. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NhwXKAgSmYDUjteWkfajHK
design/ROADMAP.md:192read as an outstanding task sourced to Zeng et al., VLDB2024. It was measured on this engine (#890) and did not reproduce. Left as
written it would have regenerated the same issue against a different corpus in a
year, which is the whole reason to amend rather than just close.
jd ruled on this directly: change nothing about the default, amend the roadmap.
What both files now carry
No shape fires NET COST. And there is no storage tier to key a default off: a
native table's blocks always live in the data directory; object storage is an
import/export surface.
The narrow reading, not the headline
The shape the pre-registered rule names as the decider is
realistic, and itcomes out UNDECIDED, not NET WIN. The outcome is identical because an
undecided measurement does not move a default — but "the premise is refuted" is
true only of the two most compressible shapes. Someone reopening this needs to
know which shapes carried the conclusion. @OffgridwithJD's point, and it is the
part I would most want preserved.
Two things that would have been lost with the entry
Dictionary encoding on float columns was a separate clause of the same bullet.
Nothing in #890 measures it. It is called out as still open rather than struck
through with the rest.
The alpha4 item cost nothing in written bytes, so it need not have been alpha
work at all. It was scheduled as alpha because the plan assumed it would change
the writer, and that assumption was the part that needed checking first. Worth
recording because it is a scheduling lesson, not a compression one.
Also recorded
On
incompressible,zstdis measurably cheaper to write thannone(W = 0.92) — the FSST keep test only runs when a codec is configured, and dropping
FSST saves more than zstd costs. A property of our cascade, not of zstd, and the
opposite of the naive expectation.
Scope
Docs only.
design/is outsidedocs_style.sh's scope; the one rule that doespolice the CHANGELOG is the em/en dash ban, and this diff adds none (verified: 0).
No test reads either design file.
Three defects found by the investigation are tracked separately and do not
close with #890: #1074, #1075, #1076 — all three are next.
🤖 Generated with Claude Code
https://claude.ai/code/session_01NhwXKAgSmYDUjteWkfajHK