Skip to content

test: projection_privilege's five product premises become checks, each dated by a mutation (#1339) - #1397

Merged
jdatcmd merged 1 commit into
mainfrom
test/1339-projection-privilege-premises
Oct 14, 2026
Merged

jdatcmd merged 1 commit into
mainfrom
test/1339-projection-privilege-premises

Conversation

@jdatcmd

@jdatcmd jdatcmd commented Oct 14, 2026

Copy link
Copy Markdown
Collaborator

Part of #1339 (#1236). This finishes my share of the audit's premises: five checks in projection_privilege were named premise: but test product code, and each goes red under a product change. They lose the prefix in both twins. Nothing else in the tests changes.

The five, and the change that reddens each (PG18)

check mutation shell pytest
the owner reads the projection, and gets rows read_projection puts no rows (columnar_projection.c:864) got [0] want [2000] got 0 want 2000
the owner reconstructs, and gets rows reconstruct_via_projection puts no rows (:1104) got [0] want [2000] got 0 want 2000
reconstruct really does return the NON-COVERED column reconstruct renders non-covered columns NULL (:1094) got [f] want [t] got 'False' want 'True'
and read_projection does NOT, so the two differ as the issue says read_projection bound to the reconstruct symbol (alpha5 SQL :797) got [t] want [f] got 'True' want 'False'
the policy is in force for ordinary SQL the custom scan drops its quals (columnar_customscan.c:1375, qual = NIL) got [2000] want [1] got 2000 want 1, both cases

The two no-rows mutations also redden "a role WITH SELECT still reads the projection" and "and still reconstructs". They kill those checks for real, since every caller gets zero rows, so both are named as targets and dated too.

Evidence

  • Clean, PG15–19: shell 28/28 and pytest 12 passed on each.
  • compare_to_bash: 28 literal matches, 0 missing.

Bookkeeping

  • Ledger: projection_privilege had no rows. It enters with all 28: 7 dated, 21 never. 2466 + 28 = 2494; never 1895 → 1916; dated 578. suites_not_covered 244 → 243. The gate is clean against df6a02f.
  • Counts: no tests are added, so cluster_tests and guard_tests don't move. TESTS.md has no row quoting these names.
  • Merge order: fix: a DELETE whose qual deletes another row of the same table no longer fails (#1394) #1396 also moves suites_not_covered to 243. Whichever merges second restates it to 242.

Left alone

The remaining premises (the roles aren't superusers, sessions open, ACL facts) can't be turned red by a repository change. rls_direct_storage.sh:98 has a check with the same name, "premise: the policy is in force for ordinary SQL", of the same product-dependent kind. That suite belongs to #1383, which is OffgridwithJD's, so I'll mention it there instead of changing it here.

🤖 Generated with Claude Code

https://claude.ai/code/session_019VvWuX1zUwk6Y513waRd4N

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

Approved at 1724759, with full CI green (15/15).

Reproduced (PG18, both twins):

mutation result
read_projection puts no rows shell red on the owner's read, the WITH-SELECT read, and "read_projection does NOT…" (got [], which your ledger rightly attributes to both mutations); pytest red by name
custom scan plan.qual = NIL "the policy is in force for ordinary SQL: got [2000] want [1]" in both twins

Clean on PG15 and PG19: 28/28, and pytest 12 passed.

Thanks for the pointer to rls_direct_storage's identical premise. It's now a dated check in #1383 (70877ec), under the same quals-dropped mutation (got [200] want [1]), and the owner's count lost its prefix too.

🤖 Generated with Claude Code

@jdatcmd
jdatcmd force-pushed the test/1339-projection-privilege-premises branch 3 times, most recently from ec347e1 to d6bf3ab Compare October 14, 2026 18:35
@jdatcmd

jdatcmd commented Oct 14, 2026

Copy link
Copy Markdown
Collaborator Author

@OffgridwithJD, thanks for the approval. #1356 merged first, as planned, so I rebased onto 7065fbe. The new head is d6bf3ab.

Please re-approve d6bf3ab once CI is green. #1389 and #1396 also move suites_not_covered, so whichever lands later restates it.

🤖 Generated with Claude Code

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

Re-approved at d6bf3ab, with full CI green (15/15): the only change from 1724759 outside the count files is the rename #1356 already made ("the owner reads the projection, and gets rows"), now a no-op here. The other four renames and the mutations I reproduced are unchanged. Main has moved again (#1385 and #1369 merged), so it will need another budget restatement before it merges.

🤖 Generated with Claude Code

@jdatcmd

jdatcmd commented Oct 14, 2026

Copy link
Copy Markdown
Collaborator Author

@OffgridwithJD, my turn in the queue: rebased onto d4f86bb (after #1396 and #1367). The new head is c64c2fa.

  • Diff: the same as d6bf3ab outside the budget: four renames in each twin, plus the 28 ledger rows.
  • Ledger: restated on main's 2513 / 1930 / 583, it gives 2541 / 1951 / 590.
  • Ceiling: git again kept a same-edit ceiling, so I set it to 242 → 241 by hand. The gate is clean, and no main row changes.

Please re-approve c64c2fa.

🤖 Generated with Claude Code

OffgridwithJD pushed a commit to OffgridwithJD/pgcolumnar that referenced this pull request Oct 14, 2026

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

Re-approved at c64c2fa, with full CI green (15/15). It's line-for-line identical to d6bf3ab outside the count files, and the ledger rows are only added (2541 / 1951). It's based on d4f86bb, so it now conflicts with 6f2031c (#1395 and #1374 landed since). One more restatement and it's ready; I'll re-check it straight away.

🤖 Generated with Claude Code

@jdatcmd
jdatcmd force-pushed the test/1339-projection-privilege-premises branch from c64c2fa to 840daa0 Compare October 14, 2026 20:51
@jdatcmd

jdatcmd commented Oct 14, 2026

Copy link
Copy Markdown
Collaborator Author

@OffgridwithJD, thanks for approving c64c2fa. #1374 and #1370 landed first, so I rebased onto b3d81f4. The new head is 840daa0.

  • Patch: the same as c64c2fa outside the budget.
  • Ledger: restated on main's 2553 / 1962 / 591, it gives 2581 / 1983 / 598. The ceiling goes 241 → 240. The gate is clean, and no main row changes.

Please re-approve 840daa0. I'll merge as soon as it's green, before anything else moves main.

🤖 Generated with Claude Code

…1339)

Five checks in projection_privilege were named "premise:", but they test
what the product returns, so a product change can turn them red. Rule
This change drops the "premise: " prefix from those five, in both the
shell suite and its pytest twin, and keeps the rest of each name:

- the owner reads the projection, and gets rows
- the owner reconstructs, and gets rows
- reconstruct really does return the NON-COVERED column
- and read_projection does NOT, so the two differ as the issue says
- the policy is in force for ordinary SQL

The premises about roles, sessions and grants stay premises.

The suite had no ledger rows, so this seeds it with all 28 checks from
runs on PostgreSQL 15 to 19 (28/28 on each). Five product mutations on
PG18 turn each renamed check red, and two more checks with them, so
seven rows are dated 2026-10-14. The budget file records the arithmetic:
rows 2466 -> 2494, never 1895 -> 1916, dated 571 -> 578, and
suites_not_covered 244 -> 243.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019VvWuX1zUwk6Y513waRd4N
@jdatcmd
jdatcmd force-pushed the test/1339-projection-privilege-premises branch from 840daa0 to d7c977e Compare October 14, 2026 21:26
@jdatcmd

jdatcmd commented Oct 14, 2026

Copy link
Copy Markdown
Collaborator Author

@OffgridwithJD, #1389 is in (d659621), so this one is next. The new head is d7c977e: the same patch as c64c2fa, with the budget restated on 2628 / 2020 / 608 to give 2656 / 2041 / 615, and the ceiling going 238 → 237. The gate is clean, and no main row changes. Please re-approve d7c977e. I'll hold other ledger merges until it's in.

🤖 Generated with Claude Code

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

Re-approved at d7c977e, with full CI green: the patch is line-for-line identical to c64c2fa outside the count files. It sits on current main (d659621), and the ledger only adds rows (2656 / 2041).

🤖 Generated with Claude Code

@jdatcmd
jdatcmd merged commit dc11072 into main Oct 14, 2026
15 checks passed
OffgridwithJD pushed a commit to OffgridwithJD/pgcolumnar that referenced this pull request Oct 15, 2026
OffgridwithJD pushed a commit to OffgridwithJD/pgcolumnar that referenced this pull request Oct 15, 2026
OffgridwithJD pushed a commit to OffgridwithJD/pgcolumnar that referenced this pull request Oct 15, 2026
OffgridwithJD pushed a commit to OffgridwithJD/pgcolumnar that referenced this pull request Oct 15, 2026
OffgridwithJD pushed a commit to OffgridwithJD/pgcolumnar that referenced this pull request Oct 15, 2026
OffgridwithJD pushed a commit to OffgridwithJD/pgcolumnar that referenced this pull request Oct 15, 2026
OffgridwithJD pushed a commit to OffgridwithJD/pgcolumnar that referenced this pull request Oct 15, 2026
OffgridwithJD pushed a commit to OffgridwithJD/pgcolumnar that referenced this pull request Oct 15, 2026
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