From ff21b7bbe81c6be04f0f70da75b0fffbb9707cae Mon Sep 17 00:00:00 2001 From: OffgridwithJD Date: Thu, 10 Sep 2026 15:52:35 +0000 Subject: [PATCH 1/3] test: the grep -q rule could not see a pipeline split across two lines (#486) The sweep read one physical line at a time, so `producer |` on one line and `grep -q PATTERN` on the next was invisible to it: the producer's line holds no reader, and the reader's line holds no producer. Six live sites were written that way and the rule read past all six. test/vector_agg_rescan_memory.sh 3 sites, all plan premises test/unique_conc.sh 1 test/native_groupagg_batch.sh 1 bench/run_clickbench.sh 1 Each one answers a premise that decides whether a whole arm measures what it claims to, and the failure direction is the expensive one: the pipeline reports the thing it was looking for as ABSENT, so a plan that contains the vectorized aggregate reads as a planner regression. One of the three in vector_agg_rescan_memory.sh is worse than that -- "premise: and it is NOT the vectorized aggregate node" WANTS "no", so a spurious absence makes it pass for the wrong reason and the vacuity is silent rather than red. unique_conc.sh is the one to read. The comment directly above it explains this exact trap and captures the output into a variable for that reason, and the next line pipes that variable 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. WHAT THE JOINER DOES, and what it deliberately does not. Three behaviours of bash were measured rather than assumed: a pending `|` skips blank AND comment lines, however many a `\` joins the next physical line, with no skipping at all a comment never continues, by `|` or by `\` It is pairwise, one content line ahead, because `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 premises say why that is safe instead of leaving it unstated. No line in the corpus opens a heredoc and also continues (zero of 179 heredoc-opening lines), and no `\` continuation is followed by a blank or a comment. Both are arms. If either stops being true the gate says so, rather than the joiner walking into a heredoc body and pairing a producer with a line of text. Writing that 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. 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 is pinned by its own fixture so the next reader does not re-derive the wrong one. ONE FALSE POSITIVE IS ACCEPTED, with a fixture. A double-quoted string continued across a line break whose first line ends in a bare `|` reads as a pipeline once joined. Telling the two apart needs quote state carried 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. A SECOND LATENT DEFECT went with it. The physical stream now passes -H, because grep 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. Two arms pin it, one of them by removing the flag. THE DEAD FILENAME EXCLUSION IS GONE. `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, so the exclusion has excluded nothing for a month -- inside a rule whose stated argument is that a filename list has to be maintained and this one does not. An arm now holds the premise the removal rests on. Put a reader back into that file and the sweep will flag it, which it should. 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. MEASURED false-positive budget over test/, test/selftest/ AND bench/: 0 hits after the six conversions, 6 before, every one of the 6 a genuine pipe into grep -q 5,575 logical lines joined from two or more physical lines 37 checks in the part, 37 passed red -> green in that order: the new sweep reported the six sites and named them before any site was touched planting unique_conc.sh back in its old form takes the rule red at its file and line; restoring it goes green gate on pg17a (assert build), under the lock, tree clean: harness_selftest 0, vector_agg_rescan_memory 0, unique_conc 0, native_groupagg_batch 0, and all five converted checks pass against a live cluster Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a --- CHANGELOG.md | 37 ++ bench/run_clickbench.sh | 11 +- test/native_groupagg_batch.sh | 4 +- .../080-no-suite-pipes-a-captured-string.sh | 316 +++++++++++++++++- test/unique_conc.sh | 3 +- test/vector_agg_rescan_memory.sh | 21 +- 6 files changed, 371 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ee62d1c1..03d525f4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -160,6 +160,43 @@ true until the next version shipped. ### Fixed +- The sweep that forbids piping a captured string into `grep -q` now joins a + pipeline split across two lines, and the six suites that had split one are + fixed (#486). + + The sweep read one physical line at a time. `producer |` on one line with + `grep -q PATTERN` on the next was therefore invisible to it: the producer's line + holds no reader, and the reader's line holds no producer. Six live sites were + written that way -- three in `test/vector_agg_rescan_memory.sh`, one each in + `test/unique_conc.sh`, `test/native_groupagg_batch.sh` and + `bench/run_clickbench.sh`. Every one of them answers a premise that decides + whether a whole arm measures what it claims to, and the failure direction is the + expensive one: the pipeline reports the thing it was looking for as ABSENT, so a + plan that contains the vectorized aggregate reads as a planner regression. + + `test/unique_conc.sh` is the one to read. The comment directly above it explains + this exact trap, and captures the output into a variable for that reason. The + next line pipes that variable into `grep -q` anyway. + + The sweep now builds logical lines before it matches, and applies one pattern to + both the physical and the joined stream. Three behaviours of bash were measured + rather than assumed: a pending `|` skips blank and comment lines, a `\` joins the + next physical line with no skipping, and a comment never continues at all. Two + premises the joiner rests on are asserted instead of coded around -- no line + opens a heredoc and also continues, and no `\` continuation is followed by a + blank or a comment -- so if either stops being true the gate says so rather than + reading past it. + + One false positive is accepted, 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 the two lines are joined. It fails loud, where the blindness it + replaces failed silent. A second latent defect went with it -- the physical + stream now passes `-H`, because `grep -n` omits the filename when it reads a + single file and the heredoc exemption keys on `file:line`. + + Planting any one of the six sites back in its old form takes the rule red and + names the file and the line. + - `ALTER TABLE ... RENAME COLUMN` now carries the new name into `pgcolumnar.projection_declaration`, for the named relation and for every inheritance descendant, including a `PARTITION OF` child (#888). diff --git a/bench/run_clickbench.sh b/bench/run_clickbench.sh index 7fb3da85..9b61bb77 100755 --- a/bench/run_clickbench.sh +++ b/bench/run_clickbench.sh @@ -815,12 +815,15 @@ run_one() { # run_one # difference in the table means nothing about that node unless it engaged, and on # this dataset it usually does not (#369). grouped_engaged() { # grouped_engaged - local arm="$1" tbl + local arm="$1" tbl plan tbl=$(arm_table "$arm") - env "$BINDIR/psql" -h /tmp -p "$CB_PORT" -U postgres -d clickbench -X -At \ + # Captured, then read from a here-string (#486). A plan this size is well past + # the point where the writer loses the race, and the answer this helper returns + # decides whether a whole arm's numbers are reported as the grouped node's. + plan="$(env "$BINDIR/psql" -h /tmp -p "$CB_PORT" -U postgres -d clickbench -X -At \ -c "$(arm_settings "$arm")" \ - -c "EXPLAIN (COSTS OFF) ${2//FROM hits/FROM $tbl}" 2>&1 | - grep -qi 'Vectorized Group Keys' && echo yes || echo no + -c "EXPLAIN (COSTS OFF) ${2//FROM hits/FROM $tbl}" 2>&1)" || true + grep -qi 'Vectorized Group Keys' <<<"$plan" && echo yes || echo no } declare -A COLD HOT ERRS WARMSPREAD diff --git a/test/native_groupagg_batch.sh b/test/native_groupagg_batch.sh index cdf3cfb8..44662439 100755 --- a/test/native_groupagg_batch.sh +++ b/test/native_groupagg_batch.sh @@ -363,9 +363,9 @@ check_text "parallel: and they equal the heap mirror" \ "$(q "SELECT k, count(*), sum(v) FROM gbb_h GROUP BY k ORDER BY k" | md5sum)" # And the premise those two need: the parallel arm really did run in parallel. # Without it both arms are the same serial plan and the comparison is vacuous. +gbb_par_plan="$(PGOPTIONS="$PAR_OPTS" q "$ANALYZE_PFX $Q_PAR")" check_text "parallel: premise: the value arm's own plan launches workers" \ - "$(PGOPTIONS="$PAR_OPTS" q "$ANALYZE_PFX $Q_PAR" | - grep -qiE 'Workers Launched: [1-9]' && echo yes || echo no)" yes + "$(grep -qiE 'Workers Launched: [1-9]' <<<"$gbb_par_plan" && echo yes || echo no)" yes # ---- a column added after some row groups: predicted yes, ran no ------------ # Same shape as #602 on the ungrouped node. The old row groups have no chunk for diff --git a/test/selftest/080-no-suite-pipes-a-captured-string.sh b/test/selftest/080-no-suite-pipes-a-captured-string.sh index e19fedbd..627de969 100644 --- a/test/selftest/080-no-suite-pipes-a-captured-string.sh +++ b/test/selftest/080-no-suite-pipes-a-captured-string.sh @@ -150,14 +150,106 @@ _epipe_hd="$(_epipe_heredoc_lines "${_epipe_globs[@]}" 2>/dev/null || true)" # rather than crossing a threshold, which refuted a clean pipe-capacity # hypothesis. Reasoning about "small enough" is how #473, #476 and selftest 350 # each survived, so the rule sweeps instead. +# THE PATTERN IS DEFINED ONCE. Four places used to carry a copy of this regex -- +# the sweep and three probe arms -- and a copy is how a probe comes to test a +# pattern the sweep no longer uses. The arms below read this variable, so they go +# red when the sweep's own pattern drifts rather than passing against the old one. +# # The leading [^|] excludes the `||` OPERATOR. Widening from the echo/printf # form lost that exclusion for free: `[ "$rc" = 124 ] || grep -q PAT <<<"$out"` # is a fallback branch reading a here-string -- no writer process, so no EPIPE -- # and the first version of the widened pattern flagged both fuzz suites for it. -_epipe_hits="$(grep -nE '[^|]\|[[:space:]]*grep[[:space:]]+-[a-zA-Z]*q' \ - "${_epipe_globs[@]}" 2>/dev/null \ - | grep -v '/harness_selftest.sh:' \ - | grep -vE '^[^:]+:[0-9]+:[[:space:]]*#' || true)" +_epipe_pat='[^|]\|[[:space:]]*grep[[:space:]]+-[a-zA-Z]*q' + +# A PIPELINE CAN BE SPLIT ACROSS LINES, AND A LINE-ORIENTED SWEEP IS BLIND TO IT. +# Two shapes, both one pipeline to bash, neither visible to a grep that reads one +# line at a time: +# +# q "$SQL" | the reader is on the NEXT line, so +# grep -q 'Vectorized' && echo yes the producer's line holds no reader +# +# sed 's/x//' "$1" \ +# | grep -qE 'pgc_summary' the reader's line BEGINS with the +# operator, and at column 1 there is +# no [^|] in front of it +# +# The first shape is invisible to a physical-line pattern, and six live sites were +# written that way: three in vector_agg_rescan_memory.sh, one each in +# unique_conc.sh, native_groupagg_batch.sh and bench/run_clickbench.sh. +# +# THE SECOND SHAPE IS NARROWER THAN IT LOOKS, and the measurement is recorded +# because the obvious reading is wrong. An INDENTED `| grep -q` continuation was +# never hidden: the leading whitespace satisfies [^|], so the physical pattern +# matches it on its own line, which 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 -- zero lines begin with `|` at column 1. Joining +# covers it anyway: the cost is nothing once the joiner exists, and "nobody writes +# it unindented" is a habit rather than a rule. unique_conc.sh is the one +# to read. The comment directly above it explains this exact trap, the output is +# already captured into a variable for that reason, and the next line pipes that +# variable into an early-exit reader 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. +# +# So the sweep joins the continuation and applies the SAME pattern to the joined +# text. Three decisions in the joiner, each measured rather than reasoned about: +# +# PAIRWISE, one content line ahead, not a full logical-line assembly. `a |` / +# `b |` / `grep -q` needs no three-line join: the (b, grep) pair matches on its +# own and reports at b, which is the producer whose write takes the EPIPE and so +# is where the fix goes. +# +# BLANK AND COMMENT LINES ARE SKIPPED, because bash skips them after a `|`. +# Measured: `echo hi |` / blank / `# one` / blank / `grep -q hi` runs the reader +# and reports the match. A joiner that stopped at the first blank line would be +# blind to exactly the shape someone creates by annotating a long pipeline. +# +# A COMMENT IS NEVER A PRODUCER. Measured both ways: a comment line ending in `\` +# does not continue into the next line, and a comment line ending in `|` does not +# make the next line its reader. This matters here more than anywhere -- the file +# is full of comments that spell the forbidden shape out, and not one of them can +# become half of a hit. +_epipe_joined_lines() { + awk ' + function trim(s) { gsub(/^[ \t]+|[ \t]+$/, "", s); return s } + function content(s) { return s !~ /^[ \t]*$/ && s !~ /^[ \t]*#/ } + # Reset per FILE, for the reason the heredoc scanner does: awk globals + # outlive the input boundary, so a file whose last line ends in a pipe + # would otherwise be joined to the first line of the file after it. + FNR == 1 { pend = 0 } + { + # A pending producer skips blanks and comments and stays pending, + # which is what bash does after a `|`. + if (pend && !content($0)) next + if (pend) { + printf "%s:%d:%s %s\n", FILENAME, pline, ptext, trim($0) + pend = 0 + } + # ...and the line that closed one may open the next. + if (!content($0)) next + t = $0; sub(/[ \t]+$/, "", t) + if (t ~ /\|$/ && t !~ /\|\|$/) { ptext = t; pline = FNR; pend = 1 } + else if (t ~ /\\$/) { sub(/\\$/, "", t); ptext = t; pline = FNR; pend = 1 } + } + ' "$@" +} +_epipe_logical="$(_epipe_joined_lines "${_epipe_globs[@]}" 2>/dev/null || true)" + +# ONE pattern, TWO streams: the physical lines as grep -n reports them, and the +# logical lines the joiner builds. Keyed on file:line, so a site that matches in +# both streams is one site rather than two. +_epipe_hits="$( + { + # -H, not -n alone: grep omits the filename when it reads ONE file, and + # the heredoc exemption below keys on file:line. A corpus that ever + # narrowed to a single file would hand the exemption keys it cannot + # match, and the sweep would silently stop exempting anything. + grep -nHE "$_epipe_pat" "${_epipe_globs[@]}" 2>/dev/null + printf '%s\n' "$_epipe_logical" | grep -E "$_epipe_pat" + } | grep . \ + | grep -vE '^[^:]+:[0-9]+:[[:space:]]*#' \ + | awk -F: '!seen[$1 ":" $2]++' || true)" + # Drop hits whose file:line is inside a quoted heredoc. if [ -n "$_epipe_hd" ] && [ -n "$_epipe_hits" ]; then _epipe_hits="$(printf '%s\n' "$_epipe_hits" | while IFS= read -r _eh; do @@ -179,6 +271,21 @@ check "no suite pipes a captured string into an early-exit reader" \ # a premise that recomputes its condition tests the world, not the code. _epipe_files="$(grep -lE 'grep' "${_epipe_globs[@]}" 2>/dev/null || true)" _epipe_scanned="$(printf '%s' "$_epipe_files" | grep -c . || true)" +# THE FILENAME EXCLUSION IS GONE, because it excluded nothing and this rule's own +# argument is that a filename list is the thing it exists to avoid. `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 +# this directory three days later, and it has held none since: 60 lines, zero. +# +# What remains is the DERIVED exemption -- a line inside a quoted heredoc is text, +# whatever file it sits in -- which is the form the comment above already argues +# for. The arm below keeps the removal honest. Put a reader back into +# harness_selftest.sh and the sweep will flag it, which it should: that file runs +# its pipelines like any other. +check "premise: the file the old filename exclusion named holds no reader to exclude" \ + "$(grep -c 'grep' "$TESTDIR/harness_selftest.sh" || true)" "0" + check "and the scan examined the suites rather than finding nothing to read" \ "$([ "${_epipe_scanned:-0}" -ge 20 ] && echo yes || echo "no (scanned $_epipe_scanned)")" "yes" @@ -249,7 +356,7 @@ _epipe_shape='x() { printf "%s" "$1" | grep -%s PATTERN; }' } > "$_epipe_probe" _epipe_probe_hd="$(_epipe_heredoc_lines "$_epipe_probe" 2>/dev/null || true)" check "the sweep's pattern sees both lines of the probe" \ - "$(grep -cE '[^|]\|[[:space:]]*grep[[:space:]]+-[a-zA-Z]*q' "$_epipe_probe")" "2" + "$(grep -cE "$_epipe_pat" "$_epipe_probe")" "2" check "and the heredoc exemption covers the one inside the heredoc, not the other" \ "$(printf '%s' "$_epipe_probe_hd" | grep -c ':3:' || true)" "1" check "and does not cover the live one above it" \ @@ -260,7 +367,7 @@ check "and does not cover the live one above it" \ _epipe_wide="$PGC_WORKDIR/epipe_wide.sh" printf 'ldd /bin/sh | grep -%s libc && echo yes || echo no\n' q > "$_epipe_wide" check "the sweep catches a producer that is neither echo nor printf" \ - "$(grep -cE '[^|]\|[[:space:]]*grep[[:space:]]+-[a-zA-Z]*q' "$_epipe_wide")" "1" + "$(grep -cE "$_epipe_pat" "$_epipe_wide")" "1" check "premise: and the old echo/printf pattern did NOT catch it" \ "$(grep -cE '(echo|printf)[^|]*\|[[:space:]]*grep -[a-zA-Z]*q' "$_epipe_wide")" "0" @@ -271,7 +378,202 @@ check "premise: and the old echo/printf pattern did NOT catch it" \ _epipe_oror="$PGC_WORKDIR/epipe_oror.sh" printf '[ "$rc" = 124 ] || grep -%s PAT <<<"$out"\n' q > "$_epipe_oror" check "the sweep does not mistake the || operator for a pipe" \ - "$(grep -cE '[^|]\|[[:space:]]*grep[[:space:]]+-[a-zA-Z]*q' "$_epipe_oror")" "0" + "$(grep -cE "$_epipe_pat" "$_epipe_oror")" "0" check "premise: and the line really does hold the reader it must not flag" \ "$(grep -c 'grep -q PAT' "$_epipe_oror")" "1" +# ---- the joiner itself, asserted --------------------------------------------- +# +# A sweep whose joiner joins nothing is the line-oriented sweep with extra prose, +# and it reports zero hits exactly as a clean one does. Measured at 5,559 logical +# lines built from two or more physical lines. A FLOOR, not a ceiling: the number +# grows with every continuation anyone writes, and a hand-written ceiling is what +# went red at 2,502 heredoc lines in a corpus that was entirely healthy. +_epipe_joins="$(printf '%s' "$_epipe_logical" | grep -c . || true)" +echo " epipe sweep: logical lines joined from two or more physical lines=$_epipe_joins" +check "premise: the joiner joined continuations, so the sweep is not still line-oriented" \ + "$([ "${_epipe_joins:-0}" -ge 500 ] && echo yes || echo "no ($_epipe_joins)")" "yes" + +# THE JOINER IS NOT HEREDOC-AWARE, and it does not need to be while no line that +# opens a heredoc also continues. `cat <<'"'"'X'"'"' |` puts the body on the next line, so +# the join would pair the producer with a line of TEXT instead of with its reader +# -- a false negative, which is the direction this whole rule exists to prevent. +# +# Measured: zero of the 179 heredoc-opening lines in the corpus end in a bare `|` +# or a `\`. So this arm is the premise INSTEAD OF the machinery. If it goes red, +# the joiner needs to skip to the terminator; writing that now would be an +# instrument with nothing exercising it, which is how twelve suites came to +# maintain a check counter that nothing read. +_epipe_hd_cont="$(grep -nE "(^|[^<])<<-?['\"]?[A-Za-z_]" "${_epipe_globs[@]}" 2>/dev/null \ + | grep -cE '([^|]\||\\)[[:space:]]*$' || true)" +check "premise: no line opens a heredoc AND continues, which is what lets the joiner ignore bodies" \ + "$_epipe_hd_cont" "0" + +# A `\` CONTINUATION JOINS THE NEXT PHYSICAL LINE, with no skipping: bash removes +# the backslash-newline before it tokenizes, so a comment on the following line +# comments out the rest of the command and a blank line ends it. The joiner skips +# blanks and comments for BOTH forms, which is bash for `|` and is NOT bash for +# `\`, and that difference can only produce a wrong answer where such a line +# exists. None does, and it is not luck: 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. The shape is a +# defect on its own, so the arm earns its keep whichever way it goes red. +_epipe_bs_gap="$(awk ' + FNR == 1 { prev = 0 } + { + if (prev && ($0 ~ /^[ \t]*$/ || $0 ~ /^[ \t]*#/)) print FILENAME ":" FNR + t = $0; sub(/[ \t]+$/, "", t) + prev = (t ~ /\\$/ && t !~ /^[ \t]*#/) + }' "${_epipe_globs[@]}" 2>/dev/null | grep -c . || true)" +check "premise: no backslash continuation is followed by a blank or a comment" \ + "$_epipe_bs_gap" "0" + +# ---- the two split shapes, as fixtures -------------------------------------- +# +# Assembled from fragments rather than written out, for the reason the probe above +# is: a literal here would be a site the sweep then has to exempt. +_epipe_two="$PGC_WORKDIR/epipe_twoline.sh" +{ + printf 'q "$SQL" %s\n' '|' + printf '\tgrep -%s PATTERN && echo yes || echo no\n' q +} > "$_epipe_two" +check "the sweep sees a pipeline split AFTER the pipe, which all six live sites were" \ + "$(_epipe_joined_lines "$_epipe_two" | grep -cE "$_epipe_pat" || true)" "1" +check "and reports it at the PRODUCER's line, which is the line that has to change" \ + "$(_epipe_joined_lines "$_epipe_two" | grep -E "$_epipe_pat" | cut -d: -f2)" "1" + +_epipe_bsplit="$PGC_WORKDIR/epipe_backslash.sh" +{ + printf 'sed %ss/x//%s "$1" \\\n' "'" "'" + printf '%s grep -%sE PATTERN && echo yes || echo no\n' '|' q +} > "$_epipe_bsplit" +check "and a pipeline split BEFORE the pipe, with the operator at column 1" \ + "$(_epipe_joined_lines "$_epipe_bsplit" | grep -cE "$_epipe_pat" || true)" "1" +check "premise: and the physical-line pattern alone saw neither of those two" \ + "$(grep -hcE "$_epipe_pat" "$_epipe_two" "$_epipe_bsplit" | awk '{ s += $1 } END { print s + 0 }')" "0" + +# THE SAME SPLIT, INDENTED, WAS NEVER INVISIBLE. This arm exists because the first +# version of the one above claimed both shapes were hidden and went red saying so: +# one tab in front of the reader is a [^|], so the physical pattern matches it +# unaided. Pinning the narrower claim keeps the next reader from re-deriving the +# wrong one. +_epipe_bsind="$PGC_WORKDIR/epipe_backslash_indented.sh" +{ + printf 'sed %ss/x//%s "$1" \\\n' "'" "'" + printf '\t%s grep -%sE PATTERN && echo yes || echo no\n' '|' q +} > "$_epipe_bsind" +check "premise: indent that same reader by a tab and the physical-line pattern sees it unaided" \ + "$(grep -cE "$_epipe_pat" "$_epipe_bsind" || true)" "1" +check "premise: the physical stream names the file even when it reads only one of them" \ + "$(grep -nHE "$_epipe_pat" "$_epipe_bsind" | cut -d: -f1)" "$_epipe_bsind" +check "premise: and without -H it reports a line number where the key wants a path" \ + "$(grep -nE "$_epipe_pat" "$_epipe_bsind" | cut -d: -f1)" "2" +# And the consequence, stated rather than left to be noticed: an indented split +# site is reported TWICE, once at the reader by the physical stream and once at the +# producer by the joiner. Two hits, one pipeline. That is noise in a red report, +# not blindness in a green one, and either line is a line the fix touches -- so it +# is pinned here instead of being suppressed with a third stream to maintain. +check "and an indented split is reported twice, at the reader and at the producer" \ + "$({ grep -nHE "$_epipe_pat" "$_epipe_bsind" + _epipe_joined_lines "$_epipe_bsind" | grep -E "$_epipe_pat" + } | awk -F: '!seen[$1 ":" $2]++' | grep -c . || true)" "2" + +# THE KEY IS EXERCISED, by the one shape that matches twice at the SAME line: a +# pipeline that matches on its own line and also ends in a pipe, so the join +# restates it. Without the key the sweep would count one site as two here. +_epipe_dup="$PGC_WORKDIR/epipe_dup.sh" +{ + printf 'echo "$a" %s grep -%s X %s\n' '|' q '|' + printf '\tgrep -%s Y\n' q +} > "$_epipe_dup" +check "premise: that shape matches in both streams at one line, which is what the key is for" \ + "$({ grep -nHE "$_epipe_pat" "$_epipe_dup" + _epipe_joined_lines "$_epipe_dup" | grep -E "$_epipe_pat" + } | grep -c . || true)" "2" +check "and the sweep counts it once, because a hit is keyed on file:line" \ + "$({ grep -nHE "$_epipe_pat" "$_epipe_dup" + _epipe_joined_lines "$_epipe_dup" | grep -E "$_epipe_pat" + } | awk -F: '!seen[$1 ":" $2]++' | grep -c . || true)" "1" + +# The gap form gets its own fixture because bash's behaviour here is the whole +# reason the joiner skips: a reader separated from its producer by blank and +# comment lines still runs. +_epipe_gap="$PGC_WORKDIR/epipe_gap.sh" +{ + printf 'q "$SQL" %s\n' '|' + printf '\n' + printf '\t# why this pipeline is shaped the way it is\n' + printf '\n' + printf '\tgrep -%s PATTERN && echo yes || echo no\n' q +} > "$_epipe_gap" +check "and it follows the pipe across blank and comment lines" \ + "$(_epipe_joined_lines "$_epipe_gap" | grep -cE "$_epipe_pat" || true)" "1" +# Run, not scanned, and deliberately NOT with an early-exit reader: `wc -c` reads +# to EOF, so this demonstration cannot take the EPIPE it is demonstrating around. +# Joined, the reader receives four bytes. Unjoined, it would be its own command +# reading /dev/null and the output would carry the producer's text instead. +_epipe_gap_demo="$PGC_WORKDIR/epipe_gap_demo.sh" +{ + printf "printf '%%s' abcd %s\n" '|' + printf '\n' + printf '\t# a comment in the middle of a pipeline\n' + printf '\n' + printf '\twc -c\n' +} > "$_epipe_gap_demo" +check "premise: bash joins a pipeline across blank and comment lines, which is why the joiner must" \ + "$(bash "$_epipe_gap_demo" "$_epipe_oror2" +check "the joiner does not mistake a two-line || fallback for a split pipeline" \ + "$(_epipe_joined_lines "$_epipe_oror2" | grep -cE "$_epipe_pat" || true)" "0" +check "premise: and its second line really does hold the reader it must not flag" \ + "$(grep -c 'grep -q PAT' "$_epipe_oror2")" "1" + +# A COMMENT IS NOT A PRODUCER, which is what stops the sweep being fed its own +# documentation. This file alone spells the forbidden shape out a dozen times. +_epipe_cmt="$PGC_WORKDIR/epipe_comment.sh" +{ + printf '# a comment line that ends in a pipe %s\n' '|' + printf '%s grep -%s PATTERN\n' '|' q +} > "$_epipe_cmt" +check "a comment ending in a pipe is not a producer" \ + "$(_epipe_joined_lines "$_epipe_cmt" | grep -cE "$_epipe_pat" || true)" "0" +check "premise: and bash agrees -- with no command to continue, the reader alone will not parse" \ + "$(bash -n "$_epipe_cmt" 2>&1 | grep -c 'syntax error' || true)" "1" + +# ---- the one false positive the JOIN CREATES, and why it is accepted -------- +# +# Every fixture above reveals a pipeline that was always there. This one is the +# other kind, and it can be constructed: a double-quoted string continued across +# a line break, whose first line happens to end in a bare `|`, reads as a +# pipeline once the two lines are concatenated. +# +# 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. +# +# ACCEPTED rather than 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 of these today, which is what the zero-hit arm +# above measures, and rewriting one if it ever appears costs a line. +_epipe_str="$PGC_WORKDIR/epipe_string.sh" +{ + printf 'note="a pipeline like foo %s\n' '|' + printf 'grep -%s bar is banned"\n' q +} > "$_epipe_str" +check "the join CREATES a hit on a two-line quoted string, the sweep's one false positive" \ + "$(_epipe_joined_lines "$_epipe_str" | grep -cE "$_epipe_pat" || true)" "1" +check "premise: and bash runs that file as one assignment -- no reader, no output, rc 0" \ + "$(bash "$_epipe_str" &1; echo "rc=$?")" "rc=0" diff --git a/test/unique_conc.sh b/test/unique_conc.sh index a09c03cb..5f9fbab5 100755 --- a/test/unique_conc.sh +++ b/test/unique_conc.sh @@ -270,8 +270,7 @@ send s2 "SET application_name='cc_s2';" # message was there, which turns this into a check that can never pass. 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" + "$(grep -qE 'ERROR:.*cannot be changed' <<<"$bucket_set_err" && echo OK || echo "NO ERROR")" "OK" check "the cluster runs the bucket count the suite needs" \ "$(ctl_q 'SHOW pgcolumnar.unique_lock_buckets;')" "100003" diff --git a/test/vector_agg_rescan_memory.sh b/test/vector_agg_rescan_memory.sh index f30e65ed..71951d08 100755 --- a/test/vector_agg_rescan_memory.sh +++ b/test/vector_agg_rescan_memory.sh @@ -272,9 +272,14 @@ rescan_slope() { # rescan_slope TABLE [GUC] -> bytes per rescan awk -v a="$hi" -v b="$lo" -v d="$(( RS_HI - RS_LO ))" 'BEGIN { printf "%d", ((a - b) * 1024) / d }' } # The columnar arm must really be OUR node, or it is measuring the plain scan. +# Captured first, then read from a here-string. Piping the plan straight into an +# early-exit reader answers "absent" for a plan that CONTAINS the node whenever +# the writer takes EPIPE first, and the failure direction is the expensive one: +# this premise would report the vectorized aggregate missing and send the reader +# after a planner regression that is not there (#486). +vam_plan_on="$(q "$GUC EXPLAIN (COSTS OFF) SELECT sum(s.c) FROM (SELECT i FROM vam_drv LIMIT 100) d, LATERAL (SELECT count(*) c FROM vam_rs WHERE v > d.i) s")" check_text "premise: the columnar rescan arm is the vectorized aggregate" \ - "$(q "$GUC EXPLAIN (COSTS OFF) SELECT sum(s.c) FROM (SELECT i FROM vam_drv LIMIT 100) d, LATERAL (SELECT count(*) c FROM vam_rs WHERE v > d.i) s" | - grep -q 'Columnar Vectorized Aggregates' && echo yes || echo no)" yes + "$(grep -q 'Columnar Vectorized Aggregates' <<<"$vam_plan_on" && echo yes || echo no)" yes vec_slope="$(rescan_slope vam_rs)" heap_slope="$(rescan_slope vam_heap)" echo " per-rescan growth: columnar vectorized aggregate = ${vec_slope:-unset} B, heap floor = ${heap_slope:-unset} B" @@ -305,12 +310,16 @@ check_num "the rescanned aggregate still returns the heap's answer" \ # next start rebuilds a fresh list and a fresh metadata struct per group into the # same context. Nothing reclaims the previous one. GUC_OFF="SET pgcolumnar.enable_ungrouped_vector_agg=off; SET pgcolumnar.enable_group_vectorization=off; SET enable_material=off;" +# ONE capture, read twice. Both premises are statements about the SAME plan, so +# explaining it twice was two chances for the answers to disagree as well as two +# pipelines that could take EPIPE. The second one is the dangerous half: it WANTS +# "no", so a spurious absence makes it pass for the wrong reason and the vacuity +# is silent rather than red. +vam_plan_off="$(q "$GUC_OFF EXPLAIN (COSTS OFF) SELECT sum(s.c) FROM (SELECT i FROM vam_drv LIMIT 100) d, LATERAL (SELECT count(*) c FROM vam_rs WHERE v > d.i) s")" check_text "premise: with the aggregate off the arm is the plain columnar scan" \ - "$(q "$GUC_OFF EXPLAIN (COSTS OFF) SELECT sum(s.c) FROM (SELECT i FROM vam_drv LIMIT 100) d, LATERAL (SELECT count(*) c FROM vam_rs WHERE v > d.i) s" | - grep -q 'Custom Scan (PgColumnarScan)' && echo yes || echo no)" yes + "$(grep -q 'Custom Scan (PgColumnarScan)' <<<"$vam_plan_off" && echo yes || echo no)" yes check_text "premise: and it is NOT the vectorized aggregate node" \ - "$(q "$GUC_OFF EXPLAIN (COSTS OFF) SELECT sum(s.c) FROM (SELECT i FROM vam_drv LIMIT 100) d, LATERAL (SELECT count(*) c FROM vam_rs WHERE v > d.i) s" | - grep -q 'Columnar Vectorized Aggregates' && echo yes || echo no)" no + "$(grep -q 'Columnar Vectorized Aggregates' <<<"$vam_plan_off" && echo yes || echo no)" no plain_slope="$(rescan_slope vam_rs "$GUC_OFF")" plain_heap="$(rescan_slope vam_heap "$GUC_OFF")" echo " per-rescan growth: plain columnar scan = ${plain_slope:-unset} B, heap floor = ${plain_heap:-unset} B" From 1e9367636e9effc4f5cce2c1b3d3031d259759cc Mon Sep 17 00:00:00 2001 From: OffgridwithJD Date: Thu, 10 Sep 2026 18:56:42 +0000 Subject: [PATCH 2/3] test: a comment naming a heredoc must not exempt the rest of the file, and both new premises must print their denominator (#486) @linuxhikerpm found two things. The first is a defect in the exemption scanner that predates this change, and this change is where it becomes load-bearing: it deletes the filename exclusion and rests the argument on the exemption being DERIVED rather than listed. 1. A COMMENT NAMING THE IDIOM EXEMPTED EVERYTHING AFTER IT. The scanner matched the opener anywhere on a line and left heredoc mode only on a line equal to the tag. Measured on real code: with the genuine two-line violation in unique_conc.sh restored, this part goes red; adding ONE comment line 24 lines above it -- changing nothing else -- took it back to 37 passed with the violation still there byte for byte. The rule stopped looking. And this file's own new prose was one keystroke away. Line 398 is written `cat <<'"'"'X'"'"' |`, safe only because the character after the quote is a quote; writing it plainly took the exempt-line count inside 080 from 6 to 187, after which a live violation appended as the last line went unseen. TWO CONDITIONS NOW, each measured. An opener is recognised only on a NON-COMMENT line, and only when a later line EQUALS its tag. The second is what stops a TRAILING comment doing the same thing -- the reported fixture names the tag on its own line -- and it retires the per-file reset: an unterminated candidate exempts nothing rather than leaking into the next file. Proof, in three steps: violation restored -> RED; comment added -> STILL RED (was 0 failures before this fix); `cat <<'X' |` written plainly in the prose -> exempt lines unchanged at 2505. Five arms pin it, including the control that a REAL heredoc still exempts its body -- without which the new arms would be satisfied by an exemption that never fires -- and one for the unterminated case. 2. BOTH NEW PREMISE ARMS WERE NUMERATOR-ONLY, which makes my own CHANGELOG sentence false: "if either stops being true the gate says so". They reported zero whether or not their detector worked. Measured: replacing the heredoc-opener pattern with one that cannot match left the arm green, and so did making the continuation detector never arm. The denominators are printed and asserted now -- 179 openers, 5,533 continuations -- which is the inputs == sum(buckets) rule the rest of this directory applies. Both mutations redden the new premise arms. MEASURED selftest 080 44 checks, 44 passed (37 before) harness_selftest 567 checks, 567 passed + 0 failed + 0 unrunnable, rc 0 shellcheck clean under CI's exact invocation the two detector mutations redden their own premise and nothing else Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a --- CHANGELOG.md | 25 ++++ .../080-no-suite-pipes-a-captured-string.sh | 129 +++++++++++++++--- 2 files changed, 132 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 03d525f4..b684b267 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -194,6 +194,31 @@ true until the next version shipped. stream now passes `-H`, because `grep -n` omits the filename when it reads a single file and the heredoc exemption keys on `file:line`. + A COMMENT NAMING A HEREDOC USED TO EXEMPT THE REST OF THE FILE. The exemption + scanner matched the opener anywhere on a line and left heredoc mode only on a line + equal to the tag, so a COMMENT that merely named the idiom switched the rule off for + everything after it. @linuxhikerpm measured it: with a genuine two-line violation + restored this part went red, and adding one comment line 24 lines above it -- changing + nothing else -- took it back to 37 passed while the violation was still there byte for + byte. This change is where that becomes load-bearing, because it deletes the filename + exclusion and rests the argument on the exemption being DERIVED rather than listed. + + Two conditions now, each measured. An opener is recognised only on a NON-COMMENT line, + and only when a later line EQUALS its tag -- one with no terminator exempts nothing. + The second condition is what stops a TRAILING comment doing the same thing, and it + retires the old per-file reset: an unterminated candidate can no longer leak into the + next file. Proved by restoring the violation, adding the comment, and staying red; and + by writing `cat <<'X' |` plainly in this file's own prose, which used to take its + exempt-line count from 6 to 187 and now changes nothing. + + AND BOTH NEW PREMISE ARMS WERE NUMERATOR-ONLY. They reported zero whether or not their + detector worked: replacing the heredoc-opener pattern with one that cannot match left + the arm green, and so did making the continuation detector never arm. The denominators + are printed and asserted now -- 179 openers and 5,533 continuations -- which is the + inputs == sum(buckets) rule the rest of this directory applies. An earlier draft of + this entry claimed "if either stops being true the gate says so"; that was the half + which was not true. + Planting any one of the six sites back in its old form takes the rule red and names the file and the line. diff --git a/test/selftest/080-no-suite-pipes-a-captured-string.sh b/test/selftest/080-no-suite-pipes-a-captured-string.sh index 627de969..039a0469 100644 --- a/test/selftest/080-no-suite-pipes-a-captured-string.sh +++ b/test/selftest/080-no-suite-pipes-a-captured-string.sh @@ -91,29 +91,54 @@ _epipe_globs=("$TESTDIR"/*.sh) # file:line pairs that sit inside a quoted heredoc, computed from the files. _epipe_heredoc_lines() { - awk ' - # Reset per FILE. awk keeps globals across inputs, so an unterminated - # heredoc in one file leaves the scanner inside one for every file after - # it -- and a later line that happens to equal the stale tag closes it in - # the wrong place. Measured: without this, 080s own line 26 fell OUTSIDE - # the exemption when the sweep ran over all 302 files, and inside it when - # the same function ran over that file alone. - FNR == 1 { inhd = 0; tag = "" } - !inhd && match($0, /<<[-]?'"'"'[A-Za-z_][A-Za-z0-9_]*'"'"'/) { - tag = substr($0, RSTART, RLENGTH) - gsub(/^<<[-]?'"'"'/, "", tag); gsub(/'"'"'$/, "", tag) - inhd = 1; next - } - inhd { - t = $0; gsub(/^[ \t]+|[ \t]+$/, "", t) - if (t == tag) { inhd = 0; next } - # FNR, not NR. NR is cumulative across inputs, so the second file - # onwards reports line numbers from a running total and no key ever - # matches the grep -n output it is compared against. It is the - # neighbouring question answered plausibly: single-file runs agree, - # because there NR == FNR. - print FILENAME ":" FNR ":" + # TWO PASSES PER FILE, and each condition is there because it was measured. + # + # A COMMENT IS NOT A HEREDOC OPENER. The first version matched the opener anywhere + # on a line and left heredoc mode only on a line equal to the tag, so a COMMENT that + # merely NAMED the idiom exempted every line after it. Measured by @jdatcmd on real + # code: with a genuine two-line violation restored this part went red, and adding one + # comment line 24 lines above it -- changing nothing else -- took it back to 37 + # passed while the violation was still there byte-for-byte. The rule stopped looking. + # + # AND THE TERMINATOR MUST EXIST. That comment names the tag on its own line, so + # skipping comment lines alone would still leave a TRAILING comment able to open one. + # A candidate with no later line equal to its tag is not a heredoc -- which is the + # property, rather than a guess about where the `#` was. It also retires the old + # per-file reset: an unterminated candidate now exempts nothing instead of leaking + # into the next file. + # + # THE QUOTE IS PASSED IN, not written in the program. Building the regex from `q` + # keeps a single quote out of a single-quoted shell string, where the escaping is + # its own source of defects. + awk -v q="'" ' + function flush( i, j, k, tag, t, re) { + re = "<<[-]?" q "[A-Za-z_][A-Za-z0-9_]*" q + i = 1 + while (i <= n) { + if (L[i] !~ /^[ \t]*#/ && match(L[i], re)) { + tag = substr(L[i], RSTART, RLENGTH) + sub("^<<[-]?" q, "", tag) + sub(q "$", "", tag) + for (j = i + 1; j <= n; j++) { + t = L[j] + gsub(/^[ \t]+|[ \t]+$/, "", t) + if (t == tag) break + } + if (j <= n) { + # Line numbers within THIS file, which is what the + # grep -nH keys are compared against. A running total + # would match nothing. + for (k = i + 1; k < j; k++) print fname ":" k ":" + i = j + 1 + continue + } + } + i++ + } } + FNR == 1 { if (n) flush(); delete L; n = 0; fname = FILENAME } + { L[++n] = $0 } + END { if (n) flush() } ' "$@" } _epipe_hd="$(_epipe_heredoc_lines "${_epipe_globs[@]}" 2>/dev/null || true)" @@ -404,8 +429,19 @@ check "premise: the joiner joined continuations, so the sweep is not still line- # the joiner needs to skip to the terminator; writing that now would be an # instrument with nothing exercising it, which is how twelve suites came to # maintain a check counter that nothing read. +# THE DENOMINATOR IS PRINTED AND ASSERTED, because a numerator of zero is what a +# broken detector reports too. Measured by @jdatcmd: replacing the opener pattern with +# one that cannot match anything left this arm green, so "no line opens a heredoc AND +# continues" could not be told from "no line opens a heredoc". That is the +# inputs == sum(buckets) rule this directory applies everywhere else, missing from the +# two arms I added. +_epipe_hd_open="$(grep -chE "(^|[^<])<<-?['\"]?[A-Za-z_]" "${_epipe_globs[@]}" 2>/dev/null \ + | awk '{ s += $1 } END { print s + 0 }')" _epipe_hd_cont="$(grep -nE "(^|[^<])<<-?['\"]?[A-Za-z_]" "${_epipe_globs[@]}" 2>/dev/null \ | grep -cE '([^|]\||\\)[[:space:]]*$' || true)" +echo " epipe sweep: heredoc openers=$_epipe_hd_open, of which continuing=$_epipe_hd_cont" +check "premise: the opener detector found heredocs to classify" \ + "$([ "${_epipe_hd_open:-0}" -ge 50 ] && echo yes || echo "no ($_epipe_hd_open)")" "yes" check "premise: no line opens a heredoc AND continues, which is what lets the joiner ignore bodies" \ "$_epipe_hd_cont" "0" @@ -425,9 +461,58 @@ _epipe_bs_gap="$(awk ' t = $0; sub(/[ \t]+$/, "", t) prev = (t ~ /\\$/ && t !~ /^[ \t]*#/) }' "${_epipe_globs[@]}" 2>/dev/null | grep -c . || true)" +_epipe_bs_n="$(awk ' + { + t = $0; sub(/[ \t]+$/, "", t) + if (t ~ /\\$/ && t !~ /^[ \t]*#/) n++ + } + END { print n + 0 }' "${_epipe_globs[@]}" 2>/dev/null)" +echo " epipe sweep: backslash continuations=$_epipe_bs_n, of which followed by a gap=$_epipe_bs_gap" +check "premise: the continuation detector found continuations to classify" \ + "$([ "${_epipe_bs_n:-0}" -ge 500 ] && echo yes || echo "no ($_epipe_bs_n)")" "yes" check "premise: no backslash continuation is followed by a blank or a comment" \ "$_epipe_bs_gap" "0" +# A COMMENT NAMING THE IDIOM MUST NOT EXEMPT WHAT FOLLOWS IT, which is the hole +# @jdatcmd found: one comment line 24 lines above a genuine violation took this part +# from red to 37 passed with the violation still there byte-for-byte. A heredoc opener +# is now recognised only on a non-comment line AND only when a later line equals its +# tag, so a comment that names the tag on its own line -- which is how anybody writes +# it in prose -- cannot open one. +_epipe_cmthd="$PGC_WORKDIR/epipe_comment_heredoc.sh" +{ + printf '# the idiom is cat <<%sEOF%s ... EOF, which this suite does not use\n' "'" "'" + printf 'x="value"\n' + printf 'echo "$x" %s grep -%s PLANTED && echo yes || echo no\n' '|' q +} > "$_epipe_cmthd" +check "a comment naming a heredoc exempts nothing, so the line below it is still seen" \ + "$(_epipe_heredoc_lines "$_epipe_cmthd" | grep -c . || true)" "0" +check "and the planted violation in that file is found by the pattern" \ + "$(grep -cE "$_epipe_pat" "$_epipe_cmthd")" "1" + +# The control: a REAL heredoc, opened in code and terminated, still exempts its body. +# Without this the arm above is satisfied by an exemption that never fires at all. +_epipe_realhd="$PGC_WORKDIR/epipe_real_heredoc.sh" +{ + printf 'cat > /dev/null <<%sEOF%s\n' "'" "'" + printf 'echo "$x" %s grep -%s INSIDE && echo yes || echo no\n' '|' q + printf 'EOF\n' +} > "$_epipe_realhd" +check "control: a real heredoc, opened in code and terminated, still exempts its body" \ + "$(_epipe_heredoc_lines "$_epipe_realhd" | grep -c . || true)" "1" +check "premise: and that body line is the one the pattern would otherwise flag" \ + "$(grep -cE "$_epipe_pat" "$_epipe_realhd")" "1" + +# And an UNTERMINATED candidate exempts nothing, which is what stops a trailing comment +# naming a tag from switching the rule off for the rest of the file. +_epipe_unterm="$PGC_WORKDIR/epipe_unterminated.sh" +{ + printf 'echo hi # see cat <<%sNOPE%s for the idiom\n' "'" "'" + printf 'echo "$x" %s grep -%s PLANTED && echo yes || echo no\n' '|' q +} > "$_epipe_unterm" +check "an opener with no terminator exempts nothing" \ + "$(_epipe_heredoc_lines "$_epipe_unterm" | grep -c . || true)" "0" + # ---- the two split shapes, as fixtures -------------------------------------- # # Assembled from fragments rather than written out, for the reason the probe above From 7cabf23a6bdae308f3fdcd7fa0b2f2be8bfd1590 Mon Sep 17 00:00:00 2001 From: OffgridwithJD Date: Thu, 10 Sep 2026 19:59:22 +0000 Subject: [PATCH 3/3] test/selftest: a premise read off a dead detector is not a premise (#486) Three of this part's premise arms reported "the corpus does not contain this shape" from detectors that had stopped detecting, and nothing could tell the two apart. Measured by @linuxhikerpm, who also corrected their own first prescription for it: a denominator is not the fix, a positive fixture is. A DENOMINATOR PROVES THE INPUT LIST, NOT THE CLASSIFIER. The continuing-opener arm printed and asserted its denominator -- 179 heredoc openers -- and still went green with the continuing pattern neutered, because the denominator counts OPENERS and the numerator counts a SUBSET, computed by a different detector that was no longer working. `inputs == sum(buckets)` says the input is non-empty. It says nothing about whether the thing that splits it still fires. So 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 the fixtures went in: 179 openers, 0 continuing, 0 gaps, 5537 continuations, identical to the inline forms on all 312 swept files. AND THE COMMENT CONDITION NEEDED A TERMINATED TAG. The fixture guarding it named a tag no later line closed, so the terminator condition refused the candidate first and dropping the comment condition alone changed nothing -- 44/44 green with the condition gone. The new fixture's comment names a tag that IS closed below, which leaves the comment condition as the only thing between the planted violation and an exemption. Each condition mutated alone, against a control, with every edit asserted to have landed and the file asserted to still parse: control 576 checks, 0 failed comment condition dropped 1 failed -- the new terminated-tag fixture terminator condition dropped 2 failed -- both unterminated fixtures continuing detector dead 1 failed -- the new continuing-opener fixture gap detector never arms 1 failed -- the new gap fixture I ALSO EXPECTED A FOURTH GAP AND THERE WAS NONE. 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: its candidate sits after a MID-LINE `#`, and the line does not start with one. The comment in the new arm now says that, because a wrong comment is an input to the next defect. The arm stays as a second shape of the property -- a trailing comment there, a string assignment here. My first mutation round was invalid on three of four arms and I am recording that rather than the clean second round alone: two perl substitutions never applied, and the third corrupted the grep pattern so the arm reddened about a broken line instead of a zeroed detector. A mutation that breaks the file measures nothing. The harness now asserts each edit landed, that the file still parses, and that the run reached a summary at all -- the last one because a selftest handed a pg_config that does not exist exits 0 having evaluated nothing (#934). Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a --- .../080-no-suite-pipes-a-captured-string.sh | 142 +++++++++++++++--- 1 file changed, 125 insertions(+), 17 deletions(-) diff --git a/test/selftest/080-no-suite-pipes-a-captured-string.sh b/test/selftest/080-no-suite-pipes-a-captured-string.sh index 039a0469..f9f88bc8 100644 --- a/test/selftest/080-no-suite-pipes-a-captured-string.sh +++ b/test/selftest/080-no-suite-pipes-a-captured-string.sh @@ -435,10 +435,25 @@ check "premise: the joiner joined continuations, so the sweep is not still line- # continues" could not be told from "no line opens a heredoc". That is the # inputs == sum(buckets) rule this directory applies everywhere else, missing from the # two arms I added. -_epipe_hd_open="$(grep -chE "(^|[^<])<<-?['\"]?[A-Za-z_]" "${_epipe_globs[@]}" 2>/dev/null \ - | awk '{ s += $1 } END { print s + 0 }')" -_epipe_hd_cont="$(grep -nE "(^|[^<])<<-?['\"]?[A-Za-z_]" "${_epipe_globs[@]}" 2>/dev/null \ - | grep -cE '([^|]\||\\)[[:space:]]*$' || true)" +# ONE DEFINITION PER DETECTOR, because each one is about to be 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 still detects. +# +# A DENOMINATOR IS NOT THAT PROOF. Measured by @linuxhikerpm: neutering the +# CONTINUING pattern alone left this part at 44/44 green. The denominator arm below +# it could not catch that, because the denominator counts OPENERS -- a different +# detector, which was still working. `inputs == sum(buckets)` proves the input list +# is non-empty; it says nothing about whether the classifier that splits it still +# fires. Only a positive fixture does that, and my first version of these two arms +# had the denominator and not the fixture. +_epipe_hd_openers() { # _epipe_hd_openers FILE... -> file:line: of heredoc openers + grep -nE "(^|[^<])<<-?['\"]?[A-Za-z_]" "$@" 2>/dev/null +} +_epipe_hd_continuing() { # ...of those, the ones that ALSO continue + _epipe_hd_openers "$@" | grep -E '([^|]\||\\)[[:space:]]*$' +} +_epipe_hd_open="$(_epipe_hd_openers "${_epipe_globs[@]}" | grep -c . || true)" +_epipe_hd_cont="$(_epipe_hd_continuing "${_epipe_globs[@]}" | grep -c . || true)" echo " epipe sweep: heredoc openers=$_epipe_hd_open, of which continuing=$_epipe_hd_cont" check "premise: the opener detector found heredocs to classify" \ "$([ "${_epipe_hd_open:-0}" -ge 50 ] && echo yes || echo "no ($_epipe_hd_open)")" "yes" @@ -454,19 +469,27 @@ check "premise: no line opens a heredoc AND continues, which is what lets the jo # 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. The shape is a # defect on its own, so the arm earns its keep whichever way it goes red. -_epipe_bs_gap="$(awk ' - FNR == 1 { prev = 0 } - { - if (prev && ($0 ~ /^[ \t]*$/ || $0 ~ /^[ \t]*#/)) print FILENAME ":" FNR - t = $0; sub(/[ \t]+$/, "", t) - prev = (t ~ /\\$/ && t !~ /^[ \t]*#/) - }' "${_epipe_globs[@]}" 2>/dev/null | grep -c . || true)" -_epipe_bs_n="$(awk ' - { - t = $0; sub(/[ \t]+$/, "", t) - if (t ~ /\\$/ && t !~ /^[ \t]*#/) n++ - } - END { print n + 0 }' "${_epipe_globs[@]}" 2>/dev/null)" +# Same shape as the pair above, and same reason: the gap detector is armed by a +# condition of its own (`prev`), and arming it wrongly reports zero gaps from a +# corpus full of continuations. A fixture that HAS a gap is what tells those apart. +_epipe_bs_conts() { # _epipe_bs_conts FILE... -> file:line: of backslash continuations + awk ' + { + t = $0; sub(/[ \t]+$/, "", t) + if (t ~ /\\$/ && t !~ /^[ \t]*#/) print FILENAME ":" FNR + }' "$@" 2>/dev/null +} +_epipe_bs_gaps() { # ...of those, the ones a blank or a comment follows + awk ' + FNR == 1 { prev = 0 } + { + if (prev && ($0 ~ /^[ \t]*$/ || $0 ~ /^[ \t]*#/)) print FILENAME ":" FNR + t = $0; sub(/[ \t]+$/, "", t) + prev = (t ~ /\\$/ && t !~ /^[ \t]*#/) + }' "$@" 2>/dev/null +} +_epipe_bs_gap="$(_epipe_bs_gaps "${_epipe_globs[@]}" | grep -c . || true)" +_epipe_bs_n="$(_epipe_bs_conts "${_epipe_globs[@]}" | grep -c . || true)" echo " epipe sweep: backslash continuations=$_epipe_bs_n, of which followed by a gap=$_epipe_bs_gap" check "premise: the continuation detector found continuations to classify" \ "$([ "${_epipe_bs_n:-0}" -ge 500 ] && echo yes || echo "no ($_epipe_bs_n)")" "yes" @@ -513,6 +536,91 @@ _epipe_unterm="$PGC_WORKDIR/epipe_unterminated.sh" check "an opener with no terminator exempts nothing" \ "$(_epipe_heredoc_lines "$_epipe_unterm" | grep -c . || true)" "0" +# ---- the detectors, driven by files that DO contain what they look for ------- +# +# Every arm above this point asks a detector whether the corpus contains a shape, +# and the corpus does not. A detector that has stopped detecting answers exactly +# the same way. @linuxhikerpm neutered the continuing-opener pattern and the gap +# detector's arming one at a time and this part stayed at 44/44 both times, so the +# premises were reading "the corpus is clean" off a dead instrument. +# +# EVERY FIXTURE BELOW IS ASSEMBLED, never written out, for the reason the probe is: +# these globs include this file, so a literal opener here would be a site the +# sweep then has to exempt -- and exempting the part that tests the exemption is +# how the hole @jdatcmd found got in. + +# A line that opens a heredoc AND continues. Both forms, because the joiner treats +# `|` and `\` through the same code path and only `|` is bash's own continuation. +_epipe_hdcont="$PGC_WORKDIR/epipe_hd_continuing.sh" +{ + printf 'cat <<%sAAA%s %s\n' "'" "'" '|' + printf '\tbody of the first heredoc\n' + printf 'AAA\n' + printf 'cat <<%sBBB%s \\\n' "'" "'" + printf '\tbody of the second heredoc\n' + printf 'BBB\n' +} > "$_epipe_hdcont" +check "premise: the fixture really has two heredoc openers for the detector to see" \ + "$(_epipe_hd_openers "$_epipe_hdcont" | grep -c . || true)" "2" +check "the continuing-opener detector finds an opener that ends in a pipe, and one that ends in a backslash" \ + "$(_epipe_hd_continuing "$_epipe_hdcont" | grep -c . || true)" "2" + +# A backslash continuation followed by a comment, and one followed by a blank. +# Both are the shape that makes the joiner's blank-and-comment skipping wrong for +# `\`, which is the only reason the corpus premise is asserted at all. +_epipe_bsgap="$PGC_WORKDIR/epipe_bs_gap.sh" +{ + printf '%s\n' 'echo first \' + printf '%s\n' '# a comment directly after a continuation, which bash folds in' + printf '%s\n' 'echo second \' + printf '\n' + printf '%s\n' 'echo third' +} > "$_epipe_bsgap" +check "premise: the fixture really has two backslash continuations to classify" \ + "$(_epipe_bs_conts "$_epipe_bsgap" | grep -c . || true)" "2" +check "the gap detector finds a continuation followed by a comment, and one followed by a blank" \ + "$(_epipe_bs_gaps "$_epipe_bsgap" | grep -c . || true)" "2" + +# ---- each condition of the heredoc rule, made load-bearing on its own --------- +# +# THE COMMENT CONDITION NEEDS A TERMINATED TAG. `$_epipe_cmthd` above names a tag +# that no later line closes, so the TERMINATOR condition refuses it first and +# dropping the comment condition alone changes nothing -- which @linuxhikerpm +# measured as 44/44. Here the comment names a tag that IS closed below, so the +# comment condition is the only thing standing between the planted violation and +# an exemption. +_epipe_cmthd_term="$PGC_WORKDIR/epipe_comment_terminated.sh" +{ + printf '# the idiom is cat <<%sEOF%s, closed by the tag on its own line below\n' "'" "'" + printf 'x="value"\n' + printf 'echo "$x" %s grep -%s PLANTED && echo yes || echo no\n' '|' q + printf 'EOF\n' +} > "$_epipe_cmthd_term" +check "a comment naming a TERMINATED tag still opens no heredoc, so the lines below it are seen" \ + "$(_epipe_heredoc_lines "$_epipe_cmthd_term" | grep -c . || true)" "0" +check "premise: and the violation between that comment and its tag is what the pattern would flag" \ + "$(grep -cE "$_epipe_pat" "$_epipe_cmthd_term")" "1" + +# THE TERMINATOR CONDITION IS ALREADY LOAD-BEARING, and this arm is a second shape +# of the same property rather than a repair. I expected the opposite and measured +# it: `if (j <= n)` -> `if (1)` reds `$_epipe_unterm` as well as this fixture. That +# is because `$_epipe_unterm`'s candidate sits after a MID-LINE `#`, so the line +# does not start with one and the comment condition never refuses it -- the missing +# terminator is the only thing that does. The two fixtures differ in where the +# candidate lives: a trailing comment there, a string assignment here, which is the +# shape a sweep over real code actually meets. +_epipe_unterm_code="$PGC_WORKDIR/epipe_unterminated_code.sh" +{ + printf 'x="<<%sNOPE%s is named here and nothing closes it"\n' "'" "'" + printf 'echo "$x" %s grep -%s PLANTED && echo yes || echo no\n' '|' q +} > "$_epipe_unterm_code" +check "premise: the candidate is on a line of code, not a comment" \ + "$(_epipe_hd_openers "$_epipe_unterm_code" | grep -c . || true)" "1" +check "an opener on a line of CODE with no terminator exempts nothing either" \ + "$(_epipe_heredoc_lines "$_epipe_unterm_code" | grep -c . || true)" "0" +check "premise: and the line below it is what the pattern would flag" \ + "$(grep -cE "$_epipe_pat" "$_epipe_unterm_code")" "1" + # ---- the two split shapes, as fixtures -------------------------------------- # # Assembled from fragments rather than written out, for the reason the probe above