Skip to content

test: the ANALYZE cap's guard defends a class, not one token (#1252) - #1260

Merged
jdatcmd merged 1 commit into
commandprompt:mainfrom
OffgridwithJD:fix/1252-followup
Sep 24, 2026
Merged

jdatcmd merged 1 commit into
commandprompt:mainfrom
OffgridwithJD:fix/1252-followup

Conversation

@OffgridwithJD

Copy link
Copy Markdown
Collaborator

Follow-up to #1258, addressing all three points @jdatcmd raised in review. They
were not blockers there; they are cheap and two of them are the kind of thing
this repo does not leave lying around.

1. The guard defends a class, and my comment named one member of it

I wrote that a partially numeric token was "the one input it actually
defends"
. It is not — the guard refuses everything bash will not read as an
int64:

  pgc_analyze_cap_ms 12abc                  guarded 5000   unguarded <arithmetic error>
  pgc_analyze_cap_ms 99999999999999999999   guarded 5000   unguarded 7751640039368425452

$(( )) wraps silently and returns a positive number past the floor, so the
unguarded function yields a cap that can never fail. The arithmetic error is
loud; a bound that always passes looks like a green test.

Operationally this changes nothing — no insert takes 10^20 ms. It matters
because that sentence is precisely what a later reader would cite to justify
deleting a guard which also catches the silent case. A wrong comment is an
input to the next bug.

An arm now covers the overflow member, so the claim is checkable rather than
asserted.

2. The dead conjunct goes, by the argument I used one line above

c > 0 in the awk cannot fire: pgc_analyze_cap_ms returns
max(5000, ...), so cap_ms is never <= 0. It was meaningful in the old form
where scan_ms really could be 0.

By the rule I used for the input guard — a condition either gets covered or gets
removed — and this one cannot be covered, so it goes.

3. ins_ms is now named for what it measures

_as_t0 starts before the DROP and CREATE, which arrive in the same
psql_run as the INSERT and cannot be timed apart. So ins_ms overstates the
insert, which inflates the cap — the safe direction. The variable name is kept
because the arms are ledgered under it; the comment now says what it covers.

Measured

Mutation C, the input guard removed, one merge per major:

  major 15   a malformed insert time yields the floor rather than an arithmetic error
             and an insert time past int64 yields the floor rather than a cap that cannot fail
  ... 16, 17, 18, 19 identical

  2 arms x 5 majors = 10 reds, and no other check went red in any of the five logs

Published before counting, and it agrees:

  ledger rows   1740 + 1 = 1741
  dated          166 + 1 =  167
  never                1574   unchanged
  1574 + 167 == 1741
  analyze_stats       41 checks  PASSED  pg17a / pg18a
  harness_selftest  1198 checks  PASSED  pg17a / pg18a
  docs_style          55 checks  PASSED  pg17a / pg18a
  rc=0, zero FAIL rows

  real run: ANALYZE 107 ms against a 6400 ms cap (insert 320 ms)

Not addressed here, deliberately

The cross-machine tracking assumption stays open. I tried to test it with a
controlled slowdown and the instrument failed — a fixed CPU spin timed flat at
226/222/225/225 ms across every container CPU budget, so the throttle never bit
and all three "slower" rows were taken at full speed. That is recorded on #1258
rather than quietly dropped. The floor is what makes the assumption
non-load-bearing in the flaking direction, as you said.

🤖 Generated with Claude Code

https://claude.ai/code/session_01MpajdQbkVJ9ey1XyYHcikP

…prompt#1252)

commandprompt#1258's comment said a partially numeric token was "the one input it actually
defends". It is not. The guard refuses everything bash will not read as an
int64, and the second member is the dangerous one:

    pgc_analyze_cap_ms 12abc                 guarded 5000  unguarded <arith error>
    pgc_analyze_cap_ms 99999999999999999999  guarded 5000  unguarded 7751640039368425452

`$(( ))` WRAPS SILENTLY and hands back a positive number past the floor, so the
unguarded function returns a 7.7-quintillion-millisecond cap -- a bound that can
never fail. The arithmetic error is loud; this one looks like a green test.

That matters because the original sentence is exactly what a later reader would
cite to justify deleting a guard that also catches the silent case. An arm now
covers the overflow member, so the claim is checkable rather than asserted.

A DEAD CONJUNCT GOES BY THE SAME ARGUMENT. `c > 0` in the awk cannot fire:
pgc_analyze_cap_ms returns max(5000, ...), so cap_ms is never <= 0. It was
meaningful in the old form, where scan_ms really could be 0. By the rule used
for the input guard -- a condition either gets covered or gets removed -- this
one cannot be covered.

AND ins_ms IS NAMED FOR WHAT IT MEASURES. `_as_t0` starts before the DROP and
CREATE, which arrive in the same psql_run as the INSERT and cannot be timed
apart, so ins_ms overstates the insert. That inflates the cap, which is the safe
direction. The name is kept because the arms are ledgered under it.

All three named by @jdatcmd reviewing commandprompt#1258.

Measured, mutation C (the input guard removed), one merge per major:

    2 arms x 5 majors = 10 reds, and no other check went red

    ledger rows   1740 + 1 = 1741
    dated          166 + 1 =  167
    never                1574   unchanged
    1574 + 167 == 1741

Verified: analyze_stats 41, harness_selftest 1198, docs_style 55 -- all PASSED
on pg17a and pg18a, rc=0, zero FAIL rows. Real run: ANALYZE 107 ms against a
6400 ms cap (insert 320 ms).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MpajdQbkVJ9ey1XyYHcikP

@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.

Approving. 15 of 15 green, both suites included, and the check-name set is
identical to #1258's head, so nothing was added to the matrix and skipped.

All three findings are taken, and the ledger delta says so rather than asking me
to believe it: +2/-1 is one existing row gaining a fourth mutation description
and one new row arriving, with nothing dropped. No declared count moves — the 28
budget lines are prose, and the prediction in them (1741 = 1574 + 167, never
unchanged) is keyed to 493e018d, which is current main.

I verified the overflow pair independently before you posted yours, and we agree
to the digit:

  pgc_analyze_cap_ms 99999999999999999999   guarded 5000   unguarded 7751640039368425452

The negative result is the part of this PR I would keep. You set out to
settle whether the insert term tracks ANALYZE across machines, could not, and
said so on the PR instead of publishing three rows that looked like an answer.
The fixed-CPU spin timing 226/222/225/225 ms across 8 cores, 1 core, 25% and 10%
is the control doing exactly the job a control is for: without it the throttle's
failure to bite would have read as "the terms track across a 5x slowdown", which
is a sentence about one machine measured three times.

That CI cannot supply the pair either — the line is printed but the harness
suppresses per-suite detail on a PASS, absent from 273 KB of #1258's green run —
is worth having written down. It means the first cross-machine reading arrives
on the first CI run where analyze_stats fails for any reason, which is a worse
instrument than a designed experiment, and naming that is better than implying
the number exists.

The floor is what makes this safe regardless, as you say: divergence can only
make the guard more permissive, never flakier.

@jdatcmd
jdatcmd merged commit c096eee into commandprompt:main Sep 24, 2026
15 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