Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 62 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -383,6 +383,68 @@ true until the next version shipped.
because a map that names the wrong gap is worse than one that admits it does not
know.

- 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`.

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.

- TESTS.md's contents list no longer carries a link that goes nowhere, and the
corpus gate now checks every one of them.

Expand Down
11 changes: 7 additions & 4 deletions bench/run_clickbench.sh
Original file line number Diff line number Diff line change
Expand Up @@ -815,12 +815,15 @@ run_one() { # run_one <arm> <sql>
# 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 <arm> <sql>
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
Expand Down
4 changes: 2 additions & 2 deletions test/native_groupagg_batch.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading
Loading