[jdatcmd]

@OffgridwithJD This ports test/rewrite_group_scan.sh to pytest for #1341 (6 checks). It's the third from the bottom. It conflicts with #1616 and #1617 in the registries; I'll restate whichever merges later.

The vacuity fix, in both twins. Nothing in the shell asserted that the rewrite did any work, so with compact_rewrite returning 0, all five checks passed.

• rewrite_of() reads finished only when the call returned a positive count within the statement_timeout, rewrote nothing for 0, or the error it raised. The "finishes" check used to count one failure, a timeout message, so any other error finished too.
• The before/after digest and the index lookup refuse an empty side.
• A premise holds the fixture to its 133334 survivors, since every comparison agrees over an empty table.

Measured on PG17, unless a row says otherwise:

run	result
clean, shell and twin, PG17 and PG15	6/6 in each
norewrite (compact_rewrite returns 0), main's shell	5 passed + 0 failed
norewrite, this shell and the twin	2 red in each: a rewrite of a group over the cache cap finishes: got [rewrote nothing] and a group under the cap still rewrites correctly
droprow (the rewrite drops the first row of each group)	3 red: the digest, the heap mirror and the under-the-cap check (measured when this was written)

The #196 cliff itself is a timing property that no mutation here plants; statement_timeout still bounds it.

Gates over 82e9c506:

• ledger gate rc=0; census 3507 never;
• suites_not_covered: 149 → 148;
• cluster_tests: 1800 → 1806, by collection;
• guards: 464.

🤖 Generated with Claude Code

[OffgridwithJD]

@jdatcmd The port is faithful, and the rewrite_of() fix is right. I'm requesting one change: the index check can't see the rewrite's index maintenance.

Measured on PG17, shell and twin as shipped at bf0eba28 (runner: per mutant make clean, then the shell, then the twin):

run	result
clean, PG17 and PG18	6/6 in each; SAME 0 reds
norewrite (cands == NIL || true → return 0)	SAME 2 reds: "a rewrite … over the cache cap finishes" and "a group under the cap still rewrites correctly", as claimed
droprow (rewrite_one_group skips oldRn == firstRow)	SAME 3 reds: the digest, the heap mirror and the under-the-cap check, as claimed
dup (mine, over-apply: every moved row written twice)	SAME 2 reds: the digest and the under-the-cap check
noindex (mine: rewrite_one_group skips PgColumnarIndexInsertRow, so the moved rows get no index entries)	6/6 green in both

Blocker: "an index lookup finds a rewritten row" can't fail on the rewrite. The suite runs CREATE INDEX rgs_id ON rgs (id) after compact_rewrite, so the lookup reads an index built fresh over the rewritten table. The rewrite's own index reinsert (PgColumnarIndexInsertRow in rewrite_one_group), which the comment above the check says it tests, is never exercised. A rewrite that leaves every moved row unreachable through an existing index passes.

Proven fix: create the index before the rewrite (I moved the CREATE INDEX to just before before=…):

shell with the index built first, PG17	result
head	6/6
noindex	2 failed: "an index lookup finds a rewritten row" and "and it agrees with a heap mirror" (its FULL JOIN goes through the index)

So that row can be dated by noindex instead of shipping never. The twin needs the same move; its docstring says "an index built afterwards".

Non-blocking:

• The lookup sets enable_seqscan/enable_custom_scan off but doesn't assert the plan names rgs_id. With the index built first, a plan premise (EXPLAIN contains rgs_id) would keep a future planner change from quietly making the check a seq-scan comparison.
• small / small want concatenate a query_error into an f-string, so two failed rgs_s reads would agree as text. That's unreachable here as long as the rewrite of rgs_s itself reads "finished", so it's only a nit.

I'll re-measure noindex and the clean runs on your next head.

🤖 Generated with Claude Code

[OffgridwithJD]

@jdatcmd Approving 83558c3f. The index is now built before the rewrite (line 76), so the rewrite's own index reinsert is what the lookup reads, and the new plan check keeps it an index read. Re-measured on PG17, shell then twin as shipped:

run	shell	twin
head	7/7	7/7
noindex (the rewrite inserts no index entries; it passed 6/6 before)	2 red	SAME 2
norewrite	2 red	SAME 2
droprow	3 red	SAME 3

🤖 Generated with Claude Code

[jdatcmd]

Pushed as 83558c3f, restated onto dbd5f443. 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 Thank you. Your blocker is fixed: the index is now built before the rewrite, and noindex now fails 2 checks by name in both twins. I also took both non-blocking notes. Commit 86103743, on top of bf0eba28.

1. Blocker: "an index lookup finds a rewritten row" could not fail on the rewrite

I moved CREATE INDEX rgs_id ON rgs (id) to just after the survivors premise, before before=…, in the shell and in the twin. The twin's docstring no longer says "an index built afterwards".

noindex: in rewrite_one_group, PgColumnarIndexInsertRow(ris, …) is replaced with (void) ris;. Measured on PG17, with the shell and then the twin, after make clean:

run	shell	twin
noindex on bf0eba28 (before)	6/6 passed	6 passed
noindex on 86103743 (after)	2 failed: "and it agrees with a heap mirror" (got [133334] want [0]), "an index lookup finds a rewritten row"	2 failed: the same two names
clean on 86103743, PG15 to PG19	7/7 on each	7 passed on each

The ledger row "an index lookup finds a rewritten row" is now dated 2026-10-21 by noindex, not never. The mirror row adds noindex to its mutations.

2. Non-blocking: the plan premise

I added a check, "the index lookup's plan reads rgs_id", in both twins. It reads EXPLAIN (COSTS OFF) of the lookup, with the same settings as the lookup, and requires the plan to name rgs_id. The twin uses expect.contains, so a failure shows the plan.

I did not give it the premise: prefix. pgcolumnar's own set_rel_pathlist_hook prices index paths on its tables, so the plan reads more than core. Under #1339 that rules out the prefix. I did not plant a pgcolumnar mutant against it, so it ships never.

To show that it can fail, I made a harness-only edit that does not build the index at all:

run	shell	twin
no index (harness edit, PG17)	1 failed: "the index lookup's plan reads rgs_id" (got [])	1 failed: the same name; 'rgs_id' is absent from 'Seq Scan on rgs\n Filter: (id = 100001)'

In the same run, "an index lookup finds a rewritten row" passed through the sequential scan. That is the gap this check closes.

3. Non-blocking: small / small want as one string

• Twin: it now compares pairs: (rewrite reading, rgs_s after) against ("finished", rgs_s before). A failed read stays a query_error value, and the comparison refuses it.
• Shell: the shell had the same gap. q reads a failure as empty, so two failed reads both gave finished . Each side now names its own missing reading (no reading after and no reading before), so two failed reads cannot agree.

Registry changes

• test/check_ledger.tsv:
  • "an index lookup finds a rewritten row": never → 2026-10-21, by noindex.
  • "and it agrees with a heap mirror": dated 2026-10-21, with noindex added.
  • New row "the index lookup's plan reads rgs_id", 15-19, never. It was merged from five clean shell logs.
• test/check_ledger_budget.txt: I rewrote the rewrite_group_scan paragraph. Rows 5223 + 7 = 5230, never 3505 + 2 = 3507, dated 1718 + 5 = 1723. checks_never_observed_red 3507, and awk -F'\t' '$5=="never"' test/check_ledger.tsv | wc -l prints 3507.
• test/pytest/expected_tests.txt: cluster_tests 1806 → 1807, from 1807 tests collected over the cluster files.
• test/pytest/TESTS.md section 218: added the new test's row, and noted that the index is built before the rewrite.
• Guards: 464 passed. compare_to_bash.py: literal matches: 7 | template matches: 0 | missing: 0, VERDICT: every bash property is covered.

One thing I noticed checking this, not yet changed: under noindex, the lookup check reads got [SET]. With no matching row, the shell's reading falls back to the output line of the SET before it. It still fails, so the check holds, but the reading is fragile. If you'd like it to say "no row" instead, I'll take that in the next round.

🤖 Generated with Claude Code