Port sorted_pathkeys to pytest, all 110 names (#432)

[OffgridwithJD]

test/sorted_pathkeys.sh pins one decision: when a columnar scan may hand the planner
PATHKEYS, the promise that its rows already arrive in a stated order. The planner then
drops the Sort above the scan and nothing downstream re-checks, so a wrong promise is
not a slow plan, it is wrong rows.

The port keeps the original's three shapes rather than reorganising by feature:

• CLAIM — the relation really is ordered, the Sort really does disappear.
• REFUSAL — something made the claim untrue (an append, an UPDATE, a rewrite, a
  collation change) and the Sort must come back.
• ANSWER — the rows themselves, against a heap table built from the same data.

An ANSWER arm is not a duplicate of its CLAIM arm. Dropping the Sort is only correct if
the rows arrive sorted anyway, and a plan check alone cannot say whether they did.

literal matches: 110 | template matches: 0 | missing: 0
VERDICT: every bash property is covered

One arm needed a cluster setting, not a workaround

pgcolumnar.parallel_copy prepares one transaction per worker, and
max_prepared_transactions cannot be raised without restarting the postmaster. The
default is 0, so asking for fewer workers does not help: any number is one too many.
pgc_cluster now sets it where it writes postgresql.conf, at the value lib.sh
gives this suite through PGC_EXTRA_CONF.

I wrote the refusal first — expect.cannot_run with the reason, and the suite declared
in INCOMPLETE — then measured it and rejected it, for two reasons:

1. It loses three of the bash suite's names outright. cannot_run records under the
   REASON CODE rather than under a name (compare_to_bash.py reads 5 of lib.sh's 8 check helpers, hiding 2 unported properties in hilbert_locality (#432) #1040 phase 0b), so those names cannot be
   emitted at all.
2. It turns pytest (cluster tests) red. An unrunnable check exits 67, that job runs
   pytest under set -euo pipefail, and no file in that half had ever produced one.
   Measured, with my file removed: the leg exits 0 today with 0 unrun. This file would
   have been the first to break it.

The arm now asserts pg_prepared_xacts is empty afterwards. A leaked prepared
transaction holds its locks until someone resolves it and the cluster is session-scoped,
so a leak would not fail this test — it would wedge every file that runs after it.

Two docs defects found and fixed here, both mine

A numbered section with no body. ## 37. test_iceberg_fdw.py reached main sitting
directly above ## 38., with the Iceberg body attached to the userinfo heading — my
#1066 merge put the new section into the gap between another heading and its body.
#1066's own arm cannot see this: it asks whether each file is NAMED by a numbered
heading, and a heading with no body is still a heading. test_docs_cover_the_corpus.py
now refuses a numbered section with no body, with a planted control, and the section
order matches the bodies again.

A wrapped name called fabricated. compare_to_bash.py's corpus arm asserts every
name its loop reader returns is text the file contains. A name too long for one line is
written as adjacent literals and Python joins them at parse time, so a joined name is
text the file contains but not text in src can find — this port has two. The arm exists
to catch a reader that CONSTRUCTS a name, so it now collapses the file's own
concatenation and keeps exactly that guarantee. Both directions are asserted: collapsing
rescues a split literal, and an f-string name still yields a {} template, which the
collapse does not rescue.

Verified

On /usr/local/pg16a in the audit container, both legs exactly as CI runs them:

guard leg    342 collected, 878 checks, 0 fail                rc=0
cluster leg  373 collected, 1025 checks, 0 fail, 0 unrun      rc=0
grader       110 literal, 0 template, 0 missing

Both counts in expected_tests.txt re-derived by collection on this tree, never by
adding a delta.

Mutation proof

Each mutation asserts it applied before its result is believed — a silent no-match
mutation is green and proves nothing.

mutation	result
M1 — the collation refusal removed	3 arms red: the collatable column, the collation ALTER, the domain/array
M2 — enable_sorted_pathkeys ignored	2 arms red: the off switch, and the planning-buffers control
restored (source byte-identical)	127 checks, 48 passed, 0 fail

Advances #432. I do not merge this.

🤖 Generated with Claude Code

https://claude.ai/code/session_012RSw4qMHS7ByE7PY8Ns4cs

[linuxhikerpm]

The port is one-for-one (110 literal names), and the two extras are real defects the coverage arms could not see: a numbered TESTS.md heading with no body, and a wrapped assertion name that in src called fabricated. Raising max_prepared_transactions in pgc_cluster matches what lib.sh already does via PGC_EXTRA_CONF; refusing the arm would have dropped three names and exited 67 under the cluster job. CI is green. Not merging.

[linuxhikerpm]

Follow-up 7fa0785 is docs only and puts the exit-67 / cluster-job consequence next to cannot_run, which is the page the next author will actually read. Prior approval still stands. Not merging.

[jdatcmd]

Adversarial review of the stack, since #1069 sits on this one.

Verified independently: sorted_pathkeys grades literal 110 | template 0 | missing 0, and projections on #1069 grades 75 | 0 | 0. Both clean.

The hazard is in how this merges, not in the code. git merge-base --is-ancestor confirms #1069 is a descendant of this branch, so:

• Do not merge this with --delete-branch. Deleting the base of a stacked PR auto-closes the child irreversibly — that has happened in this repo before. Retarget Port projections to pytest, all 75 names (#432) #1069 to main first, or merge without deleting.
• Merge the chain with merge commits, not rebase-merge. A rebase-merge rewrites the shas Port projections to pytest, all 75 names (#432) #1069 descends from, which orphans it.

Stacking it deliberately for deterministic TESTS.md numbering is the right call — both PRs would otherwise claim section 40, which is exactly #1024's collision. Worth noting that #1024's arm now catches that, so the stack is belt-and-braces rather than the only protection.

One thing I could not check without a build and am relying on you for: the mutation proofs (the #875 latch in both directions, the planner arms with got None want 'pc'). Flagging the boundary rather than implying I re-ran them.

[OffgridwithJD]

Adversarial self-review, at jd's request. I found a real defect in this PR and have fixed it in ef2dc71. It was in the half of the suite I defended in the body above, which is the part worth reading.

I do not approve my own PRs; this is a finding, not a sign-off.

The ANSWER half asserted order with the wrong instrument

Eight arms compared the columnar and heap results like this:

expect.text("same" if columnar == heap else "DIFFERENT", "same", name)

That is a hash comparison wearing a Python spelling. A failure says these two strings differ and nothing about which row — which is precisely what this layer's rows/ordered_rows exist to remove: "a mismatch names the position, where a hash mismatch only says two hashes differ." I removed the bash suite's pgc_set_hash for that reason in the body above, then reintroduced the same weakness by hand.

The grader cannot see this. It compares names, and all 110 names were present and correct. A port can carry every name and assert something weaker than its twin, and nothing in the parity tooling looks at strength — which is the argument for reading a port rather than trusting its verdict.

Switching to ordered_rows immediately caught a decorative arm

expect.ordered_rows refuses a sequence whose elements are all identical, because the reverse reads the same and an ordering claim about it cannot fail. One of the eight reddened on the spot:

and the first row matches heap: order cannot be observed in these sequences.
Every element is the same, so the reverse ordering is identical and the claim
asserts nothing beyond what rows() already asserts.

Its template is SELECT k FROM %T WHERE k IS NOT NULL ORDER BY k LIMIT 1. One row. That arm was in the ANSWER half — the half whose entire justification, in my own PR body, is that it catches a wrong ordering claim a plan check cannot — and it asserted nothing about ordering.

It is a legitimate value claim: the minimum under the ordering. So it now takes rows, which still names the position on a mismatch.

The fix declares orderedness rather than sniffing it

ANSWERS carries a third field saying whether a template can carry an ordering claim:

("SELECT k FROM %T WHERE k IS NOT NULL ORDER BY k LIMIT 1",
 "and the first row matches heap", False),

Declared, not decided at runtime from the data. Choosing the instrument by looking at the rows in front of you is exactly how an ordering claim silently becomes a value one, which is the failure the two instruments exist to keep apart.

Re-verified, not assumed

The parametrize arity changed, and the class-3 name reader parses those tables — so the change could have broken name extraction and scored itself green:

grader       110 literal | 0 template | 0 missing
file         127 checks, 48 passed, 0 fail
guard leg    342 collected, 0 fail   rc=0
cluster leg  406 collected, 0 fail, 0 unrun   rc=0

What I checked and did not find

• str() coercions (13 of them): all on scalars — a text field, a boolean cast, a collation name. Redundant, not weak. Left alone.
• Mutation coverage: M1 and M2 in the body still redden the same arms.
• The two remaining "same" if ... else forms: storage-id comparisons, scalar, correct as expect.text.

Merge note

#1069 is stacked on this. Do not merge with --delete-branch (it closes the child), and use a merge commit rather than rebase-merge, which would orphan it. Both branches were force-pushed with an explicit lease on the prior SHAs.

[linuxhikerpm]

Re-review on HEAD ef2dc712 (previous approval was 8b75f849). The two follow-ups belong: exit-67 is now documented next to cannot_run, and the ANSWER half uses ordered_rows with a declared third field so LIMIT 1 is a value claim (expect.rows) rather than a decorative ordering claim. That is the instrument the layer's own docstring asks for, and it caught a real no-op.

110 literal names, pgc_cluster sets max_prepared_transactions=8 to match lib.sh, empty-section and wrapped-name arms are real defects the old coverage could not see. CI 14/14 green. Not merging.

[linuxhikerpm]

Re-reviewed HEAD ef2dc712 after #1070 landed on main.

The port is still the right one: CLAIM/REFUSAL/ANSWER are paired, the LIMIT-1 arm uses expect.rows rather than a vacuous ordered_rows on a single identical row, and max_prepared_transactions is set in pgc_cluster so the parallel_copy names stay runnable. CI is green.

This PR now conflicts with main (CHANGELOG / TESTS.md / expected_tests from #1070). That is mechanical, not a product defect. Rebase before it can land; keep landing it before #1069.

[linuxhikerpm]

Re-reviewed HEAD 47a78e6d (merge of #1070). Same port as before: CLAIM/REFUSAL/ANSWER pairing, LIMIT-1 uses expect.rows, max_prepared_transactions in pgc_cluster. cluster_tests 373 is a collection re-derive, not a delta on 325. CI is green on the jobs that have finished.