From 1520b7653f927c1e092e1d392d2946c25007b022 Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Sat, 12 Sep 2026 14:59:49 -0600 Subject: [PATCH] test/pytest: port native_ownership, asserting the SQLSTATE (#432) Nine maintenance and DDL functions, each refused to a non-owner. The port asserts things the bash suite cannot. THE SQLSTATE, NOT THE MESSAGE. native_ownership.sh greps the output for `must be owner`. CLAUDE.md 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 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 unrelated to ownership. `SET ROLE` changes the effective user for permission checks without authenticating. 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 MY 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: alice gets `42P01 relation "n" does not exist`, because an unqualified name resolves through search_path and an unusable schema is skipped. So the arms FAIL rather than falsely pass, and the premise's value is that it fails FIRST and names reachability. The false-pass case is real but needs a QUALIFIED reference, which raises 42501 for the schema. AND THE PARITY TOOL CANNOT GRADE THIS PAIR. Both sides build 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 names overall. That bounds how much of #432's parity the tool can certify. It is a false red rather than a false green, which is the safe direction, but it means the verdict cannot be used as a gate for a third of the suites. guard half 277 passed, 679 checks, count enforced cluster half 177 passed, 465 checks, count enforced (166 -> 177) Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01EbyGSaU93XYQr8aH4NrUiw --- CHANGELOG.md | 28 +++++ test/pytest/TESTS.md | 54 ++++++++++ test/pytest/expected_tests.txt | 4 +- test/pytest/test_native_ownership.py | 148 +++++++++++++++++++++++++++ 4 files changed, 233 insertions(+), 1 deletion(-) create mode 100644 test/pytest/test_native_ownership.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 3ec129fe..7baf8839 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,34 @@ true until the next version shipped. ### Added +- `native_ownership.sh` has a pytest twin, and it asserts the SQLSTATE (#432). + + Nine maintenance and DDL functions, each refused to a non-owner with 42501 rather + than with a grep for `must be owner`. `CLAUDE.md` already states the rule: 42501 + comes only from `aclcheck_error`, and the refusal is + `aclcheck_error(ACLCHECK_NOT_OWNER, ...)` at `columnar_vacuum.c:189` and `:205`. A + text grep passes whatever code the server attached. + + It also stops conflating refusal with login: the bash suite runs each call as a role + that must be able to connect, so a role that could not log in fails the arm for a + reason unrelated to ownership. `SET ROLE` changes the effective user without + authenticating. + + 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. + + THE PREMISE ARM CORRECTED ITS OWN DOCSTRING. Every refusal carries `premise: alice + reaches the table`, because each test runs in a private schema. Measured by removing + the grant, alice gets `42P01 relation does not exist` rather than the 42501 I had + claimed: an unqualified name resolves through `search_path` and an unusable schema + is skipped, so the arms fail rather than falsely pass. The false-pass case needs a + QUALIFIED reference, which raises 42501 for the schema. + + AND THE PARITY TOOL CANNOT GRADE THIS PAIR. Both sides build names at runtime, so + `compare_to_bash.py` reports `PORT IS INCOMPLETE` for a complete port. Measured: 81 + of 253 suites carry at least one interpolated check name. That bounds how much of + #432's parity the tool can certify, and it is a false red rather than a false green. + - The mutation ledger records WHICH MAJORS each check exists on (#1010). suitepartnamemajorslast-redmutations diff --git a/test/pytest/TESTS.md b/test/pytest/TESTS.md index c18afb96..49905eb5 100644 --- a/test/pytest/TESTS.md +++ b/test/pytest/TESTS.md @@ -76,6 +76,7 @@ behaviour, the source of that number is named. - [28. test_docs_join_clustering.py: the runtime filter's layout precondition](#28-test_docs_join_clusteringpy-the-runtime-filters-layout-precondition) - [29. test_join_vector_agg.py: ungrouped fold over a unique-key join](#29-test_join_vector_aggpy-ungrouped-fold-over-a-unique-key-join) - [30. test_differential.py: the heap oracle, type matrix](#30-test_differentialpy-the-heap-oracle-type-matrix) +- [31. test_native_ownership.py: every maintenance function is owner-only](#31-test_native_ownershippy-every-maintenance-function-is-owner-only) ## 1. How to read a test in here @@ -2936,3 +2937,56 @@ arms while asserting nothing. Four columns of four types in one WHERE, where a per-column arm cannot reach: the scan combines their skip decisions, and a predicate right alone can be wrong in conjunction. + + +## 31. test_native_ownership.py: every maintenance function is owner-only + +Port of `test/native_ownership.sh` (#432). The maintenance functions rewrite data, +reclaim space, or take strong locks, so they are owner-only like VACUUM and CLUSTER. + +### Two things the bash suite cannot assert + +**The SQLSTATE, not the message.** `native_ownership.sh` greps the output for +`must be owner`. `CLAUDE.md` 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 a refusal 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 unrelated to ownership. +`SET ROLE` changes the effective user for permission checks without authenticating. + +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, and what measuring it corrected + +Every refusal arm carries `premise: alice reaches the table`. `pgc_conn` puts each +test in a private schema, and without USAGE on it the refusals would be about +something else. + +**What that something else is turned out not to be what the docstring first said.** +Measured by removing the grant: alice gets `42P01 relation "n" does not exist`, +because an unqualified name resolves through `search_path` and an unusable schema is +skipped. So the arms FAIL rather than falsely pass, and the premise's value is that +it fails first and names reachability. The false-pass case is real but narrower: a +QUALIFIED reference into a schema without USAGE raises `42501 permission denied for +schema`, the ownership refusal's own SQLSTATE from a different check. + +| test | what it pins | +| --- | --- | +| `test_non_owner_is_refused_with_42501` | nine arms, one per function, each 42501 and each carrying the reachability premise | +| `test_the_owner_is_allowed` | the control: a gate that refused everyone would satisfy all nine | +| `test_the_check_fires_before_the_work` | a non-owner is refused for a projection that does not exist | + +### compare_to_bash.py cannot grade this pair + +Both sides build their names at runtime -- bash as `non-owner refused: ${1%%(*}`, +pytest as an f-string -- so the tool reports `PORT IS INCOMPLETE` for a port that is +complete. Measured across the corpus: **81 of 253 suites** carry at least one +interpolated check name, 252 of 4345 names overall. The verdict is a false red for a +third of the suites, which bounds how much of #432's parity the tool can certify. diff --git a/test/pytest/expected_tests.txt b/test/pytest/expected_tests.txt index f1da3f3e..ba9d1148 100644 --- a/test/pytest/expected_tests.txt +++ b/test/pytest/expected_tests.txt @@ -50,4 +50,6 @@ guard_tests 277 # # The rate is the point. The ungated half grew by 67 tests in the time this took to review, # which is the argument for gating it rather than a detail about it. -cluster_tests 166 +# 166 -> 177 when test_native_ownership.py landed: nine parametrized refusal arms, +# the owner control, and the check-ordering arm. +cluster_tests 177 diff --git a/test/pytest/test_native_ownership.py b/test/pytest/test_native_ownership.py new file mode 100644 index 00000000..57092831 --- /dev/null +++ b/test/pytest/test_native_ownership.py @@ -0,0 +1,148 @@ +"""Every maintenance and DDL function refuses a non-owner (#432, port of native_ownership.sh). + +The maintenance functions rewrite data, reclaim space, or take strong locks -- +`truncate` takes AccessExclusiveLock -- so they are owner-only, like VACUUM and +CLUSTER. The ownership check sits immediately after the relation is opened and +confirmed columnar, before any work. + +TWO THINGS THIS PORT ASSERTS THAT THE BASH SUITE CANNOT. + +**The SQLSTATE, not the message.** `native_ownership.sh` greps the output for +`must be owner`. `CLAUDE.md` states the rule this breaks: "Assert SQLSTATE, not +error text: 42501 comes only from `aclcheck_error`." The refusal here is +`aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_TABLE, ...)` at +`src/columnar_vacuum.c:189` and `:205`, which is `ERRCODE_INSUFFICIENT_PRIVILEGE`. +A text grep passes whatever code the server attached, so the day one of these +refusals is raised as 22023 or 0A000 the bash suite stays green and every client +that switches 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 no `must be owner` and the arm fails -- for a reason that +has nothing to do with ownership. `SET ROLE` changes the effective user for +permission checks without involving authentication, so what is measured here is +the ownership check and nothing else. + +A third arm states the ordering the bash suite's 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. +""" + +import pytest + +# Every maintenance and DDL entry point, with an argument list that would be valid +# for the owner. The names are the bash suite's, character for character, so +# compare_to_bash.py can diff the two by property. +OWNER_ONLY = [ + "compact('n')", + "compact_rewrite('n', 0.0)", + "recluster('n', 'id')", + "vacuum('n')", + "vacuum_sorted('n', 'id')", + "cluster('n', 'id')", + "truncate('n')", + "add_projection('n', 'p', ARRAY['id','v'])", + "drop_projection('n', 'p')", +] + + +def _fixture(cur): + """Table, role, and enough grants that ONLY ownership can refuse alice. + + THE SCHEMA GRANT IS NOT HOUSEKEEPING, and what it protects against is not what + I first wrote. `pgc_conn` puts each test in a private schema. Measured by + removing the grant: alice gets `42P01 relation "n" does not exist`, because an + unqualified name resolves through `search_path` and a schema she cannot use is + simply skipped. So the refusal arms FAIL rather than falsely pass, and the + premise arm's value here is that it fails FIRST and names reachability instead + of leaving `42501 != 42P01` to be interpreted. + + The false-pass case is real but narrower: a QUALIFIED reference into a schema + without USAGE raises `42501 permission denied for schema`, which is the + ownership refusal's own SQLSTATE from a different check. These calls pass the + table as an unqualified string, so they land on the 42P01 side today -- and the + premise arm is what keeps that from being an assumption. + + The role is cluster-global and these tests run in parallel under xdist, so it is + created idempotently and never dropped: `DROP ROLE` fails with + DependentObjectsStillExist once it holds a grant, and dropping a role another + worker is using is worse than leaving it. + """ + cur.execute("SELECT 1 FROM pg_roles WHERE rolname = 'alice'") + if cur.fetchone() is None: + cur.execute("CREATE ROLE alice NOSUPERUSER") + cur.execute("GRANT USAGE ON SCHEMA pgcolumnar TO alice") + cur.execute("SELECT current_schema()") + schema = cur.fetchone()[0] + cur.execute(f'GRANT USAGE ON SCHEMA "{schema}" TO alice') + cur.execute("DROP TABLE IF EXISTS n") + cur.execute("CREATE TABLE n (id int, v int) USING pgcolumnar") + cur.execute( + "SELECT pgcolumnar.set_options('n', stripe_row_limit => 1000," + " chunk_group_row_limit => 1000)" + ) + cur.execute("INSERT INTO n SELECT g, g FROM generate_series(1, 3000) g") + cur.execute("GRANT SELECT ON n TO alice") + + +def _alice_can_read(cur): + """The premise every refusal arm rests on: alice reaches the table.""" + cur.execute("SET ROLE alice") + try: + cur.execute("SELECT count(*) FROM n") + return cur.fetchone()[0] + finally: + cur.execute("RESET ROLE") + + +@pytest.mark.parametrize("call", OWNER_ONLY, ids=lambda c: c.split("(")[0]) +def test_non_owner_is_refused_with_42501(pgc_conn, expect, call): + """One arm per function, named as the bash suite names it.""" + fn = call.split("(")[0] + with pgc_conn.cursor() as cur: + _fixture(cur) + reachable = _alice_can_read(cur) + cur.execute("SET ROLE alice") + try: + with pytest.raises(Exception) as excinfo: + cur.execute(f"SELECT pgcolumnar.{call}") + finally: + cur.execute("RESET ROLE") + expect.num(reachable, 3000, + f"premise: alice reaches the table, so a 42501 is about ownership: {fn}") + expect.sqlstate(excinfo.value, "42501", f"non-owner refused: {fn}") + + +def test_the_owner_is_allowed(pgc_conn, expect): + """The control. A gate that refused everyone would satisfy every arm above.""" + with pgc_conn.cursor() as cur: + _fixture(cur) + cur.execute("SELECT pgcolumnar.compact('n')") + cur.execute("SELECT count(*) FROM n") + after_compact = cur.fetchone()[0] + cur.execute("SELECT pgcolumnar.recluster('n', 'id')") + cur.execute("SELECT count(*) FROM n") + after_recluster = cur.fetchone()[0] + expect.num(after_compact, 3000, "owner compact allowed") + expect.num(after_recluster, 3000, "owner recluster allowed") + + +def test_the_check_fires_before_the_work(pgc_conn, expect): + """The ordering the bash suite states in prose and does not assert. + + `drop_projection` on a projection that does not exist. A non-owner must still get + 42501 rather than a missing-object error, which is what "the check sits right + after the relation is opened, before any work" means. + """ + with pgc_conn.cursor() as cur: + _fixture(cur) + reachable = _alice_can_read(cur) + cur.execute("SET ROLE alice") + try: + with pytest.raises(Exception) as excinfo: + cur.execute("SELECT pgcolumnar.drop_projection('n', 'no_such_projection')") + finally: + cur.execute("RESET ROLE") + expect.num(reachable, 3000, "premise: alice reaches the table") + expect.sqlstate(excinfo.value, "42501", + "ownership is checked before the projection is looked up")