Pin six more guards to their property, not their caller count (#1078)

CHANGELOG.md

@@ -18,6 +18,45 @@ true until the next version shipped.
 
 ### Added
 
+- Six more guards counted their callers instead of pinning their property (#1078's
+  class). Two were blind to the defect they name.
+
+  #1078 repaired two arms in `native_fetch_projection.sh` that compared a whole-file
+  `grep -c` against a literal. A sweep of `test/` found **twelve** sites of that shape;
+  these are six of the remaining ten, in `native_fetch_cache.sh` and
+  `native_fetch_position.sh`.
+
+  THE REPAIR DIFFERS PER ARM, BECAUSE THE FAILURE DIRECTION DOES. A count over a call
+  site is blind; a count over a guarded FORM is blind and noisy; and a count is CORRECT
+  where the count is the property. Two arms in the sweep were left alone for that reason
+  -- `decode_interrupts.sh`'s `^#define COLUMNAR_DECODE_INTERRUPT(i)` would be a
+  redefinition if it appeared twice, and `native_saop_pushdown.sh`'s premise is
+  load-bearing for an `awk` range that would silently concatenate two expressions.
+
+      the entry key    both directions   -> self-referential, keyed N of N
+      the cid reject   noise only        -> scoped to the function that must contain it
+      the geometry     blind to 3 of 4   -> membership over all four compared fields
+      the discard      proxy for "where" -> the two functions named
+      rank, valOffset  noise only        -> scoped to pgcolumnar_fetch_row
+
+  Measured, every mutation compiling so the suite rebuilds and runs end to end:
+
+      case                        OLD arms            NEW arms
+      unkeyed group lookup        key=1     PASS      RED  keyed 1 of 2
+      second keyed lookup         key=2     RED       24 passed
+      rowCount dropped            geom=1    PASS      RED  rowCount
+      executor-end discard gone   discard=1 RED       RED  names the function
+      third discard call          discard=3 RED       24 passed
+      rank replaced by a walk     rank=0    RED       RED  rank prefix
+
+  **Both blindness rows are the case for this change**: an unkeyed lookup and a dropped
+  geometry field both leave the old arms green. The two noise rows are what fired on
+  #1077 and cost a correct PR a red.
+
+  No ledger change: `native_fetch_cache` and `native_fetch_position` have zero rows, so
+  they are two of the 249 uncovered suites and no check name here is a ledger key.
+  Verified rather than inherited.
+
 - The piped-loop sweep reported a clean tree without reading one (#1033).
 
   `selftest/400` proves its detector FIRES -- a fixture with a check inside a piped

test/native_fetch_cache.sh

@@ -195,17 +195,71 @@ check "the untouched rows are unchanged" \
 
 SRC="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)/src"
 
-check "the entry key includes the storage id" \
-	"$(grep -c 'e->storageId == storageId && e->groupNumber == groupNumber' "$SRC/columnar_reader.c")" "1"
+# PIN THE PROPERTY, NOT THE CALLER COUNT (#1078's class). Each of these four
+# counted a string across a whole file and compared it against a literal, which
+# asserts HOW MANY sites exist today rather than that every site is honest. The
+# failure direction differs per arm and so does the repair, which is why they are
+# not all rewritten the same way:
+#
+#   the entry key    both directions -- a second honest lookup reddens it, and a
+#                    lookup that drops the storage id is invisible. Self-referential.
+#   the cid reject   noise only -- it is an EXISTENCE claim, and a second honest
+#                    site reddened it. Scoped to the function that must contain it.
+#   the geometry     the count was a proxy for one of FOUR compared fields, and
+#                    said nothing about the other three. Membership instead.
+#   the discard      the count 2 was a proxy for "from these two places". Name them.
+
+_nfc_slot="$(awk '/^pgcolumnar_fetch_group_slot\(/,/^}/' "$SRC/columnar_reader.c")"
+_nfc_row="$(awk '/^pgcolumnar_fetch_row\(/,/^}/' "$SRC/columnar_reader.c")"
+
+check "premise: the fetch-cache functions were extracted, not empty ranges" \
+	"$([ -n "$_nfc_slot" ] && [ -n "$_nfc_row" ] && echo yes || echo no)" "yes"
+
+# EVERY group-number comparison is keyed by the storage id as well. Honest extra
+# lookups move both counts together; one that forgets the storage id moves only
+# the total. Both numbers are in the compared strings, so the message reconciles.
+_nfc_grp="$(grep -c 'e->groupNumber == groupNumber' "$SRC/columnar_reader.c")"
+_nfc_keyed="$(grep -c 'e->storageId == storageId && e->groupNumber == groupNumber' \
+	"$SRC/columnar_reader.c")"
+
+check "premise: there is a group-number comparison to key" \
+	"$([ "${_nfc_grp:-0}" -ge 1 ] && echo yes || echo no)" "yes"
+
+check "every group-number comparison is keyed by the storage id too" \
+	"keyed $_nfc_keyed of $_nfc_grp" "keyed $_nfc_grp of $_nfc_grp"
 
+# `case` OVER A CAPTURED STRING, NOT A PIPE INTO grep -q. `grep -q` exits on its
+# first match and closes the pipe, so the writer can take SIGPIPE -- #486, and
+# selftest/080 refuses it. The same file family already has the answer at
+# native_fetch_projection.sh:155: a `case` needs no subprocess and no pipe.
 check "an entry from an earlier command is rejected" \
-	"$(grep -c 'e->cid != cid' "$SRC/columnar_reader.c")" "1"
+	"$(case "$_nfc_slot" in *'e->cid != cid'*) echo yes ;; *) echo no ;; esac)" "yes"
 
-check "a hit re-checks the group geometry it was filled with" \
-	"$(grep -cE 'entry->fileOffset != rg->fileOffset' "$SRC/columnar_reader.c")" "1"
+# The recheck compares four fields. Counting one of them asserted nothing about
+# the other three, and a reflow that dropped one would have passed.
+for _f in firstRowNumber rowCount fileOffset natts; do
+	check "a hit re-checks the group's $_f against the row group" \
+		"$(case "$_nfc_row" in \
+			*"entry->$_f != rg->$_f"*|*"entry->$_f != natts"*) echo yes ;; \
+			*) echo no ;; esac)" "yes"
+done
+
+# Named call sites rather than a count of 2: a third honest caller is not a defect,
+# and losing either of these two is.
+#
+# CAPTURED FIRST, WITH ITS OWN PREMISE, so "this function does not discard the
+# cache" and "the awk range matched nothing" are different answers. Without the
+# premise a renamed function reads as a missing call, which is the both-readings
+# problem a literal match always has.
+for _fn in pgcolumnar_executor_end pgcolumnar_xact_callback; do
+	_nfc_fn="$(awk "/^$_fn\\(/,/^}/" "$SRC/columnar_tableam.c")"
 
-check "the cache is released at executor end, not only at transaction end" \
-	"$(grep -c 'PgColumnarDiscardFetchCache' "$SRC/columnar_tableam.c")" "2"
+	check "premise: $_fn was extracted, not an empty range" \
+		"$([ -n "$_nfc_fn" ] && echo yes || echo no)" "yes"
+
+	check "the fetch cache is discarded from $_fn" \
+		"$(case "$_nfc_fn" in *PgColumnarDiscardFetchCache*) echo yes ;; *) echo no ;; esac)" "yes"
+done
 
 # The SETs every fetch-path guard below runs under, defined once because the
 # premise that asserts the plan and the measurement that depends on it must not

test/native_fetch_position.sh

@@ -227,10 +227,23 @@ check "doubling the row group does not double the cost of the same fetches" \
 # already built from it and make had nothing to do.
 SRC="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)/src"
 
+# PIN THE PROPERTY, NOT THE CALLER COUNT (#1078's class). Both counted a string
+# across the whole file against a literal 1. These two are EXISTENCE claims, so the
+# direction they failed in is noise: a second honest use of either mechanism
+# elsewhere in the reader would have reddened them while the property held. Scoped
+# to the function the claim is about, which also says WHERE it must be true.
+_nfp_row="$(awk '/^pgcolumnar_fetch_row\(/,/^}/' "$SRC/columnar_reader.c")"
+
+check "premise: pgcolumnar_fetch_row was extracted, not an empty range" \
+	"$([ -n "$_nfp_row" ] && echo yes || echo no)" "yes"
+
+# `case`, not a pipe into `grep -q`: that exits on its first match and closes the
+# pipe under its writer (#486), which selftest/080 refuses. The pattern is quoted so
+# the brackets in valOffset[c][present] are literal rather than glob classes.
 check "the rank comes from a prefix rather than a loop over earlier rows" \
-	"$(grep -c 'present = pgcolumnar_rank_before' "$SRC/columnar_reader.c")" "1"
+	"$(case "$_nfp_row" in *'present = pgcolumnar_rank_before'*) echo yes ;; *) echo no ;; esac)" "yes"
 
 check "a varying-length column reaches its value through an offset table" \
-	"$(grep -c 'entry->valOffset\[c\]\[present\]' "$SRC/columnar_reader.c")" "1"
+	"$(case "$_nfp_row" in *'entry->valOffset[c][present]'*) echo yes ;; *) echo no ;; esac)" "yes"
 
 pgc_summary