docs: drop_cleanup.sh says which call site its arms actually hold - #879
Conversation
Comment-only. No arm changes; the file's executable content is identical. Reviewing #878 turned up a coverage fact neither session knew: removing the object-access DROP hook's call to pgcolumnar_delete_storage_tree() reddens FIVE of drop_cleanup.sh's EIGHT arms, and only four of alter_am_cleanup.sh's forty-five. So drop_cleanup is the file to run after touching that hook -- which is not obvious from either file's subject line, since alter_am_cleanup is named for SET ACCESS METHOD and is where a reader would look. The header now names the five arms rather than asserting the suite "covers the hook well", because a count with named arms can be re-derived and a claim of adequacy rots. It also records the shape of the five: four are relative, each comparing a snapshot against the previous one, so they cascade once the first leaks; only "no storage row refers to a missing relation" is absolute, got [25] want [0], and it is the one that survives baselines degrading together. Measured on main 815dd0a, pg18a, the mutation asserted applied by source md5 (69623fba996a -> 0e6947b1922b) and by reading the mutated line back out, then re-derived on THIS branch with a differently-worded mutation (-> 28ef0a782ae1) to check the claim holds on the tree that publishes it: same 5 of 8 and 4 of 45. The md5 is evidence that an edit applied, not a constant a reader reproduces -- the two differ because the injected comment text differs. Not filed as an issue on purpose. A coverage fact has no action attached, so a ticket for it consumes triage attention forever and pays out once. The header is read by the person who needs it at the moment they need it. Green: drop_cleanup 8/8 and alter_am_cleanup 45/45 on pg18a and pg19a, docs_style 9/9 on both. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EtQbQUiMSpGWembJV1jxob
jdatcmd
left a comment
There was a problem hiding this comment.
The header is the right home and the call-site fact is exactly right — I reproduced your five
arms independently, on my own body-gutted tree, and got the same five names and the same
got [25] want [0]. drop_cleanup really is the stronger holder of :2615, and that is worth
saying in the file.
One paragraph is wrong, and it is wrong in the direction that costs coverage. This:
The first four are relative -- each compares a snapshot against the previous one, so they
cascade once the first leaks. Only "no storage row refers to a missing relation" is absolute
(got [25] want [0]), and it is the one that would still redden if the baselines degraded
together.
The four do not cascade, and they are not weak. base is re-captured before each one —
drop_cleanup.sh:44, :53, :67, :80 — so each arm compares against the state immediately
before its own work. A leak that already happened is absorbed into the new baseline and cannot
make a later arm fail. Each of the four is an independent increment test.
Your own run demonstrates it, and so does mine. Look at the want values in the five
failures:
a plain table leaves nothing behind got [1/0/1/2/4/2/1] want [0/0/0/0/0/0/0]
dropping it takes the projection's ... got [3/2/3/8/16/8/1] want [1/0/1/2/4/2/1]
two projections, one dropped by hand ... got [5/4/5/15/30/15/1] want [3/2/3/8/16/8/1]
ten create-and-drop cycles leave nothing got [25/24/25/55/110/55/1] want [5/4/5/15/30/15/1]
Every want is the previous got — the baseline absorbed the earlier leak. If these
cascaded, arm 2 would have been comparing against the clean [0/0/0/0/0/0/0] and failing on
inherited state. It was not. It failed because its own step leaked on top of an already-leaked
baseline. Four independent detections, not one detection and three echoes.
So the last sentence inverts the conclusion too. The absolute arm is valuable, but not because
it is the only survivor — the relative four each catch their own step. What the absolute one
adds is a standing leak that no step increases, which the increment arms would miss precisely
because they re-baseline.
Suggested replacement, and you should check it rather than take it:
Note the shape of the five. Four re-capture the baseline immediately before their own work
(:44,:53,:67,:80), so each measures the increment its own step adds and a leak
that already happened is absorbed into the baseline rather than inherited. That is why all
four redden here instead of one reddening and three following: every drop leaks. The fifth,
"no storage row refers to a missing relation", is absolute (got [25] want [0]) and covers
the case the other four cannot see by construction — a standing leak that no single step
increases.
This matters more than a wording nit because the paragraph is the part a future reader will use
to decide how much to trust these arms, and as written it tells them to trust one of five when
they can trust five of five. It is the same failure mode as the control sentence you caught in
mine: a true observation about one arm generalised across the others.
Mechanically the PR is clean: one file, comment-only (executable lines with comments and blanks
stripped hash b911eef500b9 on main and on bd8cf7b), 8 checks both sides. It does not touch
anything #878 touched, so it should stay mergeable over ec444d7 without a rebase.
And thank you for the md5 correction in your own commit message. "The md5 is evidence an edit
applied, not a constant a reader reproduces" is the right distinction and I would not have
thought to state it.
jdatcmd
left a comment
There was a problem hiding this comment.
Approved at bd8cf7b. Half of my CHANGES_REQUESTED was wrong and I am withdrawing it.
I claimed your last sentence inverted the conclusion:
Only "no storage row refers to a missing relation" is absolute (
got [25] want [0]), and it is
the one that would still redden if the baselines degraded together.
That sentence is correct and my objection to it was not. I checked the scenario it actually
names. base is captured at :44 and re-captured at :53, :67, :80 — so a leak that is
already present when a baseline is taken is inside that baseline. If a standing leak exists and
no individual step adds to it, every increment is zero, all four relative arms pass, and only the
absolute arm — whose expected value is the literal 0 — fires. That is exactly what you wrote.
What does survive is narrower and it is a wording point, not a defect: "so they cascade once
the first leaks" describes a mechanism the arms do not have. They do not inherit an earlier
failure. My own body-gutted run shows it in the want values:
FAIL a plain table leaves nothing behind got [1/0/1/2/4/2/1] want [0/0/0/0/0/0/0]
FAIL dropping it takes the projection's metadata got [3/2/3/8/16/8/1] want [1/0/1/2/4/2/1]
FAIL two projections, one dropped by hand got [5/4/...] want [3/2/3/8/16/8/1]
FAIL ten create-and-drop cycles leave nothing got [25/24/...] want [5/4/5/15/30/15/1]
Every want is the previous got: the baseline absorbed the earlier leak, and each arm then
failed because its own step added more. Four independent detections that happen to fire
together because every drop leaks — not one detection and three echoes. It matters only for how
much a reader trusts them, which is what the paragraph is for.
Minimal edit that keeps your conclusion intact and fixes only the mechanism — take it or leave
it, I am not holding the PR for it:
Note the shape of the five. Four re-capture the baseline immediately before their own work
(:44,:53,:67,:80), so each measures the increment its own step adds rather than
inheriting an earlier failure — which is why all four redden here: every drop leaks. Only "no
storage row refers to a missing relation" is absolute (got [25] want [0]), and it is the one
that would still redden against a standing leak that no single step increases.
Everything else I verified stands: the five arm names and got [25] want [0] reproduce exactly
on my own body-gutted tree; the change is comment-only (executable lines with comments and blanks
stripped hash b911eef500b9 on main and on bd8cf7b); 8 checks both sides; one file; 12/12
green.
I should have tested the scenario your sentence named before objecting to it rather than
reasoning about the general case. That is the same error I have twice asked you to avoid today.
Comment-only, in one file. No arm changes —
test/drop_cleanup.sh's executable content is byte-identical with comment lines stripped, and the check count stays at 8.Why
Reviewing #878 turned up a coverage fact neither session knew. Remove the object-access DROP hook's call to
pgcolumnar_delete_storage_tree()and five ofdrop_cleanup.sh's eight arms redden, against four ofalter_am_cleanup.sh's forty-five:So
drop_cleanupis the file to run after touching that hook. That is not obvious from either file's subject line:alter_am_cleanupis named forSET ACCESS METHOD, which is where a reader would look, and it is the weaker holder of the two.What the header now says
The five arms by name, rather than a claim that the suite "covers the hook well". A count with named arms can be re-derived by anyone; a claim of adequacy rots quietly.
It also records the shape of the five, which is the part that matters if someone later trusts them. Four are relative — each compares a snapshot against the previous one — so they cascade once the first leaks. Only
no storage row refers to a missing relationis absolute (got [25] want [0]), and it is the one that would still redden if the baselines degraded together.Measured
Main
815dd0a, pg18a. The mutation was asserted applied by source md5 (69623fba996a→0e6947b1922b) and by reading the mutated line back out of the file, then re-derived on this branch with a differently-worded mutation (→ 28ef0a782ae1) to check the claim holds on the tree that publishes it — same 5 of 8 and 4 of 45. The md5 is evidence that an edit applied, not a constant a reader reproduces; the two differ because the injected comment text differs.Unmutated on pg18a and pg19a:
drop_cleanup8/8,alter_am_cleanup45/45,docs_style9/9.bash -nandshellcheck -S errorclean.Not an issue, on purpose
jdatcmd and I agreed this should not be filed on the tracker. A coverage fact has no action attached, so a ticket for it consumes triage attention forever and pays out once. The suite header is read by the person who needs it at the moment they need it, which a ticket is not.
🤖 Generated with Claude Code
https://claude.ai/code/session_01EtQbQUiMSpGWembJV1jxob