native_agg reports the metadata aggregate node as absent when it is present: EPIPE into grep -q under pipefail

[jdatcmd]

Hit on #484's CI, PG18. The same job re-run on the identical commit passed, so
this is a flake, and the log says exactly which one.

What happened

/tmp/pgcolumnar-matrix-18.2QoMGY/test/native_agg.sh: line 41: echo: write error: Broken pipe
FAIL  count(*) uses the metadata agg node: got [no] want [yes]
PASS  sum/min/max uses the metadata agg node

The check that failed and the check that passed on the next line call the same
helper on plan text from the same node.

Why

test/native_agg.sh:41:

has_aggnode() { echo "$1" | grep -q 'Columnar Vectorized Aggregates' && echo yes || echo no; }

The suite runs under set -o pipefail. grep -q exits as soon as it matches, so
the writer can take EPIPE, and pipefail then calls the whole pipeline failed.
The && arm is skipped and the helper answers no while the line it was looking
for is present.

This is the shape #473 already found once, in harness_selftest:

Never pipe into an early-exit reader under pipefail. The tell is a
Broken pipe line beside a result the same run's summary contradicts.

That is precisely the tell here.

Why it is worth fixing rather than tolerating

The failure direction is the dangerous one. It reports "the metadata aggregate
node did not run" when it did, which reads as a real planner regression in
exactly the area #133 and #140 live in. Anyone triaging it starts by looking for
a costing change that is not there.

has_gather() on line 42 has the identical shape.

Fix

No pipe and no subprocess:

has_aggnode() { case "$1" in *"Columnar Vectorized Aggregates"*) echo yes;; *) echo no;; esac; }

case cannot take EPIPE and is faster besides.

Scope

About twenty instances of <string> | grep -q exist across test/, including
native_ownership.sh:34, native_groupagg.sh:235 and :326, and several in
fuzz_arrow.sh and fuzz_parquet.sh. They are not all equally exposed: the
larger the string, the likelier the writer is still writing when the reader
exits. Worth sweeping the lot in one pass, since the fix is mechanical and the
failure mode is a false red that names innocent code.

A harness_selftest check that greps test/*.sh for the shape would keep it
from coming back, which is what stopped the SUITES mistake recurring.

[jdatcmd]

Correcting the mechanism I wrote above, because part of it does not survive measurement.

Measured, deterministically:

form	300 KB string, match on the first line	small string
echo "$s" | grep -q	no, which is wrong	yes
case "$s" in *pat*)	yes	yes
grep -q pat <<<"$s"	yes	yes

So the class is real and both replacements cure it. But the explanation I gave for THIS failure does not hold: an EXPLAIN plan is about a kilobyte, it fits entirely in the 64 KB pipe buffer, so the writer's single write() completes before grep can have matched anything. grep -q exiting early cannot by itself produce EPIPE at that size.

For the reader's end to be closed at that size, the reader has to have gone away for some other reason. Under a six-way parallel matrix on a CI runner, a grep killed under memory pressure is the plausible one. I have not proved that, and I am not going to claim it without evidence.

What this does not change is the defect or the fix, and it arguably strengthens both. The real problem is not "grep exits early". It is that the helper's answer depends on the reader process surviving, and pipefail converts anything that happens to that process into a wrong answer in the direction of "the thing you were looking for is not there". case has no second process, no pipe, and no such failure mode, whatever killed the reader.

The size table above is still the reason to sweep the other instances rather than only fix line 41: several of them grep over server log tails and fuzzer output, which are not a kilobyte.

[ChronicallyJD]

Diagnosis and fix both look right to me, and the case form is the one I would use. Two things to add: the class is wider than native_agg.sh, and one instance is in test/lib.sh.

To be clear about what I did and did not establish: I have not reproduced EPIPE on any of these. What I checked is the shape — writer piped into an early-exit reader, under pipefail, with the pipeline's exit status deciding a yes/no the suite then asserts on. Your PG18 log is the only confirmed firing. The list below is "same shape, same failure direction", not "also broken today".

The shape, ranked by how exposed the writer is

grep -q exits on the first match, so EPIPE requires the writer to still have data to write. That makes the writer's size and its nature decisive:

External writer (psql) — most exposed. A real process, a real SIGPIPE, exit 141, and a whole plan still queued in the pipe:

test/lib.sh:521	pgc_is_columnar_scan() — shared, called by 8 suites: native_bloom, native_fastdecode, native_skip, native_recluster, native_sort_by, native_cluster, sorted_projection, write_minmax_fastpath
test/native_groupagg.sh:50	pgc_is_groupvec()
test/arrow_import.sh:172	idx_plan_is_index_scan()

Builtin writer (echo / printf) — the native_agg.sh case. Short strings usually clear in one write, which is why this surfaces as a rare flake rather than a steady red: native_truncate.sh:87, native_groupagg.sh:235 and :326, cancel_decode.sh:82, and eleven in parallel_copy.sh (193, 201, 268, 302, 317, 330, 340, 348, 355, 363, 432).

Not exposed, and worth saying so — the here-string forms are correct as written and need no change, because a here-string is not a pipe: advisory_lock_class.sh:138, native_ctas.sh:51. Likewise the ... | grep -oE ... | head -1 value extractions are fine — the pipeline's exit status is discarded and head has already printed the value.

Why the lib.sh one is worth more than the others

The comment directly above it makes the design intent explicit:

A positive grep for the marker is deliberately not an absence test — a plan that fell back to a sequential scan has no Columnar lines to be absent, so an absence test would pass for exactly the case worth catching.

The EPIPE flip produces precisely the outcome that reasoning was written to prevent. The marker is present, the helper answers no, and the suite reports "this query did not use the columnar scan" — a planner regression that is not there, in eight suites, in the same neighbourhood as #133 and #140. Same failure direction you identified, one layer more load-bearing.

The three psql helpers are also the ones where the writer has the most left to write: a full EXPLAIN plan, against grep -q that exits at the first line matching a marker which is typically early in that plan. That is the worst combination of the two factors.

One note on the fix

case is right for the builtin-writer cases. For the three psql ones the pipe has to go regardless, so it wants a capture first:

pgc_is_columnar_scan() {	# query -> yes|no
	local plan
	plan="$(env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" -U postgres \
		-d "$PGC_DB" -At -c "EXPLAIN (COSTS OFF) $1" 2>/dev/null)"
	case "$plan" in *'Columnar Projected Columns'*) echo yes;; *) echo no;; esac
}

That also removes the second, quieter hazard in the current form: psql failing outright is presently indistinguishable from "the marker was absent", so a broken query reads as a planner fallback. Capturing first lets the two be told apart if anyone ever wants to.

Happy to send this as a PR if you would rather not carry it — say the word and whether you want the lib.sh change separated from the suite-local ones, since the shared helper touches eight suites' behaviour and the rest are local.