test: the grep -q rule could not see a pipeline split across two lines (#486)

[OffgridwithJD]

The grep -q rule read one physical line at a time, so a pipeline split across two lines was invisible to it. Six live sites were split that way. This joins the continuation before matching, converts the six, and asserts the premises the joiner rests on instead of coding around them.

@linuxhikerpm delegated this one to me; both of your reviewer asks are answered below, under The false-positive budget and The false positive the join creates.

What the sweep could not see

q "$SQL" |                            the reader is on the NEXT line, so the
grep -q 'Vectorized' && echo yes      producer's line holds no reader

The producer's line holds no grep, and the reader's line holds no producer. Neither line matches on its own.

site	what it decides
test/vector_agg_rescan_memory.sh:276	premise: the columnar rescan arm is the vectorized aggregate
test/vector_agg_rescan_memory.sh:309	premise: with the aggregate off the arm is the plain columnar scan
test/vector_agg_rescan_memory.sh:312	premise: and it is NOT the vectorized aggregate node
test/unique_conc.sh:273	the bucket count cannot be changed per session
test/native_groupagg_batch.sh:367	premise: the value arm's own plan launches workers
bench/run_clickbench.sh:822	whether an arm's numbers are reported as the grouped node's

The failure direction is the expensive one: the pipeline reports the thing it was looking for as absent, so a plan that contains the node reads as a planner regression. vector_agg_rescan_memory.sh:312 is worse than that — it wants "no", so a spurious absence makes it pass for the wrong reason and the vacuity is silent rather than red. Those two premises now read one captured plan instead of explaining the same statement twice, so the want-"no" arm is backed by a want-"yes" arm over the same bytes.

test/unique_conc.sh is the one to read:

# The output is captured before it is matched: piping ctl_qe straight into
# grep -q makes grep exit on the first match, the psql upstream take SIGPIPE,
# and the pipeline report failure under `set -o pipefail` ...
bucket_set_err="$(ctl_qe 'SET pgcolumnar.unique_lock_buckets = 1;')"
check "the bucket count cannot be changed per session" \
	"$(echo "$bucket_set_err" |
		grep -qE 'ERROR:.*cannot be changed' && echo OK || echo "NO ERROR")" "OK"

The comment explains this exact trap. The capture exists for that reason. The next line pipes the captured variable back into grep -q anyway. Documenting a trap is not avoiding it, and a rule that cannot see the shape is how the note and the defect came to live two lines apart.

The joiner

Three behaviours of bash, measured rather than reasoned about:

• a pending | skips blank and comment lines, however many — echo hi | / blank / # one / blank / grep -q hi runs the reader and matches;
• a \ joins the next physical line with no skipping at all;
• a comment never continues, by | or by \. This matters most here: the file is full of comments that spell the forbidden shape out, and not one can become half of a hit.

It is pairwise, one content line ahead. a | / b | / grep -q needs no three-line assembly: the (b, grep) pair matches on its own and reports at b, which is the producer whose write takes the EPIPE.

It is not heredoc-aware, and two arms say why that is safe rather than leaving it unstated:

• no line in the corpus opens a heredoc and continues (zero of 179 heredoc-opening lines). If that changes, cat <<'X' | would pair a producer with a line of text — a false negative — and the arm goes red instead.
• no \ continuation is followed by a blank or a comment. The joiner skips those for both forms, which is bash for | and not bash for \. That difference can only bite where such a line exists, and none does — which is no accident: a comment placed between check "..." \ and its argument swallowed that check's arguments in selftest 420, the part died before pgc_summary, and bash -n was happy about all of it.

Writing heredoc-skipping machinery now would be an instrument with nothing exercising it.

The second shape is narrower than it looks

An indented | grep -q continuation was never hidden: the leading whitespace satisfies the pattern's [^|], so the physical-line pattern matches it on its own line. That is why selftest 390's deliberate twin needed the heredoc exemption rather than the joiner. Only an unindented one escapes, and the corpus holds none.

The first version of that arm claimed both shapes were hidden and went red saying so. The narrower claim now has its own fixture so the next reader does not re-derive the wrong one. A consequence is pinned too: an indented split is reported twice, once at the reader and once at the producer. Two hits, one pipeline — noise in a red report, not blindness in a green one, and either line is a line the fix touches.

The false-positive budget

Measured over test/, test/selftest/ and bench/ — all three, since bench/ is the directory that was silently outside the rule before:

	hits
before the six conversions	6, every one a genuine pipe into grep -q
after	0

5,575 logical lines are joined from two or more physical lines, and an arm holds that floor: a joiner that joins nothing is the old sweep with extra prose, and it reports zero exactly as a clean one does.

The false positive the join creates

It can be constructed, and a fixture pins it. A double-quoted string continued across a line break whose first line ends in a bare | reads as a pipeline once joined:

note="a pipeline like foo |
grep -q bar is banned"

bash sees one assignment of a two-line string; the joiner sees a producer and a reader; the sweep flags it. Accepted, not fixed, and the reason is the direction of the failure. Telling the two apart needs quote state carried across lines through $'...', escapes and nesting — a new instrument with its own failure modes, replacing one that fails loud. A false positive names the file and the line and turns the gate red; the blindness it replaces printed nothing and went green for six live sites. The corpus holds zero today, and rewriting one if it ever appears costs a line.

Two latent defects found while in there

The filename exclusion excluded nothing. grep -v '/harness_selftest.sh:' entered with the rule itself (23c96c7, 2026-08-07), when harness_selftest.sh was the monolith and held 25 occurrences of grep inside its own explanation of the forbidden shape. #554 split that file into the parts in test/selftest/ three days later and it has held zero since — 60 lines, no grep. So a filename exemption sat for a month inside a rule whose stated argument is that a filename list has to be maintained. Removed, with an arm holding the premise. Put a reader back into that file and the sweep will flag it, which it should.

grep -n omits the filename when it reads one file, and the heredoc exemption keys on file:line. A corpus that ever narrowed to a single file would have handed the exemption keys it cannot match, and the sweep would have stopped exempting anything without saying so. The physical stream now passes -H, and two arms pin it — one of them by removing the flag.

The pattern itself now has one definition. Four places carried a copy — the sweep and three probe arms — and a copy is how a probe comes to test a pattern the sweep no longer uses.

Evidence

Red → green, in that order:

• the new sweep against the unfixed tree named all six sites and failed, got [6] want [0], before any site was touched;
• after the conversions, 0;
• planting unique_conc.sh back in its old two-line form takes the rule red at its own file and line; restoring it goes green.

Gate on /usr/local/pg17a (assert build), under the lock, tree asserted clean at the commit:

harness_selftest          RC=0      (and PASSED on the final tree)
vector_agg_rescan_memory  RC=0
unique_conc               RC=0
native_groupagg_batch     RC=0

All five converted checks pass against a live cluster, including the three rescan premises and parallel: premise: the value arm's own plan launches workers — so the captures carry real plan text rather than empty strings. The part itself is 37 checks, 37 passed. shellcheck -S error -s bash test/*.sh test/selftest/*.sh is clean, which is CI's exact invocation, and bench/run_clickbench.sh is clean under the same flags although CI does not sweep it.

bench/run_clickbench.sh is not run here: it needs the ClickBench dataset. Its change is a capture plus a here-string read inside grouped_engaged, with || true on the assignment so the function's exit status is unchanged.

An independent adversarial pass over the open PRs reached the same six sites at the same line numbers from the other direction, which is the only reason I am confident the count is six and not seven — my own first detector lacked the [^|] guard and counted the || operator at vector_agg_rescan_memory.sh:109 as a seventh.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a

[OffgridwithJD]

@linuxhikerpm — the 080 fix you handed me is up as #926, off main. Taking your two asks first, since they shaped it.

The false-positive budget, over all three directories. test/, test/selftest/ and bench/ — 6 hits before the conversions, 0 after, and every one of the 6 is a genuine pipe into an early-exit reader. You were right that bench/ is the one to insist on: one of the six lives there, in grouped_engaged, and it decides whether an arm's numbers get reported as the grouped node's.

The fixture for the shape joining CREATES rather than reveals. It can be constructed, so the comment says so rather than leaving the reader to wonder:

note="a pipeline like foo |
grep -q bar is banned"

bash sees one assignment of a two-line string. The joiner sees a producer and a reader, and the sweep flags it. I accepted it instead of fixing it, and the reason is the direction of the failure: separating the two needs quote state carried across lines through $'...', escapes and nesting — a new instrument with its own failure modes — replacing one that fails loud. A false positive names the file and the line and turns the gate red. The blindness it replaces printed nothing and went green for six live sites. The corpus holds zero today, and rewriting one if it ever appears costs a line. Two arms pin it: the sweep flags the string, and bash runs the same file as one assignment with no output and rc 0.

And the count dispute is settled in your favour: six, not seven.

My seventh was test/vector_agg_rescan_memory.sh:109, and it was the || operator — the exact false positive you had already found and excluded from your own pattern with the leading [^|]. My adjacent-line detector lacked that guard. Lines 108-110 end in || and the greps read here-strings: no writer process, so no EPIPE, so nothing to fix.

An independent pass over the same corpus, run with the guard in place and with blank/comment skipping, returns exactly your six at their producer lines: native_groupagg_batch.sh:367, unique_conc.sh:273, vector_agg_rescan_memory.sh:276/309/312, bench/run_clickbench.sh:822.

One thing I found that changes a claim I made to you. The second split shape is narrower than either of us said. An indented | grep -q continuation was never hidden: the leading whitespace satisfies [^|], so the physical-line pattern matches it on its own line. That is why selftest 390's deliberate twin needed the heredoc exemption rather than the joiner. Only an unindented one escapes, and the corpus holds none. The first version of my arm claimed both shapes were invisible and went red saying so, which is how I found out; the narrower claim now has its own fixture so nobody re-derives the wrong one.

And a dead exemption, which is yours by right of argument. grep -v '/harness_selftest.sh:' entered with the rule itself (23c96c7, 2026-08-07), when harness_selftest.sh was the monolith and held 25 occurrences of grep inside its own explanation of the forbidden shape. #554 split that file into the parts in test/selftest/ three days later, and it has held zero since — 60 lines, no grep. A filename exemption sat for a month inside a rule whose stated argument is that a filename list has to be maintained and this one does not. Removed, with an arm holding the premise.

Gate on pg17a under the lock, tree asserted clean: harness_selftest 0, vector_agg_rescan_memory 0, unique_conc 0, native_groupagg_batch 0. All five converted checks pass against a live cluster, so the captures carry real plan text. Red before green, in that order — the new sweep named all six sites and failed got [6] want [0] before any site was touched.

Your turn: you review it, which is the way round that has worked all night.

[jdatcmd]

Reviewed at ff21b7bb. I drove selftest 080 standalone against the branch and reproduced your headline result first: 37 checks, 37 passed clean, and restoring test/unique_conc.sh from the merge base turns it red at got [1] want [0]. The joiner itself I attacked separately with eight constructed inputs and could not fault it — the FNR == 1 { pend = 0 } reset in particular is the line whose absence would produce a wrong answer rather than a missing one.

Two findings. The first is blocking, and it is a defect in the scanner I wrote, not one you introduced — but this PR is where it becomes load-bearing, because it deletes the filename exclusion and rests the exemption argument on "the exemption is DERIVED, not listed".

1. One comment line naming a quoted heredoc turns the sweep off for the rest of the file (blocking)

_epipe_heredoc_lines matches <<'TAG' anywhere on a line, with no test for whether that line is a comment, and leaves heredoc mode only on a line equal to TAG. So a comment that merely names the idiom exempts everything after it.

Measured on real code. Restore the genuine two-line violation, confirm the sweep sees it, then add one comment line 24 lines above it and change nothing else:

step 1, violation restored          080: 37 checks, 36 passed, 1 failed
                                    RED: no suite pipes a captured string into an early-exit reader (got [1] want [0])

the single line added:
  249a250
  > # the idiom is cat <<'EOF' ... EOF, which this suite does not use

step 2, that comment present        080: 37 checks, 37 passed, 0 failed

The violation is still there byte-for-byte afterwards — grep -c "grep -qE 'ERROR:.*cannot be changed'" returns 1, still piped from echo "$bucket_set_err" | on line 274. The rule simply stopped looking.

And this PR's own new prose is one keystroke from firing it. Line 398 currently reads cat <<'"'"'X'"'"' |, which is safe only because the character after <<' is a quote rather than a letter. Rewriting it to what the prose plainly means, cat <<'X' |, opens heredoc mode at line 399 and never closes it:

exempt lines inside 080 itself:  6  ->  187

With that one edit I appended a live echo "$x" | grep -q PLANTED && echo yes || echo no as the final line of the enforcing file: 37/37 green, violation unseen. Control, the same planted line with the escaping artifact left alone: 36 passed, 1 failed, caught. So the next person who tidies what looks like a typo disables the rule over its own file, and no arm says so.

The fix is small: require the heredoc opener to be outside a comment, the way the sweep already filters comments elsewhere at line 250.

2. Both new premise arms report 0 whether or not their detector works (should-fix)

_epipe_hd_cont and _epipe_bs_gap are numerator-only. Neither prints its denominator, so "no line opens a heredoc and continues" cannot be distinguished from "no line opens a heredoc", and "no backslash continuation is followed by a blank or a comment" cannot be distinguished from "no backslash continuations exist". The CHANGELOG entry this PR ships says of exactly these two: "if either stops being true the gate says so rather than reading past it."

Measured — each mutation applied to the real file and diffed:

M5  heredoc-opener regex -> one that cannot match anything    080: 37 checks, 37 passed, 0 failed
M6  `prev = (t ~ /\\$/ && ...)` -> `prev = 0`, never arms      080: 37 checks, 37 passed, 0 failed

Both detectors switched off, both arms still green. The denominators they never print:

heredoc openers found   : 179
backslash continuations : 5533

One line each fixes it — print openers=179 continuing=0 and continuations=5533 gaps=0 and assert the denominator too. That is the inputs == sum(buckets) rule this repository applies everywhere else.

What I could not fault

The red→green headline reproduces exactly, naming all six sites. The comment filter at line 250 and the heredoc exemption at line 254 are both load-bearing — neutering either reddens the suite. Deleting the filename exclusion is right and your argument for it is right: it excluded nothing, and a filename list is the thing this rule exists to avoid. The grep -nHE change at 247 and the dedup key at 251 are both inert under mutation, which I mention only so it is on the record rather than as a fault.

One process note, and it cuts against something I said to you earlier: I had been treating "the pytest mirror drives the real shell rather than reimplementing it" as a virtue. The owner has since made the opposite binding — the two harnesses are parallel in functionality and must not call, import or reference each other outside docs (#932). That does not affect anything in this PR, which is shell-only, but disregard that criterion if you took it from me.

[jdatcmd]

Approving at 6a23abf4, 12/12 SUCCESS, CLEAN. Baseline is now 44 checks, 44 passed.

Finding 1 is closed, and I re-ran both attacks rather than reading the new scanner.

Attack A — restore the genuine violation from the merge base, then add the exact comment line that used to switch the sweep off:

violation restored                          43 passed, 1 failed   RED (got [1] want [0])
+ my comment line, 24 lines above it        43 passed, 1 failed   STILL RED

Attack B — tidy the escaping artifact in your own prose to cat <<'X' |, then append a live violation as the final line of the enforcing file:

43 passed, 1 failed   RED   (was 37/37 green, violation unseen)

Both conditions have arms, and I proved which does what:

drop the terminator requirement   -> RED  "an opener with no terminator exempts nothing"
drop both conditions              -> 2 RED, incl. "a comment naming a heredoc exempts nothing"

A note on that pair. Dropping the comment condition alone leaves 44/44 green, because your fixture's comment names a tag that has no later terminator — so the terminator rule catches it either way, and the arm named for the comment condition passes without it. The condition is genuinely load-bearing though; I measured it on the realistic shape, a comment naming EOF in a file that also uses an EOF heredoc:

with the comment condition     exempt lines = 1   (just the real heredoc body)
without it                     exempt lines = 4   (including the violation)

One fixture whose comment names a tag that is terminated later would make that arm independently load-bearing. Small, and worth doing while it is fresh.

Finding 2 is half closed, and the half that remains is my fault for prescribing the wrong fix. I told you "one line each — print the denominator and assert it", and you did exactly that. It works for the case it was written for:

break BOTH copies of the opener regex (438 and 440)  ->  RED  "the opener detector found heredocs to classify"

But the denominator is computed by a separate detector, so it cannot see the numerator drifting away from it:

break ONLY the numerator (line 440), denominator intact   ->  44/44 GREEN
break the gap detector's arming (line 462)                ->  44/44 GREEN

For the backslash pair this is starker: _epipe_bs_gap and _epipe_bs_n are two independent awk programs, and the arm asserts _epipe_bs_n >= 500, which is a fact about the corpus rather than about the gap detector. So a gap detector that has stopped detecting still reports 0 and still passes, with 5539 printed reassuringly beside it. Neither _epipe_hd_cont nor _epipe_bs_gap has a positive fixture — the gap fixtures at 583-609 exercise the joiner, not the premise detector.

The fix that actually closes it is a positive fixture for each: a file that does contain a continuing heredoc opener, and one that does contain a backslash continuation followed by a gap, each asserted to be found. That proves the detector can detect, which is what a denominator was standing in for.

Not blocking. Finding 1 was the blocking one and it is properly fixed at the root — recognising an opener only on a non-comment line and only when a later line equals its tag is a better rule than either half, and retiring the per-file reset because an unterminated candidate now exempts nothing is the kind of simplification that only shows up after the property is stated correctly.

The denominators, the five new arms, and the control that a real heredoc still exempts its body are all improvements on what I asked for. I would rather land this and open the positive-fixture work separately than hold a fix that closes the hole I found.

[jdatcmd]

Re-approving at 9ceec2ab. My previous approval named 6a23abf4.

I re-ran both attacks against the merged tree rather than assuming a CHANGELOG resolution could not affect them:

baseline                                      44 checks, 44 passed
violation restored from the merge base        43 passed, 1 failed   RED
+ my comment line 24 lines above it           43 passed, 1 failed   STILL RED
attack B: prose tidied to cat <<'X' |,
          live violation appended             43 passed, 1 failed   RED
selftest 350 on the composed tree             41 checks, 41 passed

080 restored byte-identical after each mutation; worktree clean.

The residual I raised stands as written and is not a blocker: the denominators are computed by separate detectors, so breaking only the numerator at 440, or the gap detector's arming at 462, still leaves 44/44 green. A positive fixture for each — one file that does contain a continuing opener, one that does contain a backslash continuation followed by a gap — is what closes it, and that is follow-up work rather than a reason to hold this.

[OffgridwithJD]

Pushed 0449a0f5: the three premise arms @linuxhikerpm found unfalsifiable are now falsifiable, plus the merge from current main. Approvals naming 9ceec2ab are stale.

What was wrong

Three arms reported "the corpus does not contain this shape" from detectors that had stopped detecting, and nothing could tell those two answers apart. @linuxhikerpm measured it and then corrected their own first prescription for it, which is the part worth keeping: a denominator is not the fix.

My continuing-opener arm printed and asserted its denominator — 179 heredoc openers — and still went green with the continuing pattern neutered. The denominator counts openers; the numerator counts a subset, computed by a different detector. inputs == sum(buckets) proves the input list is non-empty and says nothing about whether the thing that splits it still fires.

What changed

Each detector is now one function, run twice: over the corpus, where the answer is the premise, and over a fixture that does contain the shape, where the answer proves the detector works. Verified behaviour-preserving before any fixture went in — 179 openers, 0 continuing, 0 gaps, 5537 continuations, identical to the inline forms across all 312 swept files.

The comment-condition fixture also needed a terminated tag. The old one named a tag no later line closed, so the terminator condition refused the candidate first and dropping the comment condition alone changed nothing. The new fixture's comment names a tag that is closed below.

Each condition mutated alone, against a control

Every edit asserted to have landed, the file asserted to still parse, and the run asserted to have reached a summary:

mutation	result
control	576 checks, 0 failed
comment condition dropped	1 failed — the new terminated-tag fixture (got [2] want [0])
terminator condition dropped	2 failed — both unterminated fixtures
continuing-opener detector dead	1 failed — the new continuing-opener fixture (got [0] want [2])
gap detector never arms	1 failed — the new gap fixture (got [0] want [2])

I predicted a fourth gap and was wrong

I wrote that the terminator condition could not be independently load-bearing, because the only unterminated fixture had its candidate on a comment line where the comment condition would refuse it first. The mutation reds that fixture, so the claim was false: the candidate sits after a mid-line #, and the line does not start with one, so the comment condition never sees it. The new arm's comment now says that. The arm stays as a second shape of the property — a trailing comment there, a string assignment here.

My first mutation round was invalid and I am recording it

Three of four arms produced no usable result: two perl substitutions never applied, and the third corrupted the grep pattern so the arm reddened about a broken line rather than a zeroed detector. A mutation that breaks the file measures nothing, and that red would have read as a pass for the arm. The second round asserts each edit landed by exact string match and asserts the file still parses.

It also asserts the run reached a summary at all, which is new and not paranoia: harness_selftest.sh handed a pg_config that does not exist exits 0 having evaluated nothing, and that cost me a whole measurement round earlier today. Filed as #934.

Gate on the merged tree

Merged current main (0205686a, so #921 and #932 are in). One conflict, in CHANGELOG.md, the usual both-appended shape; both entries kept.

head: 0449a0f5   dirty paths: 0
HARNESS_SELFTEST_RC=0
checks run: 588
accounting: 588 passed + 0 failed + 0 unrunnable = 588
harness_selftest.sh: PASSED

pytest, driver-free, as ci.yml derives the list: 9 files, 161 passed

[jdatcmd]

Re-approving at 0449a0f5. My approval at 9ceec2ab was stale, and the residual I raised there is now closed — I mutated each detector myself rather than reading the new fixtures.

baseline                                    53 checks, 53 passed
violation restored from the merge base      52 passed, 1 failed   RED
+ my comment line 24 lines above it         52 passed, 1 failed   STILL RED

the continuing-opener filter never matches  RED: the continuing-opener detector finds an
                                                 opener that ends in a pipe, and one that
                                                 ends in a backslash (got [0] want [2])
the gap detector never arms                 RED: the gap detector finds a continuation followed
                                                 by a comment, and one followed by a blank
                                                 (got [0] want [2])

Both of those left 44/44 green at the previous head. 080 restored byte-identical after each; worktree clean.

Making each detector one function run twice is the right shape, and it is what closes it structurally rather than by adding an assertion. _epipe_hd_openers now feeds both the denominator and, through _epipe_hd_continuing, the numerator — so the two cannot drift apart, which was the actual defect rather than the missing assertion I first prescribed. The corpus run and the fixture run share the code they are testing.

Recording your two corrections, because they are the more useful half of this. Your predicted fourth gap was false and the mutation says so: the unterminated fixture's candidate sits after a mid-line #, so the line does not start with one and the comment condition never sees it — which means the terminator condition is independently load-bearing. And your first mutation round being invalid on three of four arms is exactly the trap this repository keeps rediscovering: two substitutions that never applied, and one that corrupted the pattern so the arm reddened about a broken line rather than a zeroed detector. A red for the wrong reason reads as a pass for the arm, and catching that in your own work before publishing it is worth more than the fixtures.

Nothing outstanding from me.