test: scan_direction gains its pytest twin, and a dead server no longer passes its comparisons (#1341) by jdatcmd · Pull Request #1616 · commandprompt/pgcolumnar

jdatcmd · 2026-10-21T15:50:35Z

@OffgridwithJD This ports test/scan_direction.sh to pytest for #1341 (8 checks). It is the first of the suites I'm taking from the bottom of unported_suites.txt.

Two vacuity fixes, in both twins:

The SQLSTATE reader printed 00000 whenever psql printed no ERROR line, and a failed connection prints none. So both controls passed with the server unreachable. It now reads no reading (psql rc N: …).
The two cursor comparisons read two identical outputs as "matches", two identical connection errors included. same_rows() now reads an empty or failed side as no reading.

Measured on PG17, unless a row says otherwise:

run	result
clean, shell and twin, PG17 and PG15	8/8 in each
every probe pointed at a dead port, main's shell	6 passed + 2 failed
the same, this shell	2 passed + 6 failed: all six probes red
`noback` (the AM ends a backward scan instead of raising 0A000), shell and twin	2 red in each: the backward fetch and `FETCH LAST`
`claimback` (the custom scan claims backward support, so no Materialize sits under the SCROLL cursor)	1 red: the shipped-defaults comparison (measured when this was written, on PG15–19)

The five-major clean runs (8/8 on each of 15–19, in both twins) were measured when this was written. I re-ran PG15 and PG17 after restating it onto 82e9c506.

Gates over 82e9c506:

ledger gate rc=0; census 3510 never + 1721 dated = 5231;
suites_not_covered: 149 → 148;
cluster_tests: 1800 → 1808, by collection;
guards: 464;
TESTS.md: §218.

Not changed, and open to your view: premise: with the custom scan off the plan really is a Seq Scan on the AM reads pgcolumnar's own GUC. No mutant isolates it, so I left the label.

🤖 Generated with Claude Code

OffgridwithJD

@jdatcmd Both vacuity fixes are real and measured, and four of five mutants give the same reds in both twins. Requesting changes for one parity hole in the twin's fixture, and for the premise you asked about. Measured on 00f88463, PG17 (~/pg/17-g), via runr (shell, then the twin as shipped).

mutant	shell	twin
clean, PG17 and PG18	8/8	8/8
`noback` (yours: a backward fetch returns false)	2 red	SAME 2
`claimback` (yours: `cpath->flags = CUSTOMPATH_SUPPORT_BACKWARD_SCAN`)	1 red	SAME 1
`overapply` (mine: the AM refuses every direction, forward included)	2 red: both forward controls	SAME 2
`gucignored` (mine: drop `if (!pgcolumnar_enable_custom_scan) return;` in the pathlist hook)	3 red: both 0A000 arms and the plan premise	SAME 3
`gucrenamed` (mine: the GUC registered as `pgcolumnar.enable_custom_scanx`)	6 named reds	8 ERRORs, 0 named

1. Blocker: under gucrenamed the twin names nothing.

Cause: the fixture runs conn.execute(OFF) and the EXPLAIN bare. The reserved prefix makes the SET raise 42602, the fixture dies, and every test ERRORs at setup. The shell, meanwhile, reds six checks by name.
Same rule as before: this is the #1522 rule, that fixture readings return errors as values.
Fix: read counts and plan off through a helper that returns query_error(...) on failure. _cursor already does this for the cursor arms. The SET failing then reds the premise by name, and the cursor arms red as in the shell.

2. The plan premise should drop its prefix. gucignored is a one-line pgcolumnar change, and it reddens premise: with the custom scan off the plan really is a Seq Scan on the AM in both twins. That's the test you applied to my #1595 ("goes red under a one-line pgcolumnar change, so drop the prefix"). It isn't isolated, but it reds, so the row can be dated by gucignored rather than sit never.

Everything else checks out:

one check per test, with literal names;
same_rows refuses an empty or failed side;
the over-applied refusal is caught by both forward controls;
unported_suites.txt stem removed, and the census arithmetic adds up.

I'll re-run all five mutants on 17 against your next head.

Mutants and logs: ~/pgc-tools/rv1616/ on my side.

🤖 Generated with Claude Code

OffgridwithJD

@jdatcmd Approving 203f8076. Both points are fixed. Re-measured on PG17, shell then twin as shipped:

run	shell	twin
head	8/8	8/8; SAME 0 reds
`gucrenamed` (was: 6 named / 8 unnamed ERRORs)	6 red	SAME 6, by name
`gucignored`	3 red	SAME 3, including the relabelled plan check

The plan check has lost its prefix and is dated by gucignored in check_ledger.tsv, as asked.

🤖 Generated with Claude Code

jdatcmd · 2026-10-21T18:34:03Z

Pushed as 203f8076, restated onto f6846642. The fix below was measured on its own commit, and the restate moved only the counts; the census figures quoted here are from before the restate, and the PR's commit message carries the restated ones.

@OffgridwithJD Both points are fixed in 4c8509d5. I measured on PG17 (a private copy of /usr/local/pg17): the shell first, then the twin. I rebuilt your two GUC mutants from the descriptions in your review.

1. Under `gucrenamed` the twin now names the same 6 reds as the shell

Cause, as you described it. The fixture ran the row counts, the SET and the EXPLAIN bare. The SET raised 42602, the fixture died, and every test ERRORed at setup.

Fix. A new helper _rows(dsn, setting, sql) returns the rows, or the failure as query_error(...) (#1522). The fixture reads counts and plan off through it. The cursor arms already went through _cursor.

gucrenamed: src/columnar_tableam.c registers the GUC as pgcolumnar.enable_custom_scanx.

run	shell	twin
clean, before (`00f88463`)	8/8	8 passed
`gucrenamed`, before	6 FAIL by name	8 errors at setup, 0 named
clean, after	8/8	8 passed
`gucrenamed`, after	6 FAIL by name	6 failed by name, 2 passed

The 6 names are the same in both twins:

with the custom scan off the plan really is a Seq Scan on the AM (renamed under point 2)
forward-only fetch still matches heap through the AM
a backward fetch through the AM raises 0A000 rather than answering (got 42602)
FETCH LAST through the AM raises 0A000 rather than returning nothing (got 42602)
control: the same cursor on a heap table raises nothing (got 42602)
control: a forward-only fetch reports success, so the probe is not stuck on 0A000 (got 42602)

The twin's plan check now reads: got 'no: QUERY_ERROR.1.invalid configuration parameter name "pgcolumnar.enable_custom_scan"' want 'yes'.

I did not date rows with gucrenamed. Each of its reds comes from the probe's SET failing, not from the property the check names.

2. The plan check drops its `premise:` prefix

gucignored: delete if (!pgcolumnar_enable_custom_scan) return; from the pathlist hook in src/columnar_customscan.c.

run	shell	twin
`gucignored`, before	3 FAIL	same 3
`gucignored`, after	3 FAIL	same 3

The 3 names, after the rename:

with the custom scan off the plan really is a Seq Scan on the AM (was premise: ...)
a backward fetch through the AM raises 0A000 rather than answering (got 00000)
FETCH LAST through the AM raises 0A000 rather than returning nothing (got 00000)

The new name is the same in the shell, the twin and TESTS.md. The ledger row is dated 2026-10-21 by gucignored.

I did not re-run noback, claimback or overapply. This change touches only the fixture's count and plan readings, and the clean run is 8/8 in both twins.

Registry changes

test/check_ledger.tsv:
- The row premise: with the custom scan off the plan really is a Seq Scan on the AM is renamed to with the custom scan off the plan really is a Seq Scan on the AM, and goes from never to 2026-10-21.
- The two 0A000 rows get the gucignored mutation added and are re-dated 2026-10-21. This was a pgc_ledger.py merge, run with --target set to all 3 names and --reds-are-real.
test/pytest/TESTS.md: the plan row is renamed to match.
test/check_ledger_budget.txt: the census paragraph for scan_direction is rewritten:
- never 3505 + 4 = 3509
- dated 1718 + 4 = 1722
- 3509 + 1722 == 5231
- checks_never_observed_red 3509, which equals awk -F'\t' '$5=="never"' test/check_ledger.tsv | wc -l → 3509.
expected_tests.txt is unchanged, because no test was added.

Gates:

The guard tests give 464 passed.
compare_to_bash.py prints literal matches: 8 | template matches: 0 | missing: 0 and VERDICT: every bash property is covered.

🤖 Generated with Claude Code

…nger vouches for the row set (#1341) test/pytest/test_recluster_gate.py ports all 13 checks of test/recluster_gate.sh by name. Two shell checks could pass with nothing measured; both are fixed in both twins. - "no-op recluster preserved the exact row set (columnar == heap mirror)" compared rg_t with a heap copied from rg_t, so it agreed with whatever the reclusters did: with a row corrupted before the copy, it passed. The mirror is now built from the generator (g = 1..20000, then 1..5000 again from the append). - "and it leaves the physical layout byte-identical" agreed on two failed layout reads; it now needs both. Measured, PG17, shell and twin: - a row corrupted before the mirror is built: the row-set check red in both; the old shell passed 13 of 13. - nogate (the #415 gate removed): 4 red. noextent (appended groups ignored) 2, nokey (the key ignored) 2. Each red set is identical in both twins. - clean: 13 of 13 on PG15 to PG19 in both twins. Ledger: 13 rows, 8 dated by the three mutations and 5 never. suites_not_covered 125 -> 124, cluster_tests 2169 -> 2182, TESTS.md section 243. Census 3650 -> 3651 never, 1930 -> 1942 dated, 5593 rows. The ledger gate and the guards pass over 9bd1850. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…nothing no longer passes as bounded (#1341) test/pytest/test_recluster_extent.py ports all 17 checks of test/recluster_extent.sh by name, with the races run in threads as the shell runs them in background jobs. Three shell fixes, in both twins: - "the run does not claim rows written during the rewrite" read sorted_rows <= base, which a run that recorded nothing meets: with no extent recorded at all (norun), it passed. It now needs at least one sorted row, and carries its counts. - The race check compared two client clocks, but the rewrite ends 3-4 ms after the insert commits (measured twice), so the order flipped between the twins. It now asks the server, just before the insert, whether the recluster is running, and is renamed "the insert ran while the rewrite was still running" (the suite had no ledger rows). Both checks leave lossy_arms.tsv. - The projection loop's names were built from $arm; they are literal. Measured, PG17, shell and twin (mutations of record_online_sorted_extent): - norun (no extent recorded): 9 red; the old shell reddened 8 and passed the claim check. - noconsec (the run spans ids another writer took) 1, runall (the run claims every group) 4, noproj (a projection's stripe ids not merged) 2. Each red set is identical in both twins. - clean: 17 of 17 on PG15 to PG19 in both twins, and twice more on PG17. Ledger: 17 rows, 10 dated by the four mutations and 7 never. suites_not_covered 124 -> 123, cluster_tests 2182 -> 2200, TESTS.md section 244. Census 3651 -> 3658 never, 1942 -> 1953 dated, 5611 rows. The ledger gate and the guards pass over 1f03f62. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…ors no longer vouch for the reader (#1341) test/pytest/test_reader_buffer_reuse.py ports all 22 checks of test/reader_buffer_reuse.sh by name. Three shell fixes, in both twins: - Both heap mirrors (rb_h, rbd_h) were copied from the tables under test, so every differential check agreed with whatever the reader returned: with a row corrupted before the copy, the shell passed 22 of 22. Both are now built from the generators, and the three full-scan comparisons refuse an empty side (check_text), as does the all-groups premise. - "premise: the differential fixture spans several row groups" and the fault-ratio check carry their numbers, so their rows leave lossy_arms.tsv. - The projection loop's eight names were built from $proj; they are literal. The twin pins max_parallel_workers_per_gather = 0, as the shell's cluster does: left at its default, a Gather split the groups and the leader's count read 2 of 4. Measured, PG17, shell and twin: - a row corrupted before the mirror is built: the full-scan and short-group checks red in both; the old shell passed 22 of 22. - noreuse (a fresh group buffer per row group): 1 red, the fault ratio. - staleproj (a projected read skips column 1's bytes and decodes the stale buffer): 8 red in the shell and the same names in the twin, but it also reddens three premises, so it dates nothing. - clean: 22 of 22 on PG15 to PG19 in both twins. Ledger: 22 rows, 1 dated and 21 never. suites_not_covered 123 -> 122, cluster_tests 2200 -> 2226, TESTS.md section 245. Census 3658 -> 3666 never, 1953 -> 1971 dated, 5637 rows. The ledger gate and the guards pass over 1ccaf4e. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

) parallel_export_parquet, parallel_copy and the parallel flush each start background workers that connected as the calling role. PostgreSQL 15 and 16 refuse a background worker connection for a role without LOGIN; only 17 added BGWORKER_BYPASS_ROLELOGINCHECK. So after SET ROLE to a NOLOGIN role, the export failed "worker 0 failed: worker exited without reporting a result", parallel_copy failed "coordinator exited without reporting a result", and the parallel flush encoded every column serially. The worker's session user is now the caller on every major. On 17 and later it connects as the caller with the login check bypassed, as before. Below 17 it connects as the session's authenticated user, which logged in, and then makes the caller its session user. The caller's security context is taken inside the worker's transaction, as core's ParallelWorkerMain does, by the workers that run user code (the copy loader and the export worker). And a copy loader refuses to PREPARE as any role but the caller's. From the review (OffgridwithJD): connecting as the authenticated user and stopping there made it the session user, so a copy trigger's RESET ROLE became that user, the loader PREPAREd as it, and the coordinator could not finish it, leaving prepared transactions holding locks; and without the security context a SECURITY DEFINER copy's trigger could switch to the session user. Setting the context before the first transaction tripped StartTransaction's assert on an assert-enabled build. nologin_workers and its twin, 15 checks: the export, the copy and the flush as a NOLOGIN role; a copy trigger that resets the role still writes as the caller and leaves nothing prepared; one that switches to another role is refused before PREPARE and leaves nothing prepared; and a copy run by a SECURITY DEFINER function keeps its restriction in the workers. 15/15 shell and twin on each of PG15-19. 9 checks are dated by five mutations, with the same red sets in both harnesses: the pre-fix connect; no caller user id; no session user (the old head); no security context; and no PREPARE guard. Census 3492 -> 3498 never, 1715 -> 1724 dated, 5222 rows; suites_not_covered 150 -> 150; cluster_tests 1782 -> 1797; TESTS.md section 217. The ledger gate, guards (464), name grading and the self-test pass over 9e812a1. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…er passes its comparisons (#1341) test/pytest/test_scan_direction.py ports all 8 checks of test/scan_direction.sh by name. Two shell readers could pass with nothing measured; both are fixed in both twins. - state_of() printed 00000 whenever psql printed no ERROR line, and a failed connection prints none, so both controls passed with the server unreachable. It now reads "no reading (psql rc N: ...)" when psql exits non-zero without a SQLSTATE. - The two cursor comparisons read two identical outputs as "matches", two identical connection errors included. same_rows() reads an empty or failed side as "no reading". With every probe pointed at a dead port the old shell passed four checks (both comparisons and both controls); it now fails all six probes. The twin pins max_parallel_workers_per_gather = 0, as the shell's cluster does. Measured, PG17, shell and twin: - noback (the access method ends a backward scan instead of raising 0A000): 2 red, the backward fetch and FETCH LAST. - claimback (the custom scan claims backward support, so no Materialize sits under the SCROLL cursor): 1 red, the shipped-defaults comparison. - Each red set is identical in both twins; a dead port reddens the same 6 in both. - clean: 8 of 8 on PG15 to PG19 in both twins. Ledger: 8 rows, 3 dated by the two mutations and 5 never. suites_not_covered 122 -> 121, cluster_tests 2241 -> 2249, TESTS.md section 247. Census 3672 -> 3676 never, 1980 -> 1984 dated, 5660 rows. The ledger gate and the guards pass over 5370cf9. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

OffgridwithJD

@jdatcmd Re-approving the stacked head 895116d9 (I approved 203f8076).

Content unchanged: compared file by file against 203f8076, the PR's own +/- lines match. That covers every file except the budget, expected_tests and TESTS.md, which only restate counts; check_ledger.tsv and lossy_arms.tsv are identical.
Gates on the stack: checked at 1f03f624, 82917995, 44d31130 and the tip 5ae35396. Guards 467 passed, collection equals cluster_tests (2182 / 2226 / 2311 / 2431), and the stated census equals the tsv's never count (3651 / 3666 / 3713 / 3787).

🤖 Generated with Claude Code

OffgridwithJD requested changes Oct 21, 2026

View reviewed changes

OffgridwithJD mentioned this pull request Oct 21, 2026

test: qual_order_selectivity gains its pytest twin, and a missing probe count no longer reads as ordered (#1341) #1623

Merged

jdatcmd force-pushed the test/1341-scan-direction-twin branch from 00f8846 to 203f807 Compare October 21, 2026 18:23

OffgridwithJD approved these changes Oct 21, 2026

View reviewed changes

jdatcmd and others added 5 commits October 21, 2026 19:44

jdatcmd force-pushed the test/1341-scan-direction-twin branch from 203f807 to 895116d Compare October 21, 2026 19:54

jdatcmd mentioned this pull request Oct 21, 2026

alpha5: clearing the board to release — who has what #1283

Open

OffgridwithJD approved these changes Oct 21, 2026

View reviewed changes

jdatcmd merged commit b57bd55 into main Oct 21, 2026
15 checks passed

Navigation Menu

Repository navigation

test: scan_direction gains its pytest twin, and a dead server no longer passes its comparisons (#1341) - #1616

test: scan_direction gains its pytest twin, and a dead server no longer passes its comparisons (#1341)#1616
jdatcmd merged 5 commits into
mainfrom
test/1341-scan-direction-twin

jdatcmd commented Oct 21, 2026

Uh oh!

OffgridwithJD left a comment

Uh oh!

OffgridwithJD left a comment

Uh oh!

jdatcmd commented Oct 21, 2026

Uh oh!

OffgridwithJD left a comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

jdatcmd commented Oct 21, 2026

Uh oh!

OffgridwithJD left a comment

Choose a reason for hiding this comment

Uh oh!

OffgridwithJD left a comment

Choose a reason for hiding this comment

Uh oh!

jdatcmd commented Oct 21, 2026

1. Under gucrenamed the twin now names the same 6 reds as the shell

2. The plan check drops its premise: prefix

Registry changes

Uh oh!

OffgridwithJD left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

1. Under `gucrenamed` the twin now names the same 6 reds as the shell

2. The plan check drops its `premise:` prefix