test/pytest: port native_ownership, asserting the SQLSTATE (#432)

[jdatcmd]

Nine maintenance and DDL functions, each refused to a non-owner. The port asserts two things the bash suite cannot, and finding the second corrected my own docstring.

The SQLSTATE, not the message

native_ownership.sh greps the output for must be owner. CLAUDE.md already states the rule that breaks:

Assert SQLSTATE, not error text: 42501 comes only from aclcheck_error.

The refusal is aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_TABLE, ...) at src/columnar_vacuum.c:189 and :205. A text grep passes whatever code the server attached, so the day one of these is raised as 22023 the bash suite stays green and every client switching on SQLSTATE breaks.

It does not conflate refusal with login

The bash suite runs each call through a separate psql as a role that must be able to connect. If that role could not log in, the grep finds nothing and the arm fails — for a reason with nothing to do with ownership. SET ROLE changes the effective user for permission checks without authenticating, so what is measured is the ownership check alone.

A third arm states the ordering the bash comment asserts in prose: the check fires before the work, so a non-owner is refused for a projection that does not exist rather than told it is missing.

The premise arm corrected its own docstring

Every refusal carries premise: alice reaches the table, because pgc_conn puts each test in a private schema.

I wrote that without the grant every arm would falsely pass on a 42501 from the schema check. Measured by removing it:

psycopg.errors.UndefinedTable: relation "n" does not exist    -- 42P01, not 42501

An unqualified name resolves through search_path and an unusable schema is simply skipped. So the arms fail rather than falsely pass, and the premise's value is that it fails first and names reachability instead of leaving 42501 != 42P01 to be interpreted. The false-pass case is real but needs a qualified reference, which raises 42501 permission denied for schema — the ownership refusal's own SQLSTATE from a different check.

The parity tool cannot grade this pair, and that bounds #432

Both sides build their names at runtime — bash as non-owner refused: ${1%%(*}, pytest as an f-string — so compare_to_bash.py reports PORT IS INCOMPLETE for a complete port.

Measured across the corpus:

81 of 253 suites carry at least one interpolated check name
252 of 4345 check names contain a shell expansion (6%)

So the parity verdict is a false red for a third of the suites. That is the safe direction, but it means the tool cannot be used as a gate for those, and anyone counting #432 progress by its verdict will undercount.

Verified

guard half     277 passed, 679 checks, --pgc-expect-tests enforced
cluster half   177 passed, 465 checks, enforced (166 -> 177)

The cluster half ran on a real PG17 cluster in the dev container. It is verifiable in CI at all because #1018 landed an hour ago — before that, ten of 25 pytest files were cluster-bound and CI ran none of them.

🤖 Generated with Claude Code

https://claude.ai/code/session_01EbyGSaU93XYQr8aH4NrUiw

[OffgridwithJD]

Verified every claim, including the self-correction, and they hold. One measurement to
contribute rather than a finding.

The text grep is real and the SQLSTATE is the right replacement

test/native_ownership.sh:34
  "$(grep -qi 'must be owner' <<<"$out" && echo yes || echo "no")" "yes"

One helper, nine functions, all asserting the MESSAGE. And the refusal is where you say:

src/columnar_vacuum.c:189   aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_TABLE, ...)
src/columnar_vacuum.c:205   aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_TABLE, get_rel_name(relid));

Your self-correction is right, measured here

I reproduced all three branches with VERBOSITY verbose so the code and the source location
are both visible:

UNQUALIFIED, no USAGE on the table's schema
  ERROR:  42P01: relation "n" does not exist
  LOCATION:  regclassin, regproc.c:911

QUALIFIED, no USAGE on the table's schema
  ERROR:  42501: permission denied for schema probe
  LOCATION:  aclcheck_error, aclchk.c:2793

WITH the grant, non-owner -- the arm's real subject
  ERROR:  42501: must be owner of table n
  LOCATION:  aclcheck_error, aclchk.c:2931

42P01 for the unqualified form, exactly as your corrected docstring says, and the
false-pass shape is the qualified one. search_path skipping an unusable schema is the
whole mechanism.

The measurement worth adding to the docstring

The last two are the same SQLSTATE from the same function, 138 lines apart. So for this
particular pair, the code is no more specific than the text was -- 42501 cannot tell
"permission denied for schema" from "must be owner of table", and only aclchk.c:2793 versus
aclchk.c:2931 separates them, which no client can see.

That does not weaken the change. Asserting 42501 is still strictly better than grepping
must be owner, because the failure the text grep cannot see is the code CHANGING, and that
is the one that breaks every client switching on SQLSTATE. But it does mean your premise arm
is not belt-and-braces -- it is the only thing standing between the suite and nine arms
passing on a schema grant that was never made. Your docstring says that; I am adding the
line numbers because they make it checkable rather than assertable.

I went looking for this as a defect, expecting to tell you the premise arm was load-bearing
in a way the PR had not noticed. It had noticed, and said so better than I was going to.

And you took the count obligation

expected_tests.txt is in the diff, which #1018 made mandatory about an hour before you
opened this. Four PRs have now moved those numbers and this is the first one where the
author updated it in the same commit as the tests rather than discovering it from a red job.

Two smaller notes, neither a hold

SET ROLE over a second psql -U alice is the right call for the reason you give, and
there is a second: the bash form needs LOGIN on the role, so it tests the ownership check
AND the ability to authenticate, and pg_hba is not the subject. Your version removes a
dependency on the cluster's auth configuration from a test about ownership.

The ordering arm -- refused before the work, so a non-owner gets the refusal and not
"projection does not exist" -- is the one I would have asked for if it were missing. The bash
comment asserts that ordering in prose and nothing checked it.

Approving on green; 9 of 12 checks were still running when I started.

[OffgridwithJD]

Approving. Green rollup on 1520b765, 14 checks, and every claim in the body verified against
the source or reproduced on a cluster.

The change is right for the reason you give -- a text grep passes whatever code the server
attached, so the failure it cannot see is the code CHANGING, and that is the one that breaks
every client switching on SQLSTATE.

What I verified rather than read:

test/native_ownership.sh:34   grep -qi 'must be owner'          one helper, nine functions
src/columnar_vacuum.c:189     aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_TABLE, ...)
src/columnar_vacuum.c:205     aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_TABLE, get_rel_name(relid))

UNQUALIFIED, no schema USAGE  42P01: relation "n" does not exist   regclassin, regproc.c:911
QUALIFIED, no schema USAGE    42501: permission denied for schema  aclcheck_error, aclchk.c:2793
WITH the grant, non-owner     42501: must be owner of table n      aclcheck_error, aclchk.c:2931

Your corrected docstring is exactly right: 42P01 for the unqualified form, and the false-pass
shape is the qualified one, because search_path skips an unusable schema rather than refusing
through it.

The measurement I would add to that docstring is the last two lines. Both are 42501 from
aclcheck_error, 138 lines apart in the same file. So for this pair the code is no more
discriminating than the text was, and your premise arm is not defensive tidiness -- it is the
only thing between this suite and nine arms passing on a schema grant nobody made. You say that
in prose; the line numbers make it checkable.

I went into this expecting to tell you the premise arm was load-bearing in a way the PR had not
noticed. It had, and put it better than I was going to. My own first probe was under-specified
-- I had not granted USAGE on the pgcolumnar schema, so all three branches returned the same
42501 and told me nothing until I fixed the probe.

Two things beyond the SQLSTATE that I would have asked for had they been missing:

SET ROLE over a second psql -U alice. Your reason is right, and there is a second: the
bash form needs LOGIN on the role, so it tests ownership AND authentication, and pg_hba is
not the subject. This removes a dependency on the cluster's auth configuration from a test
about ownership.

The ordering arm. Refused before the work, so a non-owner gets the refusal rather than
"projection does not exist". The bash suite asserts that ordering in a comment and nothing
checked it.

And you updated expected_tests.txt in the same commit as the tests. Four PRs have moved those
numbers since #1018 landed and this is the first where the author did it rather than learning it
from a red job -- including mine, twice.