fix: a suite must not decide a verdict from a pipeline's exit status (#486)

test/alter_column_type.sh

@@ -55,7 +55,7 @@ conv() {  # label, column type, value expression, alter clause
 	err="$(psql_run "ALTER TABLE $c ALTER COLUMN v TYPE $4;" 2>&1 || true)"
 	psql_run "ALTER TABLE $h ALTER COLUMN v TYPE $4;" >/dev/null 2>&1
 
-	if echo "$err" | grep -qiE "corrupt encoded chunk|server closed|terminated"; then
+	if grep -qiE "corrupt encoded chunk|server closed|terminated" <<<"$err"; then
 		check "$1" "failed: $(echo "$err" | grep -oiE 'corrupt encoded chunk[^\"]*|server closed' | head -1)" "ok"
 		return
 	fi

test/cancel_decode.sh

@@ -79,7 +79,7 @@ cancel_ms() {
 		c=$(date +%s%N)
 		out="$(raw "SET statement_timeout = 100; $(printf "$(QUERY "$t")")")"
 		d=$(date +%s%N)
-		echo "$out" | grep -qi "canceling statement" || { echo FAILED; return; }
+		grep -qi "canceling statement" <<<"$out" || { echo FAILED; return; }
 		ms=$(( (d - c) / 1000000 ))
 		[ -z "$best" ] || [ "$ms" -lt "$best" ] && best="$ms"
 	done

test/fuzz_arrow.sh

@@ -173,8 +173,8 @@ run_stmt() {
 		-c "SET statement_timeout = '20s'; $stmt" 2>&1)"
 	rc=$?
 
-	if echo "$out" | grep -qE 'could not connect|No such file or directory|Connection refused' &&
-	   ! echo "$out" | grep -q '^ERROR:'; then
+	if grep -qE 'could not connect|No such file or directory|Connection refused' <<<"$out" &&
+	   ! grep -q '^ERROR:' <<<"$out"; then
 		if ! wait_for_cluster; then
 			echo "  FATAL: cluster unreachable and did not return"
 			return 1
@@ -183,14 +183,14 @@ run_stmt() {
 	log_since
 	newlog="$(cat "$NEWLOG" 2>/dev/null)"
 
-	if echo "$newlog" | grep -qE 'AddressSanitizer|runtime error:|UndefinedBehaviorSanitizer|LeakSanitizer'; then
+	if grep -qE 'AddressSanitizer|runtime error:|UndefinedBehaviorSanitizer|LeakSanitizer' <<<"$newlog"; then
 		sanitizer=$((sanitizer + 1))
 		save_finding san "$seedfile" "$mutseed" "$stmt" "$newlog"
 		wait_for_cluster || return 1
 		return 1
 	fi
 
-	if echo "$newlog" | grep -qE 'was terminated by signal|server process .* exited with|crashed'; then
+	if grep -qE 'was terminated by signal|server process .* exited with|crashed' <<<"$newlog"; then
 		crashes=$((crashes + 1))
 		save_finding crash "$seedfile" "$mutseed" "$stmt" "$newlog"
 		wait_for_cluster || echo "  (cluster did not come back)"
@@ -204,20 +204,20 @@ run_stmt() {
 		return 1
 	fi
 
-	if echo "$out" | grep -q 'canceling statement due to statement timeout'; then
+	if grep -q 'canceling statement due to statement timeout' <<<"$out"; then
 		hangs=$((hangs + 1))
 		save_finding hang "$seedfile" "$mutseed" "$stmt" "$out"
 		return 1
 	fi
 
-	if echo "$out" | grep -qE 'server closed the connection unexpectedly|connection to server was lost|terminating connection'; then
+	if grep -qE 'server closed the connection unexpectedly|connection to server was lost|terminating connection' <<<"$out"; then
 		crashes=$((crashes + 1))
 		save_finding crash "$seedfile" "$mutseed" "$stmt" "$out"
 		wait_for_cluster || echo "  (cluster did not come back)"
 		return 1
 	fi
 
-	if echo "$out" | grep -q '^ERROR:'; then
+	if grep -q '^ERROR:' <<<"$out"; then
 		errors=$((errors + 1))
 	else
 		clean=$((clean + 1))

test/fuzz_parquet.sh

@@ -191,8 +191,8 @@ run_stmt() {
 	rc=$?
 
 	# Never let a connection failure be read as a clean result again.
-	if echo "$out" | grep -qE 'could not connect|No such file or directory|Connection refused' &&
-	   ! echo "$out" | grep -q '^ERROR:'; then
+	if grep -qE 'could not connect|No such file or directory|Connection refused' <<<"$out" &&
+	   ! grep -q '^ERROR:' <<<"$out"; then
 		if ! wait_for_cluster; then
 			echo "  FATAL: cluster unreachable and did not return"
 			return 1
@@ -202,14 +202,14 @@ run_stmt() {
 	newlog="$(cat "$NEWLOG" 2>/dev/null)"
 
 	# A sanitizer report is a finding even when the statement then succeeds.
-	if echo "$newlog" | grep -qE 'AddressSanitizer|runtime error:|UndefinedBehaviorSanitizer|LeakSanitizer'; then
+	if grep -qE 'AddressSanitizer|runtime error:|UndefinedBehaviorSanitizer|LeakSanitizer' <<<"$newlog"; then
 		sanitizer=$((sanitizer + 1))
 		save_finding san "$seedfile" "$mutseed" "$stmt" "$newlog"
 		wait_for_cluster || return 1
 		return 1
 	fi
 
-	if echo "$newlog" | grep -qE 'was terminated by signal|server process .* exited with|crashed'; then
+	if grep -qE 'was terminated by signal|server process .* exited with|crashed' <<<"$newlog"; then
 		crashes=$((crashes + 1))
 		save_finding crash "$seedfile" "$mutseed" "$stmt" "$newlog"
 		wait_for_cluster || echo "  (cluster did not come back)"
@@ -225,20 +225,20 @@ run_stmt() {
 
 	# statement_timeout firing is a hang too: the decode did not finish in 20s
 	# on a file that is at most a few hundred kilobytes.
-	if echo "$out" | grep -q 'canceling statement due to statement timeout'; then
+	if grep -q 'canceling statement due to statement timeout' <<<"$out"; then
 		hangs=$((hangs + 1))
 		save_finding hang "$seedfile" "$mutseed" "$stmt" "$out"
 		return 1
 	fi
 
-	if echo "$out" | grep -qE 'server closed the connection unexpectedly|connection to server was lost|terminating connection'; then
+	if grep -qE 'server closed the connection unexpectedly|connection to server was lost|terminating connection' <<<"$out"; then
 		crashes=$((crashes + 1))
 		save_finding crash "$seedfile" "$mutseed" "$stmt" "$out"
 		wait_for_cluster || echo "  (cluster did not come back)"
 		return 1
 	fi
 
-	if echo "$out" | grep -q '^ERROR:'; then
+	if grep -q '^ERROR:' <<<"$out"; then
 		errors=$((errors + 1))
 	else
 		clean=$((clean + 1))

test/harness_selftest.sh

@@ -363,6 +363,72 @@ done < <(listed_suites)
 check "every registered suite has a file" \
 	"$([ -z "$missing_file" ] && echo none || echo "missing:$missing_file")" "none"
 
+# ---- no suite pipes a captured string into an early-exit reader (#486) -------
+#
+# `echo "$s" | grep -q PATTERN` under `set -o pipefail` answers "not found" when
+# the WRITER fails, whatever the string contained. The reader exits as soon as it
+# has its answer, the writer takes EPIPE, and pipefail calls the pipeline failed.
+# The `&&` arm never runs and the helper reports absence.
+#
+# This is not theoretical and it is not new here. #473 found it in this file's own
+# membership test, and it came back in native_agg.sh, where it reported "the
+# metadata aggregate node did not run" on PG18 CI for a plan that contained the
+# node -- a red that reads exactly like a planner regression in the area #133 and
+# #140 live in. The tell was a `Broken pipe` line beside a result the same run's
+# summary contradicted.
+#
+# The failure direction is what makes it worth a rule: it always reports the
+# thing you were looking for as ABSENT, which is the answer that sends someone
+# looking for a defect that is not there.
+#
+# The control below runs first, because a rule with no demonstrated failure is a
+# style preference, and this one is not.
+_epipe_demo="$PGC_WORKDIR/epipe_demo.sh"
+cat > "$_epipe_demo" <<'DEMO'
+set -uo pipefail
+big="MATCHME
+$(head -c 300000 /dev/zero | tr '\0' 'y')"
+piped() { echo "$1" | grep -q 'MATCHME' && echo yes || echo no; }
+cased() { case "$1" in *MATCHME*) echo yes ;; *) echo no ;; esac; }
+echo "piped=$(piped "$big" 2>/dev/null) cased=$(cased "$big")"
+DEMO
+_epipe_result="$(bash "$_epipe_demo" 2>/dev/null)"
+
+# The string CONTAINS the pattern, on its first line, in both arms. Only the
+# answers differ. Written large on purpose: at a few kilobytes the write fits in
+# the pipe buffer and completes before the reader can exit, which is why this
+# shape passes almost every time and then does not.
+check "control: piping a large string into grep -q reports a match as absent" \
+	"$_epipe_result" "piped=no cased=yes"
+
+# The rule itself. A pipeline whose left side is a shell builtin writing a
+# captured string, and whose right side is a reader that exits early AND whose
+# EXIT STATUS is the answer being read. That last part is the whole rule: the
+# damage is a wrong verdict, not a wrong message.
+#
+# So `grep -q` is in scope and `| head -1` inside a diagnostic string is not.
+# Those exist here (analyze_stats.sh prints a plan's first line that way) and
+# they can lose their pipeline's status without changing any check, because the
+# substitution is used as text. They are left alone on purpose rather than
+# missed; the worst they do is print to stderr.
+#
+# Scoped to echo and printf deliberately for the same reason. A pipeline out of
+# psql or a file is a different question with a different answer, and a rule that
+# flagged those too would be argued with rather than kept.
+_epipe_hits="$(grep -rnE '(echo|printf)[^|]*\|[[:space:]]*grep -[a-zA-Z]*q' \
+	"$TESTDIR"/*.sh 2>/dev/null | grep -v '/harness_selftest.sh:' || true)"
+_epipe_count="$(printf '%s' "$_epipe_hits" | grep -c . || true)"
+[ -n "$_epipe_hits" ] || _epipe_count=0
+check "no suite pipes a captured string into an early-exit reader" \
+	"$_epipe_count" "0"
+[ "$_epipe_count" = "0" ] || printf '%s\n' "$_epipe_hits" | sed 's/^/      /' | head -20
+
+# And the scan has to be looking at something. A glob that matched nothing, or a
+# TESTDIR that moved, would report zero hits and read as compliance.
+_epipe_scanned="$(grep -rlE 'grep' "$TESTDIR"/*.sh 2>/dev/null | grep -c . || true)"
+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"
+
 # ---- no suite hands every run the same default port ------------------------
 
 # #184 derived the port per run in lib.sh and in the matrix runner, which is

test/native_agg.sh

@@ -38,8 +38,8 @@ plan() {
 # The metadata aggregate path reports "Columnar Vectorized Aggregates: N" and has
 # no separate Aggregate node above a scan; a fallback plan has an Aggregate node
 # and no such line.
-has_aggnode() { echo "$1" | grep -q 'Columnar Vectorized Aggregates' && echo yes || echo no; }
-has_gather() { echo "$1" | grep -q 'Gather' && echo yes || echo no; }
+has_aggnode() { grep -q 'Columnar Vectorized Aggregates' <<<"$1" && echo yes || echo no; }
+has_gather() { grep -q 'Gather' <<<"$1" && echo yes || echo no; }
 
 # Same, with a parallel plan available: workers allowed, no setup charge, and no
 # minimum table size, since the suite runs with max_parallel_workers_per_gather=0

test/native_groupagg.sh

@@ -232,7 +232,7 @@ oc_out="$(env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" -U postg
 		SET enable_hashagg=off; SET enable_sort=off;
 		SET pgcolumnar.groupagg_max_groups=100;
 		SELECT g, count(*) FROM t_col GROUP BY g" 2>&1 || true)"
-n_err="$(printf '%s' "$oc_out" | grep -q 'groupagg_max_groups' && echo yes || echo no)"
+n_err="$(grep -q 'groupagg_max_groups' <<<"$oc_out" && echo yes || echo no)"
 check "over-cap stops with a groupagg_max_groups error" "$n_err" yes
 diff_query "oracle exact: default cap runs the high-cardinality key" \
 	"SELECT g, count(*), sum(i4) FROM %T GROUP BY g"
@@ -323,7 +323,7 @@ ov_out="$(env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" -U postg
 	-d "$PGC_DB" -Atc "SET pgcolumnar.enable_group_vectorization=on;
 		SELECT k, avg(d) FROM ov_col GROUP BY k" 2>&1 || true)"
 check "avg(float8) overflow errors like core" \
-	"$(printf '%s' "$ov_out" | grep -qi 'out of range' && echo yes || echo no)" yes
+	"$(grep -qi 'out of range' <<<"$ov_out" && echo yes || echo no)" yes
 
 # ---- 8. degenerate inputs --------------------------------------------------
 

test/native_ownership.sh

@@ -31,7 +31,7 @@ refused() {
 	local out
 	out="$(as_alice "SELECT pgcolumnar.$1;")"
 	check "non-owner refused: ${1%%(*}" \
-		"$(printf '%s' "$out" | grep -qi 'must be owner' && echo yes || echo "no")" "yes"
+		"$(grep -qi 'must be owner' <<<"$out" && echo yes || echo "no")" "yes"
 }
 
 refused "compact('n')"

test/native_reclaim_cycles.sh

@@ -51,7 +51,7 @@ for r in 1 2 3 4 5; do
 		-d "$PGC_DB" -At -c "SELECT pgcolumnar.compact_rewrite('n', 0.02);" 2>&1)"
 	# a healthy call returns an integer group count; the bug returned an ERROR line
 	check "compact_rewrite cycle $r returns a count (no self-conflict)" \
-		"$(printf '%s' "$rw" | grep -Eq '^[0-9]+$' && echo ok || echo "bad:$rw")" "ok"
+		"$(grep -Eq '^[0-9]+$' <<<"$rw" && echo ok || echo "bad:$rw")" "ok"
 	check "parity after compact_rewrite cycle $r" "$(hash_n)" "$(hash_h)"
 	sizes[$r]="$(fsize)"
 done

test/native_truncate.sh

@@ -84,7 +84,7 @@ psql_run "SELECT pgcolumnar.compact('n');"
 before_guard="$(size)"
 errout="$(raw "BEGIN; SELECT pgcolumnar.truncate('n'); ROLLBACK;")"
 check "truncate refused inside a transaction block" \
-	"$(printf '%s' "$errout" | grep -qi 'cannot run inside a transaction block' && echo yes || echo no)" "yes"
+	"$(grep -qi 'cannot run inside a transaction block' <<<"$errout" && echo yes || echo no)" "yes"
 check "file unchanged after refused in-txn truncate" "$(size)" "$before_guard"
 check "parity after refused in-txn truncate" "$(hash_n)" "$(hash_h)"
 

test/parallel_copy.sh

@@ -190,15 +190,15 @@ check_reconstruct "$F_NONL" 4 "no trailing newline / 4 workers"
 err_out="$(env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" -U postgres \
 	-d "$PGC_DB" -Atc "SELECT pgcolumnar.file_split_offsets('$F', 0)" 2>&1 || true)"
 check "workers < 1 is rejected" \
-	"$(printf '%s' "$err_out" | grep -qi "at least 1" && echo ok || echo no)" ok
+	"$(grep -qi "at least 1" <<<"$err_out" && echo ok || echo no)" ok
 
 # a directory is rejected, not reported as an 8-exabyte splittable file (regression
 # for the missing fstat/S_ISREG guard: lseek(SEEK_END) on a directory fd returns a
 # huge value on Linux, so without the guard this returned {0, 9.2e18}).
 dir_err="$(env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" -U postgres \
 	-d "$PGC_DB" -Atc "SELECT pgcolumnar.file_split_offsets('$DATADIR', 1)" 2>&1 || true)"
 check "file_split_offsets: a directory is rejected (not an 8-exabyte file)" \
-	"$(printf '%s' "$dir_err" | grep -qi "not a regular file" && echo ok || echo no)" ok
+	"$(grep -qi "not a regular file" <<<"$dir_err" && echo ok || echo no)" ok
 
 # ---- coordinator: pgcolumnar.parallel_copy (partition-parallel, atomic 2PC) ---
 # Each worker loads a DISTINCT partition (distinct storage id -> parallel AND
@@ -265,7 +265,7 @@ psql_run "DROP TABLE IF EXISTS t_txtkey CASCADE;
           CREATE TABLE t_txtkey_b PARTITION OF t_txtkey FOR VALUES FROM ('m') TO (MAXVALUE) USING pgcolumnar;" >/dev/null
 tk_err="$(err_of "SELECT pgcolumnar.parallel_copy('t_txtkey'::regclass, '$F', 2)")"
 check "text partition key is rejected (not a crash)" \
-	"$(printf '%s' "$tk_err" | grep -qi "numeric or date/time" && echo ok || echo no)" ok
+	"$(grep -qi "numeric or date/time" <<<"$tk_err" && echo ok || echo no)" ok
 check "text partition key: server still up (no crash)" "$(q "SELECT 1")" 1
 
 # ---- key NOT in column 1, with a generated column before it -------------------
@@ -299,7 +299,7 @@ F_BADKEY="$DATADIR/badkey.txt"
 mkpart t_pcp 4
 bk_err="$(err_of "SELECT pgcolumnar.parallel_copy('t_pcp'::regclass, '$F_BADKEY', 4)")"
 check "atomic: a bad partition-key value is rejected" \
-	"$(printf '%s' "$bk_err" | grep -qi 'invalid input syntax' && echo ok || echo no)" ok
+	"$(grep -qi 'invalid input syntax' <<<"$bk_err" && echo ok || echo no)" ok
 check "atomic: bad-key load leaves the target empty" "$(q "SELECT count(*) FROM t_pcp")" 0
 check "atomic: no prepared-transaction leak after bad-key" "$(q "SELECT count(*) FROM pg_prepared_xacts")" 0
 
@@ -314,7 +314,7 @@ psql_run "DROP TABLE IF EXISTS t_gap CASCADE;
           CREATE TABLE t_gap_b PARTITION OF t_gap FOR VALUES FROM (3000) TO (MAXVALUE) USING pgcolumnar;" >/dev/null
 gap_err="$(err_of "SELECT pgcolumnar.parallel_copy('t_gap'::regclass, '$F', 4)")"
 check "atomic: a loader failure fails the whole load" \
-	"$(printf '%s' "$gap_err" | grep -qiE 'no partition of relation|failed' && echo ok || echo no)" ok
+	"$(grep -qiE 'no partition of relation|failed' <<<"$gap_err" && echo ok || echo no)" ok
 check "atomic: loader-failure leaves the target empty (siblings rolled back)" \
 	"$(q "SELECT count(*) FROM t_gap")" 0
 check "atomic: no prepared-transaction leak after loader failure" \
@@ -327,7 +327,7 @@ shuf "$F" > "$F_SHUF"; [ "$(id -u)" = "0" ] && chown postgres "$F_SHUF"
 mkpart t_pcp 4
 shuf_err="$(err_of "SELECT pgcolumnar.parallel_copy('t_pcp'::regclass, '$F_SHUF', 4)")"
 check "unsorted input is rejected" \
-	"$(printf '%s' "$shuf_err" | grep -qi "not sorted" && echo ok || echo no)" ok
+	"$(grep -qi "not sorted" <<<"$shuf_err" && echo ok || echo no)" ok
 check "unsorted rejection loads nothing" "$(q "SELECT count(*) FROM t_pcp")" 0
 
 # ---- a DEFAULT partition is rejected (it could catch any worker's rows) -------
@@ -337,30 +337,30 @@ psql_run "DROP TABLE IF EXISTS t_def CASCADE;
           CREATE TABLE t_def_d PARTITION OF t_def DEFAULT USING pgcolumnar;" >/dev/null
 def_err="$(err_of "SELECT pgcolumnar.parallel_copy('t_def'::regclass, '$F', 4)")"
 check "DEFAULT partition target is rejected" \
-	"$(printf '%s' "$def_err" | grep -qi "DEFAULT partition" && echo ok || echo no)" ok
+	"$(grep -qi "DEFAULT partition" <<<"$def_err" && echo ok || echo no)" ok
 
 # ---- the max_prepared_transactions guard fires up front ----------------------
 # max_prepared_transactions is 8 (set above); a target with more partitions than
 # that, loaded with that many workers, must error before spawning, naming the GUC.
 mkpart t_pcp10 10
 guard_err="$(err_of "SELECT pgcolumnar.parallel_copy('t_pcp10'::regclass, '$F', 10)")"
 check "max_prepared_transactions guard fires" \
-	"$(printf '%s' "$guard_err" | grep -qi "max_prepared_transactions" && echo ok || echo no)" ok
+	"$(grep -qi "max_prepared_transactions" <<<"$guard_err" && echo ok || echo no)" ok
 check "guard rejects before loading anything" "$(q "SELECT count(*) FROM t_pcp10")" 0
 
 # ---- a missing file errors cleanly -------------------------------------------
 mkpart t_pcp 4
 mf_err="$(err_of "SELECT pgcolumnar.parallel_copy('t_pcp'::regclass, '$DATADIR/nope.txt', 2)")"
 check "missing file: errors, not crashes" \
-	"$(printf '%s' "$mf_err" | grep -qiE "could not (open|stat)|no such file|not a regular file" && echo ok || echo no)" ok
+	"$(grep -qiE "could not (open|stat)|no such file|not a regular file" <<<"$mf_err" && echo ok || echo no)" ok
 check "missing file: server still up (no worker crash)" "$(q "SELECT 1")" 1
 check "missing file: nothing loaded" "$(q "SELECT count(*) FROM t_pcp")" 0
 
 # ---- a non-columnar (heap) target is rejected --------------------------------
 psql_run "DROP TABLE IF EXISTS t_heaptgt; CREATE TABLE t_heaptgt (id int, txt text);" >/dev/null
 nc_err="$(err_of "SELECT pgcolumnar.parallel_copy('t_heaptgt'::regclass, '$F', 2)")"
 check "non-columnar target is rejected" \
-	"$(printf '%s' "$nc_err" | grep -qi "not a pgcolumnar table" && echo ok || echo no)" ok
+	"$(grep -qi "not a pgcolumnar table" <<<"$nc_err" && echo ok || echo no)" ok
 
 # ---- single columnar table: workers write ONE storage concurrently -----------
 # The storage-row creation lock is skipped by the loaders (the coordinator
@@ -429,7 +429,7 @@ check "witness: no prepared-transaction leak" \
 psql_run "DROP TABLE IF EXISTS t_single; CREATE TABLE t_single (id int, txt text) USING pgcolumnar;" >/dev/null
 st_bad="$(err_of "SELECT pgcolumnar.parallel_copy('t_single'::regclass, '$F_BADKEY', 4)")"
 check "single table: a bad row fails the whole load" \
-	"$(printf '%s' "$st_bad" | grep -qiE 'invalid input syntax|failed' && echo ok || echo no)" ok
+	"$(grep -qiE 'invalid input syntax|failed' <<<"$st_bad" && echo ok || echo no)" ok
 check "single table: failed load leaves the target empty" "$(q "SELECT count(*) FROM t_single")" 0
 check "single table: no prepared-transaction leak after failure" "$(q "SELECT count(*) FROM pg_prepared_xacts")" 0
 

test/parallel_export_parquet.sh

@@ -39,7 +39,7 @@ err_of() {
 expect_error() {
 	local label="$1" sql="$2" out
 	out="$(err_of "$sql")"
-	check "$label" "$(printf '%s' "$out" | grep -qi "ERROR" && echo error || echo ok)" error
+	check "$label" "$(grep -qi "ERROR" <<<"$out" && echo error || echo ok)" error
 }
 
 # ---- single columnar table: split by row-group ranges -----------------------