From aa7a51be6bdb7f62aa722e6cb007aba58d684090 Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Sun, 26 Jul 2026 16:38:30 -0600 Subject: [PATCH] test: compile against every major before merge main did not compile on PostgreSQL 17 for a few hours, and the gate that approved the change could not have caught it. scan_analyze_next_block changed signature at PG17; #159 guarded the callback body at PG18; PG15 and PG16 build because they really are pre-17, PG18 and PG19 because they are past both guards. PG17 is the only major where the mismatch exists, and the per-PR cadence is PG18 and PG19. That cadence is the right trade for test time. What it cannot do is see a defect on a major it never builds, so this is the cheap half of the answer: no clusters, no suites, a compile against each installed major, about a minute for all five. Run it before merging anything that touches a version guard, a table AM callback signature, or columnar_compat.h. Proven by reintroducing the exact defect. With the guard back at 180000: PG15, PG16, PG18 and PG19 report OK and PG17 fails with src/columnar_tableam.c:619:67: error: 'blockno' undeclared Unmutated, all five report OK with zero warnings. It also reports warning counts per major, since -Werror is not set and a warning that only appears on one major would otherwise accumulate unnoticed, and it cleans the object tree afterwards so the next build against a different major cannot link objects compiled for this one. Allowlisted in the harness self-test rather than registered as a suite, with the reason recorded there: it takes no cluster and reports per major, so the matrix cannot run it the way it runs a suite. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_012uKWWwBDt5TWWS5DR2tzDb --- docs/testing.md | 20 ++++++++++ test/build_all_versions.sh | 76 ++++++++++++++++++++++++++++++++++++++ test/harness_selftest.sh | 8 ++-- 3 files changed, 101 insertions(+), 3 deletions(-) create mode 100755 test/build_all_versions.sh diff --git a/docs/testing.md b/docs/testing.md index 01ae305..4adcfd5 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -78,6 +78,26 @@ value-stream codecs (round-trip over randomized and boundary inputs): test/pbt/run.sh [seed] [iterations] ``` +## Before merging: build every major + +```sh +test/build_all_versions.sh +``` + +No clusters and no suites, just a compile against each installed major, about a +minute for all five. Run it before merging anything that touches a version guard, +a table access method callback signature, or `columnar_compat.h`. + +The per-PR gate runs the suites on two majors, which is the right trade for test +time and structurally cannot see a defect on a major it never builds. +`scan_analyze_next_block` changed signature at PG17; a change guarded the +callback at PG18 instead; PG15, PG16, PG18 and PG19 all built, and `main` did not +compile on PG17 at all while a two-major gate reported it green. This check +catches that in a minute. The full matrix remains the thorough answer. + +Do not leave a branch broken on a supported major while a fix is pending. Land +the fix. + ## The version matrix To build and run every suite across a set of PostgreSQL majors in one pass, each diff --git a/test/build_all_versions.sh b/test/build_all_versions.sh new file mode 100755 index 0000000..7262f53 --- /dev/null +++ b/test/build_all_versions.sh @@ -0,0 +1,76 @@ +#!/usr/bin/env bash +# +# Compile the extension against every installed major. +# +# The per-PR gate runs the suites on two majors, which is the right trade for +# test time and cannot see a defect on a major it never builds. That is not +# hypothetical: scan_analyze_next_block changed signature at PG17, a change +# guarded the callback at PG18 instead, and PG15, PG16, PG18 and PG19 all built +# fine while main did not compile on PG17 at all. A two-major gate reported it +# green. +# +# This is the cheap half of the answer: no clusters, no suites, just a compile +# against each major, which takes about a minute for all five. Run it before +# merging anything that touches a version guard, a table AM callback signature, +# or columnar_compat.h. The full matrix remains the thorough half. +# +# Usage: +# test/build_all_versions.sh [pg_config ...] +# +# With no arguments it builds against the same default set the version matrix +# uses. Exits non-zero on the first major that fails, and prints its errors. + +set -uo pipefail + +SRCDIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" + +DEFAULT_CONFIGS=( + /usr/local/pg15/bin/pg_config + /usr/local/pg16/bin/pg_config + /usr/local/pg17/bin/pg_config + /usr/local/pgsql/bin/pg_config + /usr/local/pg19/bin/pg_config +) + +if [ "$#" -gt 0 ]; then + CONFIGS=("$@") +else + CONFIGS=("${DEFAULT_CONFIGS[@]}") +fi + +echo "== pgColumnar build check across majors ==" + +failed=0 +for pgc in "${CONFIGS[@]}"; do + if [ ! -x "$pgc" ]; then + printf ' SKIP %-34s (not executable)\n' "$pgc" + continue + fi + + ver="$("$pgc" --version)" + log="$(mktemp /tmp/pgc-build-XXXXXX.log)" + + make -C "$SRCDIR" clean PG_CONFIG="$pgc" >/dev/null 2>&1 + if make -C "$SRCDIR" PG_CONFIG="$pgc" > "$log" 2>&1; then + # -Werror is not set, so a warning still builds; report it rather than + # let a new one accumulate unnoticed across majors. + warns="$(grep -cE '^[^ ].*\bwarning:' "$log")" + printf ' OK %-34s %s warning(s)\n' "$ver" "$warns" + [ "$warns" != "0" ] && grep -E '^[^ ].*\bwarning:' "$log" | head -5 | sed 's/^/ /' + else + printf ' FAIL %s\n' "$ver" + grep -E '\berror:' "$log" | head -8 | sed 's/^/ /' + failed=1 + fi + rm -f "$log" +done + +# Leave no object tree behind from whichever major happened to be last: the next +# build against a different major would link objects compiled for this one. +make -C "$SRCDIR" clean >/dev/null 2>&1 || true + +if [ "$failed" != "0" ]; then + echo "build_all_versions.sh: FAILED" + exit 1 +fi +echo "build_all_versions.sh: PASSED" diff --git a/test/harness_selftest.sh b/test/harness_selftest.sh index 539488f..cfbd203 100755 --- a/test/harness_selftest.sh +++ b/test/harness_selftest.sh @@ -158,13 +158,15 @@ check "guard rejects a foreign cluster" "$_verdict" "foreign" TESTDIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" RUNNER="$TESTDIR/run_all_versions.sh" -# Not suites: the shared library, the runner itself, and the two developer -# helpers that build rather than test. native_scale is a suite but is opt-in by +# Not suites: the shared library, the two runners, and the two developer helpers +# that build rather than test. build_all_versions compiles against every major +# and is run before merging a change that touches a version guard; it takes no +# cluster and reports per major, so the matrix cannot run it as a suite. native_scale is a suite but is opt-in by # design and says so in its own header: it runs at a row count the matrix should # not carry. not_a_suite() { case "$1" in - lib|run_all_versions|devloop|rebuild|native_scale) return 0 ;; + lib|run_all_versions|build_all_versions|devloop|rebuild|native_scale) return 0 ;; *) return 1 ;; esac }