test: attribute a refusal by SQLSTATE, and fix the tool that grades it (#432)

[jdatcmd]

Two changes. The second is why the first could be measured, and it turns out to be the larger finding.

The port: projection_privilege.sh gets a pytest twin, and both halves stop reading messages

The bash suite decided four things by matching error text, and two of them were load-bearing rather than decorative. Its own comment says why:

Both layers reject this role, so a bare "refused" stays true if the REVOKE is deleted and the C check catches it instead -- measured: with the REVOKE removed this suite still passed 14 of 14.

Both ACL layers raise 42501, so the code alone does not separate them. The fixture does. Called with a projection name that does not exist, on a table the role may read:

the caller	outcome
no EXECUTE	42501 -- the body never ran to notice the name
EXECUTE	42704 -- it ran, and the projection lookup raised

The refusal is attributed by what the code REACHED, which is a fact about execution rather than about wording. RLS is a third code again: 0A000, from ERRCODE_FEATURE_NOT_SUPPORTED, a different SQLSTATE class from either ACL refusal. Both harnesses do this now; the shell half reads the state through psql -v VERBOSITY=sqlstate.

Two orderings the source asserts and neither harness tested

• The base ACL is checked before the projection is looked up, so a caller with no SELECT cannot learn whether a named projection exists on a table it may not read.
• The ACL is checked before RLS, so a caller with no privilege is not told the table has row-level security enabled. src/columnar_vacuum.c records this being corrected in review, with the measurement; nothing tested it.

Removal proof: five mutations, each asserted to apply at both call sites

mutation	pytest	shell
drop the base ACL check	4 arms red	--
drop the RLS refusal	2 arms red	--
RLS before the ACL	the RLS-ordering arm alone	the RLS-ordering arm alone
ACL below the lookup but above its raise	nothing red, correctly	same
ACL below the raise	both ordering arms	both: got [42704] want [42501]

Row four is the one that says the ordering arms measure an ordering rather than the presence of a check. Row five is the disclosure itself, printed.

The tool: compare_to_bash.py was reading the wrong argument

It decides whether a port is one-for-one, which is #432's definition of done. The python side was matched with expect\.\w+\([^)]*?"([^"]+)"..., and [^)]*? is lazy, so it stopped at the FIRST quoted argument:

call	name read
expect.num(got, 1, NAME)	NAME -- correct, which is why it looked right
expect.sqlstate(err, "42501", NAME)	"42501"
expect.text(got, "none", NAME)	"none"

Every SQLSTATE assertion was read as the literal 42501, counted as an "extra" the bash suite lacks, while the real property was reported MISSING. #432's ports are exactly the ones replacing a grep on a message with a SQLSTATE assertion, so the tool went blind in proportion to the work being done well.

test_hilbert_locality.py records somebody hitting this and working around it by rewriting their test file until the count fell, then concluding the rest needed a change here. It did.

Measured over every pair in the tree

pair	missing before	after
differential	6	0
hilbert_locality	13	0
native_ownership	1	0
native_projection	0	0
projection_privilege	23	0
stats_privilege	9	0
zonemap_boundaries	9	0
total	61	0

Of the 61, 34 were never missing. The rest were real and are closed here: stats_privilege had invented a name for a property the bash suite already named, and zonemap_boundaries was missing its backend alive premise outright. Neither was visible while the tool reported the wrong string.

It parses with ast now and takes the last string argument, resolving f-strings to templates, both arms of a conditional, and the name column of a @pytest.mark.parametrize — the idiom a repeated bash property should be ported to. Bash interpolations reduce the same way, including $1, the commonest one in a check name, which my first version of the reducer missed because its pattern required a letter after the dollar.

test_compare_to_bash.py guards it. Four mutations, each reddening its own arm plus test_the_ported_suites_in_this_tree_are_graded_one_for_one, which is the arm that matters: a guard over invented sources proves the extractor reads python, not that the tool grades this tree.

Verified

guard half            295 collected, 729 checks, 0 unrun
cluster half          217 collected, 614 checks, 0 unrun
projection_privilege.sh   24/24 PASSED
zonemap_boundaries.sh      9/9  PASSED
docs_style.sh                   PASSED
all seven pairs           rc=0, 0 missing

Closes part of #432. The remaining text-only refusal suites are rls_direct_storage and import_export_privilege.

🤖 Generated with Claude Code

https://claude.ai/code/session_01EbyGSaU93XYQr8aH4NrUiw

[OffgridwithJD]

APPROVE at ebcfabf8. 14 checks, 0 failures, CLEAN. The diagnosis is right, the fix works,
and I verified it on a real pair rather than from the diff. One residual below that I would have
expected in scope, and it is not a false red, so it does not block.

The diagnosis is the best line in the PR

The tool got blinder as the work it grades got better.

That is exactly it, and it explains why the note I left on #1027 (PORT IS INCOMPLETE for a
complete port) read as a tool limitation rather than a bug. [^)]*?"([^"]+)" being lazy means
the first quoted argument wins, so expect.sqlstate(err, "42501", NAME) reported 42501 as the
name — and every #432 port exists to replace a message grep with a SQLSTATE. The tool was
worst precisely where the work was best.

Verified on the real pair

compare_to_bash.py hilbert_locality.sh test_hilbert_locality.py
  literal matches: 19 | template matches: 11 | missing: 0
  VERDICT: every bash property is covered        rc=0

Template matching earns its keep: 11 of the 30 matches are names both harnesses build at
runtime, including the box $box: family. Reporting them separately is the right call — a
template match says the two assert a property of the same shape, not over the same values, and
folding them in would have overstated parity.

guard_tests    298, matching `298 tests collected` by derivation
cluster_tests  217, matching `217 tests collected`
guard half     298 passed / 737 checks, --pgc-expect-tests 298 armed
TESTS.md       TOC 36 entries 1..36, sections 36 1..36, both 0 out-of-order

The port asserts the SQLSTATE 10 times — 6x 42501, plus 0A000 and 42704 — so it is
attributing the refusal rather than matching its sentence, which is the point of the change.

The residual: three shapes where the name is NOT the last argument

node.args[-1] is right for num, text, sqlstate, rows, at_least and the name=
kwarg. It is wrong for three, and two of them are visible in the output above:

call	what the tool takes	the actual name
refusal(result, name, *patterns)	the last pattern	name
cannot_run(reason, detail)	detail	reason — _record(name=reason, ...)
plan_marker(plan, key, name=...)	the key and the name	the name

Measured on a probe through the new _py_names, 7 of 10 shapes correct:

expect.refusal(result, "refusal: THE NAME", "pattern one", "pattern two")
    -> 'pattern two'
expect.cannot_run("MISSING_DEPENDENCY", "cannot_run: this is the DETAIL not the name")
    -> 'cannot_run: this is the DETAIL not the name'
expect.plan_marker(plan, "a key with no name")
    -> 'a key with no name'

Two of the three extra lines on hilbert_locality are this, not real extras:

extra    Columnar Projected Columns                      <- a plan_marker KEY (line 719)
extra    the two partitions are not different ({}), ...  <- a cannot_run DETAIL (line 792)
extra    parallelism is off on the connection ...        <- genuinely extra, and the
                                                            docstring says so: "No bash twin
                                                            by name"

Why it does not block. rc is driven by MISSING, so extras move neither the exit status nor
the verdict. Nothing is graded wrong today, and no ported file uses refusal at all — it
appears only in test_raises_sqlstate.py and test_guards_pinned.py, which have no bash twin.
cannot_run and plan_marker do appear in a graded pair, which is why the noise is visible.

Why I would still finish it here. The PR's argument is that false entries are what made the
tool untrustworthy, and two false entries survive in its own output on a pair it adds to the
standing list. The next person reading that report has to know which of three extra lines to
believe.

And the tests pin the rule without its counter-examples. Eight arms, and the first is
test_the_name_is_the_last_argument_not_the_first_string. None exercises refusal,
cannot_run or plan_marker. test_the_ported_suites_in_this_tree_are_graded_one_for_one is
the right idea and cannot catch this either, because it asserts rc == 0 and extras never move
rc.

A smaller fix than it looks. Skipping args[-1] when a name= kwarg is present closes
plan_marker on its own and is one condition. The other two need the position named per helper.
Deriving it from inspect.signature(Expect.<helper>) would be self-maintaining, but it means
importing pgc_vacuity, which pulls in pytest — and the tool is deliberately standalone today
(ast, re, sys). A small map with a comment pointing at the signatures may be the cheaper
honest choice; your tool, your call.

One note for the merge

#1035 also moves expected_tests.txt, so whichever of us lands second re-derives. Your own
guard_tests 280 finding applies to exactly this: both numbers will be right for the tree they
were taken on and wrong for the merge.