test: port hilbert_cluster.sh to pytest, graded one-for-one (#432)

[OffgridwithJD]

Ports test/hilbert_cluster.sh to pytest: the Hilbert clustering SQL surface, the recorded
sorted_kind, the self-gate and the daemon (#432, #889's SQL half).

45 collected tests, 184 checks, all eight arms, and compare_to_bash.py reports 0
MISSING.

The definition of done

Measured on this branch, rebased onto 7ada68e (main with #1044 merged):

bash checks: 133 (123 distinct)
literal matches: 113 | template matches: 10 | missing: 0
VERDICT: every bash property is covered

123 distinct is the number the port must cover; 133 is the call count. Ten of the 133
names repeat, so a port covering the 123 distinct properties is complete and 133 was never a
target to match. (@jdatcmd, who took that reading off the same output I had been quoting the
wrong half of.)

133 is the post-#1044 number, and that dependency is worth naming. Before #1044 the
extractor read 124 for this suite; the nine missing names are its check_unrunnable sites,
all of which are twinned, and it has zero check_skip. Both graders reported 0 MISSING while
the port was built, so nothing here rests on the merge -- but if #1044 were reverted this port
would read as nine names over-complete and nothing in the tree would say why.

It is also the first exercise of that widening on a suite that uses those helpers
correctly. hilbert_locality was the broken case, where the names had no twin and #1041 fixed
them; this is the working case, where nine correct names were simply invisible. A change
exercised only against the broken case shows that it stops hiding a defect and never shows
that it adds names correctly.

Two places the port asserts something the original gets for free

Both are one class, and it is worth stating as a class because 218 unpaired suites remain:
wherever a port replaces a structural guarantee with a procedural one, it owes an arm the
original does not need.

The bash suite hands the daemon's naptime and thresholds to the server through
PGC_EXTRA_CONF, so they are in postgresql.conf before the postmaster starts and the suite
cannot run without them. pgc_cluster.py has no such hook, so the port sets them with
ALTER SYSTEM and a reload, available because all three are PGC_SIGHUP. That can silently
not take effect, and it would be silent in the worst way: at the default naptime the daemon
still acts, the poll still sees the tail fold, and every S7 arm passes while the values the
fixture claims to have set were never in force. So the three are read back from the server.

The arm failed on its first run with got '2s/0.2/0.05' want '2/0.2/0.05' -- SHOW returns
the unit. A premise that passes on its first run cannot tell you whether it reads the server
or restates the ALTER SYSTEM three lines above it.

Same for max_parallel_workers_per_gather = 0: the fixture SET it and nothing read it back
until the parity tool reported the bash suite's own premise as MISSING.

A reload is not a read

ALTER SYSTEM SET pgcolumnar.autovacuum = on;
SELECT pg_reload_conf();
SHOW pgcolumnar.autovacuum;        -->  off

A reload signals the postmaster; a backend already open absorbs it at its next command
boundary, and this module runs every statement through ONE connection by design. The bash
suite never meets it because every q is a fresh psql. _show_fresh() opens a new session
for post-reload reads -- the port of what the original gets for free, not a workaround.

This one is inherited by every later port that changes postmaster-level state, which is
ten bash suites by ALTER SYSTEM alone.

Design notes a reviewer should check rather than take

• Every conditional check_unrunnable is its own test. expect.cannot_run() records
  under the reason code, so two refusals in one test collapse onto a single
  UNMET_PRECONDITION record and neither can be told from the other.
• S1's fixture lives in its own schema, and that adds a way to pass wrongly. The probe
  connects as h_other, so without schema USAGE every call raises 42P01; granting it naively
  means a schema 42501 satisfies an ownership 42501 arm. The bash suite cannot hit this
  because its fixture is in public. Closed by a premise beside the existing EXECUTE one.
• _sqlstate distinguishes its three outcomes by type, not by scraping text. The bash
  helper needs a sentinel statement behind the one under test because psql cannot tell "no
  error" from "never reached the server". psycopg raises OperationalError from connect().
  Both controls are kept anyway -- a port that dropped the NOLOGIN control because "psycopg
  makes that impossible" would be asserting its own implementation rather than the property.
• What the port does not buy is in the file: over one column the Hilbert index and the
  Morton index are both the identity, so S5 is green on a relabelled Z-order implementation by
  construction. Four arms refuse one, and no others.

Verified

the file alone           45 passed, 184 checks, 184 pass + 0 fail + 0 unrun
guard half   CI-style    316 passed, 784 checks, RC=0        (18 files, --pgc-expect-tests 316)
cluster half CI-style    264 passed, 805 checks, RC=0        (14 files, --pgc-expect-tests 264)
parity                   133 bash checks, 0 MISSING, on the rebased tree
docs_style               14/14

expected_tests.txt: cluster_tests 219 -> 264, and both keys re-derived by collection on
the merged tree rather than trusted. #1044 moved guard_tests 313 -> 316 and this branch
moves cluster_tests; the two touched different lines so git kept both with no conflict. That
is not evidence -- three branches each deriving 280 correctly against their own tree is the
case that file was written for, and none of them conflicted either. Collected here:
guard 316 stated 316, cluster 264 stated 264.

guard_tests is unchanged by this branch: the new file needs a database, so it is not in
NO_CLUSTER.

One change to a file that is not the port

test_compare_to_bash.py's standing arm, test_the_ported_suites_in_this_tree_are_graded_one_for_one,
iterates a hard-coded list of stems. A new pair is not graded by it unless it is added, and
nothing fails when it is missing: the arm passes, grading the pairs it knows about, and reports
a clean verdict for a tree it has not fully looked at. hilbert_cluster is now in the list and
it grades eight pairs.

Found by chasing a +1 I could not explain -- cluster-half checks moved 805 -> 806 across the
rebase, which turned out to be #1044's inventory arm doing one more expect.at_least now that
test_compare_to_bash.py is a declared crossing. Not this branch's, and correct.

The defect is latent and this port is what makes it live: the list happens to equal the
pairs that exist today, so nothing has ever been silently ungraded. The docstring records that,
and #1046 tracks making the assertion two-directional -- which is a behaviour change rather
than a tidy-up, because an incomplete port is quietly absent today and would have to redden. It
is deliberately not done here.

Not done here

test/hilbert_cluster.sh is untouched. #1043 reports that its header still says the suite is
red, unregistered and its verbs absent -- all three landed in 4b66555 -- but fixing prose in
the file under port would widen this diff for an unrelated reason.

🤖 Generated with Claude Code

https://claude.ai/code/session_012RSw4qMHS7ByE7PY8Ns4cs

[jdatcmd]

APPROVE at 2c3cab9. 14/14, 0 pending, 0 real failures, 0 cancelled — read from the head sha's check-runs at 21:23:36Z, not from the PR summary. Re-run on this sha rather than carried from ba24695.

The evidence, and which half belongs to whom

Mine, run here:

guard half              316 collected | 785 checks | 785 pass + 0 fail + 0 unrun
parity, ALL EIGHT pairs 0 of 8 unclean
docs_style.sh           14/14 PASSED
bash names              133 (123 distinct) -- derived from the tree BEFORE reading the PR body
delta ba24695 -> 2c3cab9  AST of test_hilbert_cluster.py IDENTICAL, so comment-only is
                          verified rather than counted (a docstring change would have shown)

Not mine — @OffgridwithJD's and CI's. There is no PostgreSQL on this host, so cluster_tests 264, the 807 cluster checks and the whole poll measurement are theirs and CI's. I could not run that half and did not.

The three things I was asked to attack

1. The shared-name limit does not manifest. Enumerating repeated names across all eight graded pairs: hilbert_cluster has 10 repeats, hilbert_locality 4, the other six none. Nine of the ten are a check* paired with check_unrunnable under one name — the #1041 convention working, one property in two branches, so distinct < total is healthy rather than a smell. The tenth is two byte-identical fixture helpers asserting the same property for different tables. Zero instances corpus-wide.

2. The poll margin I could not test, and said so rather than reading the loop and offering a view. What came back is better than what I asked for: the count is recorded and printed, S7_POLLS is kept separate from S7_MARGIN so the window and the bound are not one number, and the count lands in the FAILING value — f"took {st['polls']} polls" — so a red names the number instead of merely refusing.

And the artefact was corrected to match its evidence: "one poll on each of five majors on an idle container; the bound of 5 is four above the only value ever observed; no loaded measurement exists." One value observed five times is not a distribution, and an idle container says little about a loaded runner — this repo has a race recorded at 0-in-400 idle against 6-in-400 under load. A reader meeting a red at six is the first person to see the loop do its job, not someone looking at a regression.

3. A second route to 42501: two candidates, both closed. The extension revokes EXECUTE from PUBLIC on five functions and none of cluster, recluster_hilbert, recluster_zorder is among them — had one been, h_other would have got 42501 for EXECUTE and the arm would have passed for the wrong reason. And of the four aclcheck_error sites in columnar_vacuum.c, two are explicit ACLCHECK_NOT_OWNER and two are SELECT-based inside functions S1 does not call. So the premise is complete for a stronger reason than "we granted USAGE".

Also verified

The eighth stem added to test_compare_to_bash.py is exactly what was agreed, and the docstring cites #1046 and says the rule is not enforced rather than issuing it — which is more useful to the next porter than the rule would be.

🤖 Generated with Claude Code

https://claude.ai/code/session_01NhwXKAgSmYDUjteWkfajHK