Skip to content

docs: drop_cleanup.sh says which call site its arms actually hold - #879

Merged
jdatcmd merged 1 commit into
mainfrom
docs/drop-cleanup-names-its-call-site
Sep 2, 2026
Merged

docs: drop_cleanup.sh says which call site its arms actually hold#879
jdatcmd merged 1 commit into
mainfrom
docs/drop-cleanup-names-its-call-site

Conversation

@OffgridwithJD

Copy link
Copy Markdown
Collaborator

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 of drop_cleanup.sh's eight arms redden, against four of alter_am_cleanup.sh's forty-five:

drop_cleanup      3 passed + 5 failed = 8
  a plain table leaves nothing behind
  dropping it takes the projection's metadata too
  two projections, one dropped by hand, leave nothing
  ten create-and-drop cycles leave nothing
  no storage row refers to a missing relation

alter_am_cleanup  41 passed + 4 failed = 45

So drop_cleanup is the file to run after touching that hook. That is not obvious from either file's subject line: alter_am_cleanup is named for SET 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 relation is 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 (69623fba996a0e6947b1922b) 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_cleanup 8/8, alter_am_cleanup 45/45, docs_style 9/9. bash -n and shellcheck -S error clean.

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

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 jdatcmd left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 jdatcmd left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@jdatcmd
jdatcmd merged commit 28c5d2a into main Sep 2, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants