fix: a missing pyarrow declines a suite instead of failing it (#1215)

.github/workflows/ci.yml

@@ -259,6 +259,12 @@ jobs:
           test -n "$WANT"
           echo "running $(set -- $FILES; echo $#) cluster file(s): $FILES"
           echo "expecting $WANT collected test(s)"
+          # PYARROW IS REQUIRED HERE (#1215). A ported pyarrow test declines green on a
+          # developer box without it; in CI that would be green forever, so CI proves
+          # the module imports and sets PGC_REQUIRE, which turns a decline into a
+          # failure (OffgridwithJD, review of #1286).
+          /tmp/pgcvenv/bin/python -c 'import pyarrow, pyarrow.parquet; print("pyarrow", pyarrow.__version__)'
+          export PGC_REQUIRE=pyarrow
           PYTHONPATH=. /tmp/pgcvenv/bin/pytest -q \
             --pg-config /usr/lib/postgresql/${{ matrix.pg }}/bin/pg_config \
             --pgc-expect-tests "$WANT" \

CHANGELOG.md

@@ -164,6 +164,19 @@ true until the next version shipped.
   release notes' repair procedure, reading its query from the notes, and a docs
   check refuses any documented query that tests `pg_extension` while listing
   databases.
+- Without pyarrow, the 25 Arrow and Parquet suites that call `pgc_skip pyarrow`
+  now report `SKIPPED` and exit 66, and the run does not fail (#1215). Before,
+  each reported one `FAIL` and reddened the whole run on a box without pyarrow.
+  The owner decided this on 2026-10-12. Every other missing dependency, such as
+  `btree_gist` for `temporal.sh`, still fails unless it is waived. The nightly
+  workflow installs pyarrow and stops if the suites cannot import it, so CI
+  does not lose these suites silently. `docs/testing.md` also said a skipped
+  suite exits 2; it exits 66, and the page now says so.
+  The nightly `capability_sweep.sh` counts these suites as `declined`, a fourth
+  outcome beside `preserved`, `refused` and `silent`. A suite is `declined` only
+  if it records a SKIP naming the capability, and `silent` must still be zero.
+  The sweep now also asserts that the suites that decline are exactly the ones
+  whose code calls `pgc_skip pyarrow`.
 
 - `test/pytest/README.md` now names all four registries a new pytest test has to
   be entered in, and which of them a local run can catch. Adding one test reddens

docs/testing.md

@@ -317,30 +317,37 @@ you got before deciding whether something is wrong.
 | outcome | exit | when |
 | --- | :-: | --- |
 | `PASSED` | 0 | the suite ran its checks and they held |
-| `SKIPPED (ran no checks)` | 2 | the feature under test does not exist on this major |
+| `SKIPPED (ran no checks)` | 66 | the feature under test does not exist on this major, or pyarrow is not installed |
 | `FAILED` | 1 | a check failed, **or** a tool the suite measures with is missing |
 
 The matrix prints `suites that ran: n of m (skipped: k)` for each major, and names
 the skipped suites. A skip counts in `k`, not in `n`.
 
-**A missing dependency fails rather than skips, and that is deliberate.** The box
-cannot gate that suite. Reporting a pass would claim coverage nobody has. Two
+**A missing dependency fails rather than skips, except pyarrow.** The box cannot
+gate that suite, and reporting a pass would claim coverage nobody has. Two
 dependencies come from outside this repository:
 
 | suite | needs | where it comes from |
 | --- | --- | --- |
 | the Arrow and Parquet suites | `pyarrow` | `pip install pyarrow`, as the user the suites run as |
 | `temporal.sh` | `btree_gist` | PostgreSQL **contrib**. A PGDG package ships it; a source build configured without contrib does not |
 
+**Without pyarrow, the Arrow and Parquet suites report `SKIPPED` and the run does
+not fail** (#1215). A box without pyarrow is a supported developer configuration.
+The matrix still names each skipped suite. The nightly workflow installs pyarrow
+and stops if the suites' interpreter cannot import it, so CI does not lose these
+suites silently.
+
 Build and install `contrib/btree_gist` against each source tree when the matrix
-runs against source builds. To run knowingly without a dependency, waive it:
+runs against source builds. To run knowingly without it, or without any other
+dependency, waive it:
 
 ```sh
 PGC_ALLOW_MISSING_BTREE_GIST=1 test/temporal.sh /path/to/pg_config
 PGC_ALLOW_MISSING=1            test/run_all_versions.sh
 ```
 
-A waived suite reports `SKIPPED` and exits 2, so the coverage loss stays visible
+A waived suite reports `SKIPPED` and exits 66, so the coverage loss stays visible
 in the skipped count rather than becoming a pass.
 
 Measured on PostgreSQL 18, one suite, four configurations:
@@ -349,8 +356,8 @@ Measured on PostgreSQL 18, one suite, four configurations:
 | --- | :-: | --- |
 | `btree_gist` present | 0 | `PASSED`, 5 checks |
 | `btree_gist` absent | 1 | `FAILED` |
-| `btree_gist` absent, waived | 2 | `SKIPPED` |
-| PostgreSQL 17, feature absent | 2 | `SKIPPED (ran no checks)` |
+| `btree_gist` absent, waived | 66 | `SKIPPED` |
+| PostgreSQL 17, feature absent | 66 | `SKIPPED (ran no checks)` |
 
 ## Cross-major upgrade
 

test/capability_sweep.sh

@@ -160,22 +160,37 @@ check_num "premise: and pyarrow IS importable without the shim, so absence is th
 
 # ---- the verdict, a pure function so the selftest can drive it -------------
 #
-# Two inputs and three outcomes. It takes the numbers rather than the logs so it
+# Three inputs and four outcomes. It takes the numbers rather than the logs so it
 # can be exercised without running a suite, the same reason pgc_freshness_verdict
 # and pgc_build_needs_clean take theirs.
-pgc_capability_verdict() {	# pgc_capability_verdict LOST_NAMES FAIL_RECORDS -> verdict
-	local lost="${1:-}" fails="${2:-}"
+#
+# DECLINED IS ITS OWN OUTCOME (#1215, the owner's rule of 2026-10-12): a missing
+# capability may be bypassed as a SKIP, but no test may be silent. A suite that
+# loses its names and records a SKIP naming the capability has said what it did
+# not run; a suite that loses them and records nothing has not. Folding the first
+# into `silent` reddened the sweep on the 25 suites the rule allows to skip, and
+# folding it into `preserved` would hide them. So the two stay apart, and `silent`
+# must still be zero.
+pgc_capability_verdict() {	# pgc_capability_verdict LOST FAILS NAMED_SKIPS -> verdict
+	local lost="${1:-}" fails="${2:-}" skips="${3:-}"
 	case "$lost" in ''|*[!0-9]*) echo unknown; return ;; esac
 	case "$fails" in ''|*[!0-9]*) echo unknown; return ;; esac
+	case "$skips" in ''|*[!0-9]*) echo unknown; return ;; esac
 	if [ "$lost" -eq 0 ]; then
 		echo preserved
 	elif [ "$fails" -ge 1 ]; then
 		echo refused
+	elif [ "$skips" -ge 1 ]; then
+		echo declined
 	else
 		echo silent
 	fi
 }
 
+# Its four cases are asserted by selftest part 610, which evals this text: they
+# need no suite run, and a sweep built on a verdict that misfiled one case would
+# count it wrongly thirty times over and still reconcile.
+
 cap_names() {	# cap_names LOGFILE -> the recorded check names, sorted
 	grep -oP '^RESULT\t[^\t]*\t[^\t]*\t\K[^\t]*' "$1" 2>/dev/null | LC_ALL=C sort -u
 }
@@ -195,8 +210,10 @@ check_num "premise: the priming run built and installed cleanly" "$prime_rc" "0"
 
 n_preserved=0
 n_refused=0
+n_declined=0
 n_silent=0
 silent_names=""
+declined_names=""
 mask_failures=0
 
 for s in $NARROW; do
@@ -211,22 +228,47 @@ for s in $NARROW; do
 	[ "$(cap_rc "$SHIM" 'import pyarrow')" = 1 ] || mask_failures=$((mask_failures + 1))
 	lost="$(LC_ALL=C comm -23 <(cap_names "$w_log") <(cap_names "$o_log") | grep -c .)"
 	fails="$(grep -cP '^RESULT\t.*\tFAIL\t' "$o_log")"
-	case "$(pgc_capability_verdict "$lost" "$fails")" in
+	# A SKIP COUNTS ONLY IF ITS REASON NAMES THE CAPABILITY, read from the record
+	# rather than from the exit code: exit 66 also means a major without the
+	# feature, which is not this.
+	skips="$(awk -F'\t' '$1 == "RESULT" && $5 == "SKIP" && $7 ~ /pyarrow/' "$o_log" | grep -c .)"
+	case "$(pgc_capability_verdict "$lost" "$fails" "$skips")" in
 		preserved) n_preserved=$((n_preserved + 1)) ;;
 		refused)   n_refused=$((n_refused + 1)) ;;
+		declined)  n_declined=$((n_declined + 1)); declined_names="$declined_names $s" ;;
 		*)         n_silent=$((n_silent + 1)); silent_names="$silent_names $stem" ;;
 	esac
 	rm -f "$w_log" "$o_log"
 done
 
-echo "-- preserved=$n_preserved refused=$n_refused silent=$n_silent of $n_narrow"
+echo "-- preserved=$n_preserved refused=$n_refused declined=$n_declined silent=$n_silent of $n_narrow"
 [ -n "$silent_names" ] && echo "-- silent:$silent_names"
 
 check_num "premise: the mask held for every run in the sweep" "$mask_failures" "0"
 check_num "premise: every suite in the population was classified" \
-	"$((n_preserved + n_refused + n_silent))" "$n_narrow"
+	"$((n_preserved + n_refused + n_declined + n_silent))" "$n_narrow"
 check_num "no suite loses a check without recording a skip or a refusal" \
 	"$n_silent" "0"
 
+# THE SUITES THAT DECLINE ARE THE SUITES THAT ASK TO, derived from the source and
+# compared as SETS. A count would let a suite that stopped declining trade places
+# with one that started; a literal 25 would go stale the day a suite is added. A
+# suite that stops calling pgc_skip pyarrow leaves this side, and must then read
+# preserved, or the silent arm above catches it.
+DECLINERS="$(for _cap_b in $NARROW; do
+	# grep -c, NOT grep -q: -q exits at the first match, sed dies of SIGPIPE, and
+	# under pipefail the pipeline fails, so a suite that DOES call pgc_skip drops
+	# out at random. Measured here: three of 25 dropped on one run. The header of
+	# this file records the same race for the population itself.
+	[ "$(sed 's/#.*//' "$TESTDIR/$_cap_b" | grep -cE 'pgc_skip[[:space:]]+"?pyarrow')" -gt 0 ] && printf '%s\n' "$_cap_b"
+done | LC_ALL=C sort)"
+check_num "premise: some suites in the population call pgc_skip pyarrow" \
+	"$([ -n "$DECLINERS" ] && echo 1 || echo 0)" "1"
+_cap_diff="$(LC_ALL=C comm -3 <(printf '%s\n' "$DECLINERS") \
+	<(printf '%s\n' $declined_names | LC_ALL=C sort) | tr -d '\t' | grep .)"
+[ -n "$_cap_diff" ] && echo "-- decline mismatch: $(printf '%s ' $_cap_diff)"
+check_num "the suites that decline are exactly the suites that call pgc_skip pyarrow" \
+	"$(printf '%s\n' "$_cap_diff" | grep -c .)" "0"
+
 rm -rf "$SHIM" "$prime_log"
 pgc_summary

test/check_ledger.tsv

@@ -48,19 +48,21 @@ base_scan_io	base_scan_io	premise: the later plan is still a base columnar scan
 base_scan_io	base_scan_io	premise: the later plan still does not name a covering projection	15;16;17;18;19	never	-
 base_scan_io	base_scan_io	premise: the plan is a base columnar scan	15;16;17;18;19	never	-
 base_scan_io	base_scan_io	premise: the table holds every inserted row	15;16;17;18;19	never	-
-capability_sweep	capability_sweep	no suite loses a check without recording a skip or a refusal	15;16;17;18;19	2026-09-23	a check_skip block removed from a suite so its arms vanish with no skip and no fail
+capability_sweep	capability_sweep	no suite loses a check without recording a skip or a refusal	15;16;17;18;19	2026-10-12	a check_skip block removed from a suite so its arms vanish with no skip and no fail;the pyarrow decline reason stops naming the capability, so no skip counts as declined
 capability_sweep	capability_sweep	premise: and no member of the population is this very file	15;16;17;18;19	2026-09-23	the self-exclusion removed from pgc_cap_population, so the sweep enters its own population
 capability_sweep	capability_sweep	premise: and pyarrow IS importable without the shim, so absence is the shim's doing	15;16;17;18;19	never	-
 capability_sweep	capability_sweep	premise: and pyarrow.parquet with it	15;16;17;18;19	2026-09-23	the shim's pyarrow.py emptied, so both runs are the present run
 capability_sweep	capability_sweep	premise: and the standard library still imports under it	15;16;17;18;19	never	-
 capability_sweep	capability_sweep	premise: every suite in the population was classified	15;16;17;18;19	never	-
+capability_sweep	capability_sweep	premise: some suites in the population call pgc_skip pyarrow	15;16;17;18;19	never	-
 capability_sweep	capability_sweep	premise: the mask held for every run in the sweep	15;16;17;18;19	2026-09-23	the shim's pyarrow.py emptied, so both runs are the present run
 capability_sweep	capability_sweep	premise: the population is the size this tree holds	15;16;17;18;19	never	-
 capability_sweep	capability_sweep	premise: the priming run built and installed cleanly	15;16;17;18;19	never	-
 capability_sweep	capability_sweep	premise: the shim makes pyarrow unimportable	15;16;17;18;19	2026-09-23	the shim's pyarrow.py emptied, so both runs are the present run
 capability_sweep	capability_sweep	premise: the sweep is not in the population it sweeps	15;16;17;18;19	2026-09-23	the self-exclusion removed from pgc_cap_population, so the sweep enters its own population
 capability_sweep	capability_sweep	premise: the two derivations of the population name the same suites	15;16;17;18;19	never	-
 capability_sweep	capability_sweep	premise: while python itself still runs under the shim	15;16;17;18;19	never	-
+capability_sweep	capability_sweep	the suites that decline are exactly the suites that call pgc_skip pyarrow	15;16;17;18;19	2026-10-12	the pyarrow decline reason stops naming the capability, so no skip counts as declined
 catalog_delete_index	catalog_delete_index	A1 with more catalog pages the default does less work than reading every one whole	15;16;17;18;19	2026-10-12	pgcolumnar_index_min_blocks default 3 -> 2147483647, so no site probes;pgcolumnar_scan_index_oid stops asking the relation its size, so every site probes;the eight converted scan sites back to InvalidOid
 catalog_delete_index	catalog_delete_index	B1 with large catalogs the default does far less work than reading every one whole	15;16;17;18;19	2026-10-12	pgcolumnar_index_min_blocks default 3 -> 2147483647, so no site probes;pgcolumnar_scan_index_oid stops asking the relation its size, so every site probes;the eight converted scan sites back to InvalidOid
 catalog_delete_index	catalog_delete_index	P1 with a few catalog pages the default does less work than probing every one	15;16;17;18;19	2026-10-12	pgcolumnar_scan_index_oid stops asking the relation its size, so every site probes;the eight converted scan sites back to InvalidOid
@@ -1599,6 +1601,16 @@ harness_selftest	590-a-declared-suite-must-account	the EXIT trap runs the wrappe
 harness_selftest	590-a-declared-suite-must-account	the line it emits is the line the reconciliation reads	15;16;17;18;19	2026-09-23	the EXIT trap reverted to the bare pgc_teardown, so the eight pgc_setup exits emit no accounting
 harness_selftest	590-a-declared-suite-must-account	the suite's own counters are restored after every driving block	15;16;17;18;19	never	-
 harness_selftest	590-a-declared-suite-must-account	the wrapper captures the exit status before anything overwrites it	15;16;17;18;19	2026-09-23	the EXIT trap reverted to the bare pgc_teardown, so the eight pgc_setup exits emit no accounting
+harness_selftest	600-a-missing-pyarrow-declines	a missing pyarrow declines the suite instead of failing it	15;16;17;18;19	2026-10-12	pgc_skip treats pyarrow like any other capability again, so a missing pyarrow fails the suite
+harness_selftest	600-a-missing-pyarrow-declines	and it records a skip under the check's own name, not a failure	15;16;17;18;19	2026-10-12	pgc_skip treats pyarrow like any other capability again, so a missing pyarrow fails the suite
+harness_selftest	600-a-missing-pyarrow-declines	control: a missing btree_gist still fails the suite	15;16;17;18;19	2026-10-12	pgc_skip declines every missing capability, not only pyarrow
+harness_selftest	600-a-missing-pyarrow-declines	control: and an explicit waiver still turns that failure into a skip	15;16;17;18;19	2026-10-12	pgc_skip ignores PGC_ALLOW_MISSING_<CAPABILITY>, so only the run-wide waiver works
+harness_selftest	600-a-missing-pyarrow-declines	premise: lib.sh defines pgc_skip	15;16;17;18;19	never	-
+harness_selftest	610-a-declined-capability-is-not-silent	premise: the sweep defines its verdict where this part reads it	15;16;17;18;19	never	-
+harness_selftest	610-a-declined-capability-is-not-silent	the verdict: names lost beside a failure are refused	15;16;17;18;19	2026-10-12	pgc_capability_verdict answers declined for every input
+harness_selftest	610-a-declined-capability-is-not-silent	the verdict: names lost beside a skip naming the capability are declined	15;16;17;18;19	2026-10-12	pgc_capability_verdict has no declined outcome, so a lost name beside a skip reads silent
+harness_selftest	610-a-declined-capability-is-not-silent	the verdict: names lost with neither a failure nor a skip are silent	15;16;17;18;19	2026-10-12	pgc_capability_verdict answers declined for every input
+harness_selftest	610-a-declined-capability-is-not-silent	the verdict: no name lost is preserved	15;16;17;18;19	2026-10-12	pgc_capability_verdict answers declined for every input
 index_am_support	index_am_support	btree answers its operator with the same rows the scan returns	15;16;17;18;19	2026-09-23	pgcolumnar_index_build_range_scan drops two rows in three, so the index misses entries
 index_am_support	index_am_support	btree builds an index over a columnar table	15;16;17;18;19	2026-09-23	pgcolumnar_index_build_range_scan errors, so CREATE INDEX fails
 index_am_support	index_am_support	gist answers its operator with the same rows the scan returns	15;16;17;18;19	2026-09-23	pgcolumnar_index_build_range_scan drops two rows in three, so the index misses entries

test/check_ledger_budget.txt

@@ -1276,4 +1276,24 @@ suites_not_covered 247
 #     never         1664 +  7 = 1671
 #     dated          219 +  5 =  224
 #     1671 + 224 == 1895
-checks_never_observed_red 1671
+#
+# #1215 (PR #1286): 1671 -> 1674. Twelve new checks, nine dated, three premises:
+#
+#     harness_selftest 600   5 checks: 4 dated, 1 premise
+#     harness_selftest 610   5 checks: 4 dated, 1 premise
+#     capability_sweep       2 checks: 1 dated, 1 premise; and the silent arm
+#                                       re-dated
+#
+# Every date comes from a mutation run on all five majors, merged one log at a
+# time with --mutation and --target, replayed from the saved logs onto #1288's
+# ledger. The part 600 mutation logs predate #1284's renames, so the three
+# old-name rows they carried were removed again, as #1284 did. Carried onto
+# #1302's ledger unchanged: #1291's rows are another suite, and the result is
+# main's rows less the one re-dated row plus 13 (12 new, 1 re-dated), checked
+# row by row.
+#
+#     ledger rows   1895 + 12 = 1907
+#     never         1671 +  3 = 1674
+#     dated          224 +  9 =  233
+#     1674 + 233 == 1907
+checks_never_observed_red 1674

test/lib.sh

@@ -1971,17 +1971,29 @@ chunk_group_count() {
 # deliberately not the same code path. PostgreSQL 15 has no WITHOUT OVERLAPS to
 # test and no amount of installing will give it one, so those gates call
 # pgc_summary directly and report SKIPPED. Nothing is broken there. Here it is.
+#
+# EXCEPT PYARROW, which declines by default (#1215, owner's decision 2026-10-12).
+# A box without pyarrow is a supported developer configuration, and the 25 suites
+# that need it were the reddest thing such a box produced. The nightly installs
+# pyarrow and stops when the suites' interpreter cannot import it, so CI cannot
+# lose them silently; the run summary still names every suite that skipped.
+# Every other capability keeps the rule above.
 pgc_skip() {  # pgc_skip <capability> <message>
-	local cap allow_one
+	local cap allow_one why
 	cap="$(printf '%s' "$1" | tr '[:lower:]-' '[:upper:]_')"
 	allow_one="PGC_ALLOW_MISSING_$cap"
-	if [ "${PGC_ALLOW_MISSING:-0}" = 1 ] || [ "${!allow_one:-0}" = 1 ]; then
+	why=""
+	if [ "$cap" = PYARROW ]; then
+		why="$1 is optional on a developer box, see #1215"
+	elif [ "${PGC_ALLOW_MISSING:-0}" = 1 ] || [ "${!allow_one:-0}" = 1 ]; then
+		why="waived by $allow_one or PGC_ALLOW_MISSING"
+	fi
+	if [ -n "$why" ]; then
 		# The unwaived branch below records a FAIL. This one printed and left
 		# PGC_CHECKS at zero, so waiving a dependency also erased the outcome --
 		# the same asymmetry check_timing had, in the function whose whole
 		# subject is "a missing dependency is not a pass".
-		check_skip "$2" "SKIP  $2 (waived by $allow_one or PGC_ALLOW_MISSING)" \
-			"waived by $allow_one or PGC_ALLOW_MISSING"
+		check_skip "$2" "SKIP  $2 ($why)" "$why"
 		pgc_summary
 	fi
 	pgc_record FAIL "$2" "FAIL  $2"