From 2126fd061022072070b054eb268ff9af3cafd74b Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Sat, 1 Aug 2026 16:11:44 -0500 Subject: [PATCH 1/9] Phase 4: real pg_upgrade support via a reduced bin/test_existing Adds the pg-upgrade-test CI job: install 0.9.6 on an old PostgreSQL major, plant + prove a dependency guard, binary pg_upgrade to a newer major, ALTER EXTENSION UPDATE the migrated objects, then run the suite against the real upgraded database in existing mode. bin/test_existing is much smaller than the equivalent script would have been pre-test/install: only prepare-old and run-suite are genuinely external-to-pg_regress concerns (a real pg_upgrade binary run isn't something pg_regress can invoke itself), plus a small `update` subcommand for the post-upgrade ALTER EXTENSION UPDATE step. There's no update-scenario subcommand at all - that entire scenario is just `make test-update` now (test/install/load.sql's own 'update' mode, added in phase 3), since an in-place update has no external step to drive. run_suite() gates on plain `make test`, not the old belt-and-suspenders `make test && make verify-results` - pgxntool 2.3.0 (this repo's phase 0) already made `make test` itself exit non-zero on regression failures. Not yet crossed with TEST_SCHEMA - that's the next phase, once both this job and extension-update-test can cross it together. Verified locally against PG17 (prepare-old -> update -> run-suite, without a real pg_upgrade - this container's clusters are persistent shared infra, so the actual binary pg_upgrade leg is left for CI's ephemeral containers, same reasoning as the pg-tle-test work). Co-Authored-By: Claude Sonnet 5 --- .github/workflows/ci.yml | 108 ++++++++++++- bin/test_existing | 210 +++++++++++++++++++++++++ bin/test_existing.sql/assert_guard.sql | 35 +++++ bin/test_existing.sql/drop_guard.sql | 11 ++ bin/test_existing.sql/plant_guard.sql | 30 ++++ 5 files changed, 393 insertions(+), 1 deletion(-) create mode 100755 bin/test_existing create mode 100644 bin/test_existing.sql/assert_guard.sql create mode 100644 bin/test_existing.sql/drop_guard.sql create mode 100644 bin/test_existing.sql/plant_guard.sql diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9a91946..e0dbf18 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -16,6 +16,16 @@ # own job would only duplicate this job's own # per-PG-version container/checkout setup for # no added confidence. +# pg-upgrade-test -- BINARY pg_upgrade: install 0.9.6 on an OLD +# PostgreSQL major, binary-upgrade the cluster +# to a NEWER major, then update the extension +# to current - proves objects created on an +# old server still work when read on a new +# one. A smaller old_pg/new_pg matrix (not the +# full PG matrix - by far the most expensive +# job here, installing two full PostgreSQL +# majors and running the real pg_upgrade +# binary per leg). # pg-tle-test -- pg_tle DEPLOYMENT: fresh install registered # through AWS pg_tle's database-backed catalog # instead of a filesystem .control file. @@ -266,6 +276,102 @@ jobs: - name: Update 0.9.6 -> current and run the suite run: make verify-results TEST_LOAD_SOURCE=update + # Proves count_nulls survives a BINARY pg_upgrade (in-place catalog + # migration to a newer PostgreSQL major), not just an in-place extension + # update. Installs 0.9.6 on an old cluster, plants a dependency guard, + # binary-pg_upgrades to a newer cluster, updates the extension to current, + # then runs the suite against the REAL migrated objects in existing mode. + # No bridge-update step first: count_nulls has always been pure SQL + # functions with no SELECT-*-over-catalog views, so it has no known + # pg_upgrade-unsafe old version to bridge past. + # + # Deliberately not doing a stepwise every-major-in-sequence climb (one + # cluster walking 10->11->12->...->newest, vs. the single big jumps here): + # that would catch a regression specific to one particular major-to-major + # boundary, which would matter if count_nulls had views/functions touching + # catalog internals, but it doesn't - pure SQL functions over anyarray/ + # json/jsonb, nothing version-sensitive to break at a specific boundary. + # Revisit if count_nulls ever grows something catalog-touching. + # + # Not yet crossed with TEST_SCHEMA (a later phase adds that, once it can + # do so for both this job and the test job's update leg together). + pg-upgrade-test: + needs: [changes] + if: needs.changes.outputs.docs_only != 'true' + strategy: + matrix: + old_pg: ["10", "12"] + new_pg: ["18"] + name: ๐Ÿ”„ Binary pg_upgrade ${{ matrix.old_pg }} โ†’ ${{ matrix.new_pg }} + runs-on: ubuntu-latest + container: pgxn/pgxn-tools + env: + # Both clusters must use the same initdb options or pg_upgrade + # refuses to run. + INITDB_OPTS: --data-checksums --auth trust + steps: + - name: Start PostgreSQL ${{ matrix.old_pg }} + run: pg-start ${{ matrix.old_pg }} + - name: Recreate old cluster with data checksums enabled + run: | + pg_ctlcluster ${{ matrix.old_pg }} test stop + pg_dropcluster ${{ matrix.old_pg }} test + # -p 5432: pg_createcluster assigns the next available port, which + # may not be 5432 after pg-start has claimed and released it. + # Force 5432 so subsequent psql/createdb calls connect without -p. + pg_createcluster -p 5432 ${{ matrix.old_pg }} test -- $INITDB_OPTS + pg_ctlcluster ${{ matrix.old_pg }} test start + pg_isready -t 30 + - name: Check out the repo + uses: actions/checkout@v4 + - name: Install count_nulls into old cluster + run: make install + - name: Prepare the old cluster (install + dependency guard) + # prepare-old installs count_nulls at 0.9.6, then plants + proves + # the dependency guard, so a later accidental CASCADE drop anywhere + # in this job cannot silently make the eventual existing-mode run + # test a fresh install instead. + run: bin/test_existing prepare-old count_nulls_upgrade "" 0.9.6 + - name: Install PostgreSQL ${{ matrix.new_pg }} + run: apt-get install -y postgresql-${{ matrix.new_pg }} postgresql-server-dev-${{ matrix.new_pg }} + - name: Install count_nulls into new cluster + # PG_CONFIG must be specified explicitly: at this point both old + # and new PostgreSQL are installed, and the default pg_config on + # PATH may not be the new version's. + run: make install PG_CONFIG=/usr/lib/postgresql/${{ matrix.new_pg }}/bin/pg_config + - name: Stop old cluster, binary pg_upgrade to PostgreSQL ${{ matrix.new_pg }}, start new cluster + run: | + pg_ctlcluster ${{ matrix.old_pg }} test stop + pg_createcluster -p 5432 ${{ matrix.new_pg }} test -- $INITDB_OPTS + # PG17+ writes logs to $new_datadir/pg_upgrade_output.d/; older + # versions write to CWD. Search both on failure. + mkdir -p /tmp/pg_upgrade_logs + chown postgres:postgres /tmp/pg_upgrade_logs + su -c "cd /tmp/pg_upgrade_logs && /usr/lib/postgresql/${{ matrix.new_pg }}/bin/pg_upgrade \ + -b /usr/lib/postgresql/${{ matrix.old_pg }}/bin \ + -B /usr/lib/postgresql/${{ matrix.new_pg }}/bin \ + -d /var/lib/postgresql/${{ matrix.old_pg }}/test \ + -D /var/lib/postgresql/${{ matrix.new_pg }}/test \ + -o '-c config_file=/etc/postgresql/${{ matrix.old_pg }}/test/postgresql.conf' \ + -O '-c config_file=/etc/postgresql/${{ matrix.new_pg }}/test/postgresql.conf'" postgres \ + || { find /tmp/pg_upgrade_logs \ + /var/lib/postgresql/${{ matrix.new_pg }}/test/pg_upgrade_output.d \ + -name '*.log' 2>/dev/null | sort | xargs -r tail -n +1; exit 1; } + pg_ctlcluster ${{ matrix.new_pg }} test start + - name: Update the pg_upgraded extension to the current version + # Exercises ALTER EXTENSION UPDATE on genuinely pg_upgraded objects + # (the extension binary pg_upgrade just migrated), running the + # 0.9.6->stable update script. + run: bin/test_existing update count_nulls_upgrade + - name: Run the suite against the pg_upgraded database (existing mode) + # run-suite asserts the version, re-proves the dependency guard + # still blocks a non-CASCADE drop (i.e. it survived pg_upgrade), + # drops the guard, then runs the suite against the REAL pg_upgraded + # + updated database via --use-existing (so pg_regress does not + # drop/recreate it) - a plain fresh `make test` would silently test + # a fresh install instead of the migrated objects. + run: bin/test_existing run-suite count_nulls_upgrade "" + pg-tle-test: needs: [changes] # Skipped outright (not just matrix-reduced like `test` above) on a @@ -399,7 +505,7 @@ jobs: # `changes` job on a docs-only push), and fails if any failed or were # cancelled. all-checks-passed: - needs: [changes, lint, test, pg-tle-test] + needs: [changes, lint, test, pg-upgrade-test, pg-tle-test] if: always() runs-on: ubuntu-latest steps: diff --git a/bin/test_existing b/bin/test_existing new file mode 100755 index 0000000..d0c2cb6 --- /dev/null +++ b/bin/test_existing @@ -0,0 +1,210 @@ +#!/usr/bin/env bash +# +# Exercise the count_nulls test suite against a REAL database whose extension +# was installed/upgraded OUTSIDE this pg_regress invocation ("existing" mode) +# - specifically, a real binary pg_upgrade run. Unlike an in-place ALTER +# EXTENSION UPDATE (which test/install/load.sql's own 'update' mode handles +# entirely by itself - see `make test-update`), a real pg_upgrade is an +# external binary process pg_regress can't invoke itself, so THAT leg needs +# an external script to drive it. This is a smaller script than it might look +# like it needs to be: only the pieces that genuinely can't live inside a +# single pg_regress invocation are here. +# +# The pg-upgrade-test CI job repeats the same sequence: +# +# prepare-old (install + plant guard) -> [real pg_upgrade binary, in CI] -> +# update (ALTER EXTENSION UPDATE) -> run-suite (assert + run existing-mode) +# +# so it lives here once instead of being duplicated as inline YAML. Not +# CI-only: a developer can run any subcommand locally against a scratch +# database. Modeled on Postgres-Extensions/cat_tools's bin/test_existing. +# Two differences from that script: count_nulls ships no +# SELECT-*-over-catalog views, so it has no known pg_upgrade-unsafe old +# version to bridge past before running pg_upgrade; and its own suite has a +# legitimate (though harmless - always rolled back) DROP EXTENSION test, so +# here the guard is dropped before run-suite instead of surviving through it. +# +# USAGE: bin/test_existing [args] +# +# prepare-old DB SCHEMA INSTALL_VERSION +# Old-cluster prep for pg-upgrade-test: create DB + extension at +# INSTALL_VERSION in SCHEMA, then plant + prove the dependency guard. +# +# update DB [TO_VERSION] +# ALTER EXTENSION count_nulls UPDATE [TO 'TO_VERSION'] (empty => current). +# +# run-suite DB SCHEMA +# Assert the current version, re-prove the guard, drop it, then run the +# suite in existing mode (extension must be at the current version). +# +# Run `bin/test_existing` with no subcommand to print usage. +# +# Why the dependency guard: "existing" mode must run the suite against the +# ACTUAL upgraded objects. If anything silently dropped + reinstalled the +# extension (a stray CASCADE, a logic bug, a bad CI step), the suite would +# test a FRESH install and hide a regression. We plant an object that HARD- +# references a count_nulls member so a non-CASCADE DROP EXTENSION fails, and +# actively PROVE that (see bin/test_existing.sql/assert_guard.sql): if the +# drop unexpectedly succeeds, this script fails CI rather than silently +# passing. +set -euo pipefail + +# Run from the repository root (where `make` works and test paths resolve), +# regardless of the caller's cwd. bin/ sits directly under the repo root, so +# its parent is the root. readlink -f resolves any path the script was +# invoked through. +cd "$(dirname "$(readlink -f "$0")")/.." + +# --------------------------------------------------------------------------- +# psql helpers +# --------------------------------------------------------------------------- + +psql_value() { + local db=$1 sql=$2 + psql -d "$db" -tAc "$sql" +} + +psql_do() { + local db=$1 + shift + psql -d "$db" -v ON_ERROR_STOP=1 "$@" +} + +# --------------------------------------------------------------------------- +# Version / guard helpers +# --------------------------------------------------------------------------- + +current_version() { + # EXTENSION_count_nulls_VERSION (the .control file's default_version), NOT + # PGXNVERSION (the PGXN distribution version, from META.in.json) - a + # version-less CREATE EXTENSION/ALTER EXTENSION UPDATE installs whatever + # the control file's default_version says, and count_nulls' is currently + # the 'stable' pseudo-version, not the last real release. Using PGXNVERSION + # here would compare an installed 'stable' against an expected real version + # number and always report a mismatch. See RELEASE.md's note on + # distribution vs. extension versions; the pg-tle-test CI job makes the + # same distinction for the same reason. + make -s print-EXTENSION_count_nulls_VERSION 2>/dev/null | sed -n 's/.*set to "\(.*\)"$/\1/p' +} + +installed_version() { + psql_value "$1" \ + "SELECT extversion FROM pg_extension WHERE extname = 'count_nulls'" +} + +# Plant the guard and PROVE it blocks a non-CASCADE drop. Call right after +# CREATE EXTENSION (and before any update/upgrade) so it persists through them. +plant_guard() { + local db=$1 schema=$2 + psql -d "$db" -v ON_ERROR_STOP=1 -v schema="$schema" -f bin/test_existing.sql/plant_guard.sql + assert_drop_blocked "$db" +} + +# The core safeguard self-check: a non-CASCADE DROP EXTENSION MUST fail while +# the guard exists. assert_guard.sql fails loudly (nonzero exit) if the drop +# unexpectedly succeeds, or if the extension/guard are missing afterward. +assert_drop_blocked() { + local db=$1 + psql -d "$db" -v ON_ERROR_STOP=1 -f bin/test_existing.sql/assert_guard.sql + echo "OK: non-CASCADE DROP EXTENSION is blocked in '$db' (dependency guard effective)" +} + +drop_guard() { + local db=$1 + psql -d "$db" -v ON_ERROR_STOP=1 -f bin/test_existing.sql/drop_guard.sql +} + +assert_version() { + local db=$1 expected=$2 installed + [ "$expected" = current ] && expected=$(current_version) + installed=$(installed_version "$db") + echo "version check '$db': installed='$installed' expected='$expected'" + if [ -z "$installed" ] || [ -z "$expected" ] || [ "$installed" != "$expected" ]; then + echo "FAIL: count_nulls in '$db' is '$installed', expected '$expected'" >&2 + exit 1 + fi +} + +update_ext() { + local db=$1 to=${2:-} + # Prepend "TO " only when a target version is given, so a single statement + # covers both cases (empty $to => bare "ALTER EXTENSION ... UPDATE" to + # current). Use `if`, not `&&`: a false test under `set -e` would abort. + if [ -n "$to" ]; then to="TO '$to'"; fi + psql_do "$db" -c "ALTER EXTENSION count_nulls UPDATE $to" +} + +# CREATE EXTENSION count_nulls at VERSION, targeting SCHEMA - unless SCHEMA +# is empty, in which case it's created untouched, wherever the session's +# own default search_path resolves (ordinarily 'public'). A quoted empty +# identifier ("") is a real Postgres syntax error, so this can't just always +# emit `CREATE SCHEMA IF NOT EXISTS "$schema"` - the empty case has to skip +# that entirely, mirroring test/install/load.sql's own :count_nulls_has_schema +# branch. +create_extension_in_schema() { + local db=$1 schema=$2 version=$3 sql="" + if [ -n "$schema" ]; then + sql="CREATE SCHEMA IF NOT EXISTS \"$schema\"; SET search_path = \"$schema\"; " + fi + psql_do "$db" -c "${sql}CREATE EXTENSION count_nulls VERSION '$version'" +} + +# --------------------------------------------------------------------------- +# Subcommand implementations +# --------------------------------------------------------------------------- + +# prepare-old DB SCHEMA INSTALL_VERSION +# Old-cluster preparation for pg-upgrade-test: create the database and the +# extension at INSTALL_VERSION in SCHEMA, then plant + prove the guard. No +# bridge-update step first: count_nulls ships no SELECT-*-over-catalog +# views, so it has no known pg_upgrade-unsafe old version to bridge past. +prepare_old() { + local db=$1 schema=$2 install=$3 + createdb "$db" + create_extension_in_schema "$db" "$schema" "$install" + plant_guard "$db" "$schema" +} + +# Run the pgTAP suite against an already-populated database in existing mode. +# Verifies count_nulls is at the current version, re-proves the guard still +# blocks a drop (i.e. it survived the update/upgrade), drops the guard (see +# the file header for why - count_nulls's own suite legitimately drops the +# extension, harmlessly, inside a transaction that's always rolled back), +# then runs the suite via --use-existing so pg_regress does NOT drop/recreate +# the database. +run_suite() { + local db=$1 schema=$2 + assert_version "$db" current + assert_drop_blocked "$db" + drop_guard "$db" + # In existing mode pg_regress runs against $db via --use-existing and must + # NOT create/drop its own database. `make test` (not just `make + # verify-results`) is a real gate as of pgxntool 2.3.0 - it now exits + # non-zero on regression failures instead of always exiting 0 regardless + # of pg_regress's result (see this repo's pgxntool 2.3.0 bump). + make test TEST_LOAD_SOURCE=existing TEST_SCHEMA="$schema" CONTRIB_TESTDB="$db" EXTRA_REGRESS_OPTS=--use-existing +} + +usage() { + echo "usage: bin/test_existing [args]" >&2 + echo " prepare-old DB SCHEMA INSTALL_VERSION" >&2 + echo " update DB [TO_VERSION]" >&2 + echo " run-suite DB SCHEMA" >&2 + exit 2 +} + +# Explicit subcommand dispatch on $1. Defined first for readability; INVOKED +# at the very bottom, after every helper it calls is defined (bash resolves +# calls at runtime, so main() appearing first is fine). +main() { + local cmd=${1:-} + shift || true + case "$cmd" in + prepare-old) prepare_old "$@" ;; + update) update_ext "$@" ;; + run-suite) run_suite "$@" ;; + *) usage ;; + esac +} + +main "$@" diff --git a/bin/test_existing.sql/assert_guard.sql b/bin/test_existing.sql/assert_guard.sql new file mode 100644 index 0000000..33946cf --- /dev/null +++ b/bin/test_existing.sql/assert_guard.sql @@ -0,0 +1,35 @@ +/* + * Proves the guard planted by plant_guard.sql actually blocks a + * non-CASCADE DROP EXTENSION - prove it, don't assume it. Re-run after + * every step (install, pg_upgrade, post-upgrade ALTER EXTENSION UPDATE): + * the guard disappearing at any point means a CASCADE drop happened + * somewhere upstream, i.e. the "existing" run downstream would actually be + * a silent fresh install. + * + * Usage: psql -v ON_ERROR_STOP=1 -f assert_guard.sql + */ +\set ON_ERROR_STOP on + +DO $$ +BEGIN + DROP EXTENSION count_nulls; + -- Only reached if the drop above unexpectedly succeeded. + RAISE EXCEPTION 'GUARD FAILURE: non-CASCADE DROP EXTENSION count_nulls unexpectedly succeeded'; +EXCEPTION WHEN dependent_objects_still_exist THEN + RAISE NOTICE 'guard held: DROP EXTENSION count_nulls correctly blocked'; +END +$$; + +DO $$ +BEGIN + IF NOT EXISTS (SELECT 1 FROM pg_extension WHERE extname = 'count_nulls') THEN + RAISE EXCEPTION 'GUARD FAILURE: count_nulls extension missing after guard check'; + END IF; + IF NOT EXISTS ( + SELECT 1 FROM pg_class c JOIN pg_namespace n ON n.oid = c.relnamespace + WHERE c.relname = 'guard' AND n.nspname = 'count_nulls_drop_guard' + ) THEN + RAISE EXCEPTION 'GUARD FAILURE: count_nulls_drop_guard.guard view missing'; + END IF; +END +$$; diff --git a/bin/test_existing.sql/drop_guard.sql b/bin/test_existing.sql/drop_guard.sql new file mode 100644 index 0000000..744c3b5 --- /dev/null +++ b/bin/test_existing.sql/drop_guard.sql @@ -0,0 +1,11 @@ +/* + * Removes the guard (plant_guard.sql) once its job is done - proving the + * install/pg_upgrade/update steps didn't corrupt the real extension - so it + * doesn't then block the pgTap suite's own DROP EXTENSION test + * (test__shutdown__drop_all, run in a transaction that's rolled back + * regardless, so re-dropping the real extension there is harmless). + * + * Usage: psql -v ON_ERROR_STOP=1 -f drop_guard.sql + */ +\set ON_ERROR_STOP on +DROP SCHEMA count_nulls_drop_guard CASCADE; diff --git a/bin/test_existing.sql/plant_guard.sql b/bin/test_existing.sql/plant_guard.sql new file mode 100644 index 0000000..1c3e84d --- /dev/null +++ b/bin/test_existing.sql/plant_guard.sql @@ -0,0 +1,30 @@ +/* + * Dependency guard: plants an object with a hard pg_depend dependency on a + * stable, never-dropped/redefined extension member (null_count(anyarray), + * unchanged since 0.9.0), so that a non-CASCADE DROP EXTENSION count_nulls + * is blocked. Used by the pg_upgrade CI job to prove a real pg_upgrade/ + * update run didn't silently destroy the extension it's meant to be + * testing (a stray CASCADE drop, a logic bug, a bad CI step would + * otherwise fall through to a silent fresh reinstall and the job would + * still report green). + * + * Usage: psql -v ON_ERROR_STOP=1 -v schema= -f plant_guard.sql + * (empty schema means "wherever null_count already resolves unqualified" - + * i.e. count_nulls was installed without targeting a schema). + */ +\set ON_ERROR_STOP on + +/* + * schema_prefix: either empty, or the quoted schema name followed by a + * literal '.' - so the view definition below is a single statement with a + * plain (unquoted) substitution, rather than branching the whole CREATE + * VIEW on whether a schema was given. quote_ident(), not :"schema" - + * :schema_prefix is pasted as-is (unquoted substitution), so it must + * already be valid, properly-quoted SQL text by the time it lands there. + */ +SELECT CASE WHEN :'schema' <> '' THEN quote_ident(:'schema') || '.' ELSE '' END AS schema_prefix +\gset + +CREATE SCHEMA IF NOT EXISTS count_nulls_drop_guard; +CREATE OR REPLACE VIEW count_nulls_drop_guard.guard AS + SELECT :schema_prefix null_count(NULL::int, NULL::int) AS guarded_member; From 78de57c9a12c877f133c85ef45770ddceb083f92 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Wed, 5 Aug 2026 16:41:23 -0500 Subject: [PATCH 2/9] pg-upgrade-test: run ALTER EXTENSION UPDATE before binary pg_upgrade, not after Reorders prepare-old -> update -> pg_upgrade -> run-suite (was prepare-old -> pg_upgrade -> update -> run-suite). The old order proved pg_upgrade could migrate 0.9.6's frozen objects, then updated afterward - not actionable, since that version already shipped. This job's whole point is proving pg_upgrade correctly migrates the objects count_nulls' CURRENT code creates, which requires updating BEFORE the binary upgrade runs. make install (into the old cluster) already happens earlier in the job, so the current version's update scripts are on disk in time for the moved step. Updates the job's step names/comments and bin/test_existing's own file-header sequence description to match the new order. --- .github/workflows/ci.yml | 65 ++++++++++++++++++++++++---------------- bin/test_existing | 14 ++++++--- 2 files changed, 50 insertions(+), 29 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e0dbf18..cdd43be 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -17,15 +17,18 @@ # per-PG-version container/checkout setup for # no added confidence. # pg-upgrade-test -- BINARY pg_upgrade: install 0.9.6 on an OLD -# PostgreSQL major, binary-upgrade the cluster -# to a NEWER major, then update the extension -# to current - proves objects created on an -# old server still work when read on a new -# one. A smaller old_pg/new_pg matrix (not the -# full PG matrix - by far the most expensive -# job here, installing two full PostgreSQL -# majors and running the real pg_upgrade -# binary per leg). +# PostgreSQL major, update the extension to +# current (still on the old major), THEN +# binary-upgrade the cluster to a NEWER major - +# proves pg_upgrade correctly migrates the +# objects the extension actually creates +# TODAY, not objects frozen at some past +# version (which would be untestable anyway - +# that old version already shipped). A smaller +# old_pg/new_pg matrix (not the full PG matrix +# - by far the most expensive job here, +# installing two full PostgreSQL majors and +# running the real pg_upgrade binary per leg). # pg-tle-test -- pg_tle DEPLOYMENT: fresh install registered # through AWS pg_tle's database-backed catalog # instead of a filesystem .control file. @@ -277,13 +280,19 @@ jobs: run: make verify-results TEST_LOAD_SOURCE=update # Proves count_nulls survives a BINARY pg_upgrade (in-place catalog - # migration to a newer PostgreSQL major), not just an in-place extension - # update. Installs 0.9.6 on an old cluster, plants a dependency guard, - # binary-pg_upgrades to a newer cluster, updates the extension to current, + # migration to a newer PostgreSQL major). Installs 0.9.6 on an old + # cluster, plants a dependency guard, updates the extension to CURRENT + # (still on the old major), THEN binary-pg_upgrades to a newer cluster, # then runs the suite against the REAL migrated objects in existing mode. - # No bridge-update step first: count_nulls has always been pure SQL - # functions with no SELECT-*-over-catalog views, so it has no known - # pg_upgrade-unsafe old version to bridge past. + # Updating before the binary upgrade (not after) is deliberate: the whole + # point of this job is proving pg_upgrade correctly migrates the objects + # count_nulls' CURRENT code actually creates - migrating 0.9.6's objects + # and updating afterward would instead test whether pg_upgrade can + # migrate a legacy structure frozen in the past, which isn't actionable + # (that version already shipped; nothing to fix if it turned out + # fragile). No bridge-update step first: count_nulls has always been + # pure SQL functions with no SELECT-*-over-catalog views, so it has no + # known pg_upgrade-unsafe old version to bridge past. # # Deliberately not doing a stepwise every-major-in-sequence climb (one # cluster walking 10->11->12->...->newest, vs. the single big jumps here): @@ -332,6 +341,17 @@ jobs: # in this job cannot silently make the eventual existing-mode run # test a fresh install instead. run: bin/test_existing prepare-old count_nulls_upgrade "" 0.9.6 + - name: Update the extension to the current version (still on the old cluster) + # Exercises ALTER EXTENSION UPDATE on the OLD cluster, BEFORE the + # binary pg_upgrade below, running the 0.9.6->stable update script - + # deliberately in this order (not update-after-upgrade): this job + # exists to prove pg_upgrade correctly migrates the objects + # count_nulls' CURRENT code creates, so pg_upgrade must run against + # already-current objects, not 0.9.6 ones. `make install` above + # already installed the current version's update scripts/control + # file into this (old) cluster's sharedir, so they're in place for + # this ALTER EXTENSION UPDATE to use. + run: bin/test_existing update count_nulls_upgrade - name: Install PostgreSQL ${{ matrix.new_pg }} run: apt-get install -y postgresql-${{ matrix.new_pg }} postgresql-server-dev-${{ matrix.new_pg }} - name: Install count_nulls into new cluster @@ -358,18 +378,13 @@ jobs: /var/lib/postgresql/${{ matrix.new_pg }}/test/pg_upgrade_output.d \ -name '*.log' 2>/dev/null | sort | xargs -r tail -n +1; exit 1; } pg_ctlcluster ${{ matrix.new_pg }} test start - - name: Update the pg_upgraded extension to the current version - # Exercises ALTER EXTENSION UPDATE on genuinely pg_upgraded objects - # (the extension binary pg_upgrade just migrated), running the - # 0.9.6->stable update script. - run: bin/test_existing update count_nulls_upgrade - name: Run the suite against the pg_upgraded database (existing mode) # run-suite asserts the version, re-proves the dependency guard - # still blocks a non-CASCADE drop (i.e. it survived pg_upgrade), - # drops the guard, then runs the suite against the REAL pg_upgraded - # + updated database via --use-existing (so pg_regress does not - # drop/recreate it) - a plain fresh `make test` would silently test - # a fresh install instead of the migrated objects. + # still blocks a non-CASCADE drop (i.e. it survived both the update + # and pg_upgrade), drops the guard, then runs the suite against the + # REAL pg_upgraded database via --use-existing (so pg_regress does + # not drop/recreate it) - a plain fresh `make test` would silently + # test a fresh install instead of the migrated objects. run: bin/test_existing run-suite count_nulls_upgrade "" pg-tle-test: diff --git a/bin/test_existing b/bin/test_existing index d0c2cb6..f0b12f5 100755 --- a/bin/test_existing +++ b/bin/test_existing @@ -12,11 +12,17 @@ # # The pg-upgrade-test CI job repeats the same sequence: # -# prepare-old (install + plant guard) -> [real pg_upgrade binary, in CI] -> -# update (ALTER EXTENSION UPDATE) -> run-suite (assert + run existing-mode) +# prepare-old (install + plant guard) -> update (ALTER EXTENSION UPDATE, +# on the OLD cluster, before pg_upgrade) -> [real pg_upgrade binary, in +# CI] -> run-suite (assert + run existing-mode) # -# so it lives here once instead of being duplicated as inline YAML. Not -# CI-only: a developer can run any subcommand locally against a scratch +# so it lives here once instead of being duplicated as inline YAML. update +# runs BEFORE the binary pg_upgrade, not after: the point of this job is +# proving pg_upgrade correctly migrates the objects count_nulls' CURRENT +# code creates, so pg_upgrade needs to run against already-current objects, +# not ones still frozen at the old INSTALL_VERSION. +# +# Not CI-only: a developer can run any subcommand locally against a scratch # database. Modeled on Postgres-Extensions/cat_tools's bin/test_existing. # Two differences from that script: count_nulls ships no # SELECT-*-over-catalog views, so it has no known pg_upgrade-unsafe old From a4275bc551cef71f1a42f94999d71e5833ff9a01 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Thu, 6 Aug 2026 17:09:54 -0500 Subject: [PATCH 3/9] CI: skip pg-upgrade-test on draft PRs too Propagates the draft-PR gating from phase3.5-ci-hygiene to the pg-upgrade-test job introduced by this branch: same needs:[changes]/ if: docs_only pattern as pg-tle-test, so it gets the same && github.event.pull_request.draft != true guard. --- .github/workflows/ci.yml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index cdd43be..4cffb64 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -306,7 +306,10 @@ jobs: # do so for both this job and the test job's update leg together). pg-upgrade-test: needs: [changes] - if: needs.changes.outputs.docs_only != 'true' + # Skipped outright (not just matrix-reduced like `test`) on a draft PR: + # this is a heavy job, and a draft author doesn't need a real binary + # pg_upgrade re-proven on every push while still iterating. + if: needs.changes.outputs.docs_only != 'true' && github.event.pull_request.draft != true strategy: matrix: old_pg: ["10", "12"] From a08aa83ca311fd74b1300c6ee10cf7354c548576 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Thu, 6 Aug 2026 17:57:11 -0500 Subject: [PATCH 4/9] ci: cover both pg_upgrade orderings in pg-upgrade-test's 2 legs old_pg=12 keeps updating to current before the binary pg_upgrade (proves pg_upgrade preserves CURRENT-code objects); old_pg=10 now updates after instead (proves pg_upgrade preserves objects still frozen at the oldest supported version at upgrade time - a real user scenario the update-before-only setup missed). Same 2-leg matrix, no added CI cost. Co-Authored-By: Claude Sonnet 5 --- .github/workflows/ci.yml | 110 +++++++++++++++++++++++++++------------ 1 file changed, 76 insertions(+), 34 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4cffb64..14bf3d5 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -17,18 +17,23 @@ # per-PG-version container/checkout setup for # no added confidence. # pg-upgrade-test -- BINARY pg_upgrade: install 0.9.6 on an OLD -# PostgreSQL major, update the extension to -# current (still on the old major), THEN -# binary-upgrade the cluster to a NEWER major - -# proves pg_upgrade correctly migrates the -# objects the extension actually creates -# TODAY, not objects frozen at some past -# version (which would be untestable anyway - -# that old version already shipped). A smaller -# old_pg/new_pg matrix (not the full PG matrix -# - by far the most expensive job here, -# installing two full PostgreSQL majors and -# running the real pg_upgrade binary per leg). +# PostgreSQL major, then binary-upgrade the +# cluster to a NEWER major. The two legs of +# this job's small old_pg/new_pg matrix +# deliberately update the extension to current +# on OPPOSITE SIDES of that binary upgrade: one +# leg proves pg_upgrade correctly migrates the +# objects the extension's CURRENT code creates +# TODAY, the other proves it correctly +# preserves objects still frozen at the OLDEST +# supported version at the moment of the +# upgrade (a real user scenario) - see the +# job's own comment for which leg does which +# and why. A smaller old_pg/new_pg matrix (not +# the full PG matrix - by far the most +# expensive job here, installing two full +# PostgreSQL majors and running the real +# pg_upgrade binary per leg). # pg-tle-test -- pg_tle DEPLOYMENT: fresh install registered # through AWS pg_tle's database-backed catalog # instead of a filesystem .control file. @@ -281,18 +286,39 @@ jobs: # Proves count_nulls survives a BINARY pg_upgrade (in-place catalog # migration to a newer PostgreSQL major). Installs 0.9.6 on an old - # cluster, plants a dependency guard, updates the extension to CURRENT - # (still on the old major), THEN binary-pg_upgrades to a newer cluster, - # then runs the suite against the REAL migrated objects in existing mode. - # Updating before the binary upgrade (not after) is deliberate: the whole - # point of this job is proving pg_upgrade correctly migrates the objects - # count_nulls' CURRENT code actually creates - migrating 0.9.6's objects - # and updating afterward would instead test whether pg_upgrade can - # migrate a legacy structure frozen in the past, which isn't actionable - # (that version already shipped; nothing to fix if it turned out - # fragile). No bridge-update step first: count_nulls has always been - # pure SQL functions with no SELECT-*-over-catalog views, so it has no - # known pg_upgrade-unsafe old version to bridge past. + # cluster, plants a dependency guard, binary-pg_upgrades to a newer + # cluster, then runs the suite against the REAL migrated objects in + # existing mode. + # + # The two legs of this job's small old_pg/new_pg matrix deliberately + # update the extension to CURRENT on OPPOSITE SIDES of the binary + # pg_upgrade (see the `if: matrix.old_pg == ...` guards on the two + # "Update to current..." steps below) - same total CI cost, but between + # them the two legs now cover both orderings a real user could hit: + # + # old_pg=12 -- update BEFORE pg_upgrade (on the OLD cluster): proves + # pg_upgrade correctly migrates the objects count_nulls' + # CURRENT code actually creates - migrating 0.9.6's + # objects and updating afterward would instead test + # whether pg_upgrade can migrate a legacy structure frozen + # in the past, which isn't actionable (that version + # already shipped; nothing to fix if it turned out + # fragile). + # old_pg=10 -- update AFTER pg_upgrade (on the NEW cluster): proves + # pg_upgrade itself correctly preserves objects that are + # STILL at the oldest supported version at the moment of + # the binary upgrade - a real-world scenario (a user + # upgrading their PostgreSQL major while still running an + # old extension version) that the old_pg=12 leg alone + # would miss. + # + # Both legs still end up with a current-version database by the time + # run-suite runs, so nothing downstream (the pgTAP suite, the dependency + # guard check) needs to differ - only WHICH SIDE of the pg_upgrade call + # the update happens on. No bridge-update step first, in either leg: + # count_nulls has always been pure SQL functions with no + # SELECT-*-over-catalog views, so it has no known pg_upgrade-unsafe old + # version to bridge past. # # Deliberately not doing a stepwise every-major-in-sequence climb (one # cluster walking 10->11->12->...->newest, vs. the single big jumps here): @@ -344,16 +370,16 @@ jobs: # in this job cannot silently make the eventual existing-mode run # test a fresh install instead. run: bin/test_existing prepare-old count_nulls_upgrade "" 0.9.6 - - name: Update the extension to the current version (still on the old cluster) - # Exercises ALTER EXTENSION UPDATE on the OLD cluster, BEFORE the - # binary pg_upgrade below, running the 0.9.6->stable update script - - # deliberately in this order (not update-after-upgrade): this job - # exists to prove pg_upgrade correctly migrates the objects - # count_nulls' CURRENT code creates, so pg_upgrade must run against - # already-current objects, not 0.9.6 ones. `make install` above - # already installed the current version's update scripts/control - # file into this (old) cluster's sharedir, so they're in place for - # this ALTER EXTENSION UPDATE to use. + - name: Update to current before pg_upgrade (proves pg_upgrade preserves CURRENT objects) + # old_pg=12 leg only - see the job's own header comment above for + # why the two legs deliberately differ here. Exercises ALTER + # EXTENSION UPDATE on the OLD cluster, BEFORE the binary pg_upgrade + # below, running the 0.9.6->stable update script, so pg_upgrade + # runs against already-current objects, not 0.9.6 ones. `make + # install` above already installed the current version's update + # scripts/control file into this (old) cluster's sharedir, so + # they're in place for this ALTER EXTENSION UPDATE to use. + if: matrix.old_pg == '12' run: bin/test_existing update count_nulls_upgrade - name: Install PostgreSQL ${{ matrix.new_pg }} run: apt-get install -y postgresql-${{ matrix.new_pg }} postgresql-server-dev-${{ matrix.new_pg }} @@ -381,6 +407,22 @@ jobs: /var/lib/postgresql/${{ matrix.new_pg }}/test/pg_upgrade_output.d \ -name '*.log' 2>/dev/null | sort | xargs -r tail -n +1; exit 1; } pg_ctlcluster ${{ matrix.new_pg }} test start + - name: Update to current after pg_upgrade (proves pg_upgrade preserves OLDEST-version objects) + # old_pg=10 leg only - see the job's own header comment above for + # why the two legs deliberately differ here. Exercises ALTER + # EXTENSION UPDATE AFTER the binary pg_upgrade above, against the + # NEW cluster (already started by the previous step, and the only + # cluster listening on 5432 at this point, since the old one was + # stopped in that same step) - the 0.9.6 objects prepare-old + # planted are thus still at 0.9.6 at the moment pg_upgrade itself + # runs, so this leg proves pg_upgrade correctly preserves objects + # still frozen at the oldest supported version, not just + # already-current ones. `make install` into the new cluster above + # already installed the current version's update scripts/control + # file into ITS sharedir, so they're in place for this ALTER + # EXTENSION UPDATE to use. + if: matrix.old_pg == '10' + run: bin/test_existing update count_nulls_upgrade - name: Run the suite against the pg_upgraded database (existing mode) # run-suite asserts the version, re-proves the dependency guard # still blocks a non-CASCADE drop (i.e. it survived both the update From 070672ecd661f98a5411423290e04ac5094ef2bd Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Fri, 7 Aug 2026 13:25:55 -0500 Subject: [PATCH 5/9] pg-upgrade-test: confine the ordering comparison to old_pg=10 via twin databases The old_pg=10-vs-12 split wrongly conflated two independent axes (which PG floor is tested, and which update-order is tested). Both orderings are now compared on the SAME floor (old_pg=10, the oldest supported) using twin databases migrated by a single pg_upgrade call - the same trick this file already uses to cross TEST_SCHEMA without doubling matrix legs. old_pg=12 reverts to its original single-database, single-ordering design and is now just a second, unrelated old-PG-floor data point. --- .github/workflows/ci.yml | 186 +++++++++++++++++++++++++-------------- 1 file changed, 121 insertions(+), 65 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 14bf3d5..15a72c0 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -18,19 +18,17 @@ # no added confidence. # pg-upgrade-test -- BINARY pg_upgrade: install 0.9.6 on an OLD # PostgreSQL major, then binary-upgrade the -# cluster to a NEWER major. The two legs of -# this job's small old_pg/new_pg matrix -# deliberately update the extension to current -# on OPPOSITE SIDES of that binary upgrade: one -# leg proves pg_upgrade correctly migrates the -# objects the extension's CURRENT code creates -# TODAY, the other proves it correctly -# preserves objects still frozen at the OLDEST -# supported version at the moment of the -# upgrade (a real user scenario) - see the -# job's own comment for which leg does which -# and why. A smaller old_pg/new_pg matrix (not -# the full PG matrix - by far the most +# cluster to a NEWER major. Its old_pg=10 leg +# additionally compares BOTH update-vs- +# pg_upgrade orderings a real user could hit, +# via twin databases migrated by the SAME +# binary pg_upgrade call; old_pg=12 is a +# second, unrelated old-PG-floor data point +# using the simpler single-database, single- +# ordering design - see the job's own comment +# for why the ordering comparison only needs +# one floor. A smaller old_pg/new_pg matrix +# (not the full PG matrix - by far the most # expensive job here, installing two full # PostgreSQL majors and running the real # pg_upgrade binary per leg). @@ -290,35 +288,53 @@ jobs: # cluster, then runs the suite against the REAL migrated objects in # existing mode. # - # The two legs of this job's small old_pg/new_pg matrix deliberately - # update the extension to CURRENT on OPPOSITE SIDES of the binary - # pg_upgrade (see the `if: matrix.old_pg == ...` guards on the two - # "Update to current..." steps below) - same total CI cost, but between - # them the two legs now cover both orderings a real user could hit: + # This job's small old_pg/new_pg matrix tests two INDEPENDENT axes, kept + # deliberately separate rather than crossed into their own matrix + # dimension (which would double this already-expensive job's cost for no + # added confidence): # - # old_pg=12 -- update BEFORE pg_upgrade (on the OLD cluster): proves - # pg_upgrade correctly migrates the objects count_nulls' - # CURRENT code actually creates - migrating 0.9.6's - # objects and updating afterward would instead test - # whether pg_upgrade can migrate a legacy structure frozen - # in the past, which isn't actionable (that version - # already shipped; nothing to fix if it turned out - # fragile). - # old_pg=10 -- update AFTER pg_upgrade (on the NEW cluster): proves - # pg_upgrade itself correctly preserves objects that are - # STILL at the oldest supported version at the moment of - # the binary upgrade - a real-world scenario (a user - # upgrading their PostgreSQL major while still running an - # old extension version) that the old_pg=12 leg alone - # would miss. + # old_pg=10 -- the OLDEST PG floor this job tests, AND (via TWIN + # databases in the SAME cluster, migrated by a SINGLE + # pg_upgrade call - the same trick this file uses to cross + # TEST_SCHEMA without doubling matrix legs elsewhere) the + # leg that compares BOTH update-vs-pg_upgrade orderings a + # real user could hit: + # count_nulls_upgrade_oldest_first -- stays at 0.9.6 + # through the binary pg_upgrade, updated to current + # AFTER it (on the NEW cluster). Proves pg_upgrade + # itself correctly preserves objects that are STILL at + # the oldest supported version at the moment of the + # upgrade - a real-world scenario (a user upgrading + # their PostgreSQL major while still running an old + # extension version). + # count_nulls_upgrade_current_first -- updated to + # current BEFORE the binary pg_upgrade (on the OLD + # cluster). Proves pg_upgrade correctly migrates the + # objects count_nulls' CURRENT code actually creates - + # migrating 0.9.6's objects and updating afterward + # would instead test whether pg_upgrade can migrate a + # legacy structure frozen in the past, which isn't + # actionable on its own (that version already shipped; + # nothing to fix if it turned out fragile) - which is + # exactly why this leg exists alongside oldest_first + # rather than instead of it. + # old_pg=12 -- a second, unrelated old-PG-floor data point: a SINGLE + # database (count_nulls_upgrade) using the simpler + # update-before-pg_upgrade ordering only (the original, + # pre-ordering-comparison design). The ordering comparison + # itself is a property of pg_upgrade, not of which PG + # floor it runs from, so there's no reason to re-run it at + # a second floor once old_pg=10 has already covered both + # orderings - a second ordering-comparison leg here would + # only duplicate that proof at extra CI cost. # - # Both legs still end up with a current-version database by the time - # run-suite runs, so nothing downstream (the pgTAP suite, the dependency - # guard check) needs to differ - only WHICH SIDE of the pg_upgrade call - # the update happens on. No bridge-update step first, in either leg: - # count_nulls has always been pure SQL functions with no - # SELECT-*-over-catalog views, so it has no known pg_upgrade-unsafe old - # version to bridge past. + # Every database on every leg still ends up at the current version by the + # time run-suite runs, so nothing downstream (the pgTAP suite, the + # dependency guard check) needs to differ - only how many databases exist + # per leg and which side(s) of the pg_upgrade call their update(s) happen + # on. No bridge-update step first, on any leg: count_nulls has always been + # pure SQL functions with no SELECT-*-over-catalog views, so it has no + # known pg_upgrade-unsafe old version to bridge past. # # Deliberately not doing a stepwise every-major-in-sequence climb (one # cluster walking 10->11->12->...->newest, vs. the single big jumps here): @@ -364,23 +380,44 @@ jobs: uses: actions/checkout@v4 - name: Install count_nulls into old cluster run: make install + - name: Prepare the old cluster (install + dependency guard), old_pg=10 twin databases + # old_pg=10 leg only - see the job's own header comment above for + # why this leg alone needs twin databases. Two separate databases + # (distinct names, one per ordering) so both exist in the SAME + # cluster ahead of the single pg_upgrade call below - that one + # binary upgrade migrates both at once. prepare-old installs + # count_nulls at 0.9.6 in each, then plants + proves the dependency + # guard, so a later accidental CASCADE drop anywhere in this job + # cannot silently make the eventual existing-mode run test a fresh + # install instead. + if: matrix.old_pg == '10' + run: | + bin/test_existing prepare-old count_nulls_upgrade_oldest_first "" 0.9.6 + bin/test_existing prepare-old count_nulls_upgrade_current_first "" 0.9.6 - name: Prepare the old cluster (install + dependency guard) - # prepare-old installs count_nulls at 0.9.6, then plants + proves - # the dependency guard, so a later accidental CASCADE drop anywhere - # in this job cannot silently make the eventual existing-mode run - # test a fresh install instead. + # old_pg=12 leg only (the original, single-database design) - see + # the job's own header comment above. prepare-old installs + # count_nulls at 0.9.6, then plants + proves the dependency guard, + # so a later accidental CASCADE drop anywhere in this job cannot + # silently make the eventual existing-mode run test a fresh install + # instead. + if: matrix.old_pg == '12' run: bin/test_existing prepare-old count_nulls_upgrade "" 0.9.6 - name: Update to current before pg_upgrade (proves pg_upgrade preserves CURRENT objects) - # old_pg=12 leg only - see the job's own header comment above for - # why the two legs deliberately differ here. Exercises ALTER - # EXTENSION UPDATE on the OLD cluster, BEFORE the binary pg_upgrade - # below, running the 0.9.6->stable update script, so pg_upgrade - # runs against already-current objects, not 0.9.6 ones. `make - # install` above already installed the current version's update - # scripts/control file into this (old) cluster's sharedir, so - # they're in place for this ALTER EXTENSION UPDATE to use. - if: matrix.old_pg == '12' - run: bin/test_existing update count_nulls_upgrade + # Runs on BOTH legs, but against a different database per leg - see + # the job's own header comment above. On old_pg=10 this updates + # ONLY count_nulls_upgrade_current_first (count_nulls_upgrade_oldest_first + # must NOT be touched here - it stays at 0.9.6 until AFTER + # pg_upgrade, below). On old_pg=12 this updates the single + # count_nulls_upgrade database (unchanged from the original + # design). Exercises ALTER EXTENSION UPDATE on the OLD cluster, + # BEFORE the binary pg_upgrade below, running the 0.9.6->stable + # update script, so pg_upgrade runs against already-current + # objects, not 0.9.6 ones. `make install` above already installed + # the current version's update scripts/control file into this + # (old) cluster's sharedir, so they're in place for this ALTER + # EXTENSION UPDATE to use. + run: bin/test_existing update ${{ matrix.old_pg == '10' && 'count_nulls_upgrade_current_first' || 'count_nulls_upgrade' }} - name: Install PostgreSQL ${{ matrix.new_pg }} run: apt-get install -y postgresql-${{ matrix.new_pg }} postgresql-server-dev-${{ matrix.new_pg }} - name: Install count_nulls into new cluster @@ -408,28 +445,47 @@ jobs: -name '*.log' 2>/dev/null | sort | xargs -r tail -n +1; exit 1; } pg_ctlcluster ${{ matrix.new_pg }} test start - name: Update to current after pg_upgrade (proves pg_upgrade preserves OLDEST-version objects) - # old_pg=10 leg only - see the job's own header comment above for - # why the two legs deliberately differ here. Exercises ALTER - # EXTENSION UPDATE AFTER the binary pg_upgrade above, against the - # NEW cluster (already started by the previous step, and the only - # cluster listening on 5432 at this point, since the old one was - # stopped in that same step) - the 0.9.6 objects prepare-old - # planted are thus still at 0.9.6 at the moment pg_upgrade itself - # runs, so this leg proves pg_upgrade correctly preserves objects - # still frozen at the oldest supported version, not just + # old_pg=10 leg only - old_pg=12 has no after-upgrade update step at + # all, since its single database was already updated BEFORE the + # upgrade above. Exercises ALTER EXTENSION UPDATE AFTER the binary + # pg_upgrade above, against the NEW cluster (already started by the + # previous step, and the only cluster listening on 5432 at this + # point, since the old one was stopped in that same step), on + # count_nulls_upgrade_oldest_first ONLY (count_nulls_upgrade_current_first + # was already updated before the upgrade, above, and must not be + # updated again here) - the 0.9.6 objects prepare-old planted into + # that database are thus still at 0.9.6 at the moment pg_upgrade + # itself runs, so this leg proves pg_upgrade correctly preserves + # objects still frozen at the oldest supported version, not just # already-current ones. `make install` into the new cluster above # already installed the current version's update scripts/control # file into ITS sharedir, so they're in place for this ALTER # EXTENSION UPDATE to use. if: matrix.old_pg == '10' - run: bin/test_existing update count_nulls_upgrade - - name: Run the suite against the pg_upgraded database (existing mode) + run: bin/test_existing update count_nulls_upgrade_oldest_first + - name: Run the suite against the pg_upgraded database(s) (existing mode), old_pg=10 twin databases + # old_pg=10 leg only - see the job's own header comment above. # run-suite asserts the version, re-proves the dependency guard # still blocks a non-CASCADE drop (i.e. it survived both the update # and pg_upgrade), drops the guard, then runs the suite against the # REAL pg_upgraded database via --use-existing (so pg_regress does # not drop/recreate it) - a plain fresh `make test` would silently + # test a fresh install instead of the migrated objects. Once per + # database, since each holds an independent ordering's result. + if: matrix.old_pg == '10' + run: | + bin/test_existing run-suite count_nulls_upgrade_oldest_first "" + bin/test_existing run-suite count_nulls_upgrade_current_first "" + - name: Run the suite against the pg_upgraded database (existing mode) + # old_pg=12 leg only (the original, single-database design) - see + # the job's own header comment above. run-suite asserts the + # version, re-proves the dependency guard still blocks a + # non-CASCADE drop (i.e. it survived both the update and + # pg_upgrade), drops the guard, then runs the suite against the + # REAL pg_upgraded database via --use-existing (so pg_regress does + # not drop/recreate it) - a plain fresh `make test` would silently # test a fresh install instead of the migrated objects. + if: matrix.old_pg == '12' run: bin/test_existing run-suite count_nulls_upgrade "" pg-tle-test: From 83b6214a5c1bbfefbce506468ddeed067debab4b Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Fri, 7 Aug 2026 15:49:26 -0500 Subject: [PATCH 6/9] CI: single META.json-derived floor for pg-upgrade-test, not two hardcoded majors old_pg=["10","12"] had no technical justification for the second leg (no dependency-version SQL, no other package tie) and old_pg=10 stopped being the real floor once #51 derived it as 9.4 from META.json. Add a floor_pg output to the changes job (same $FLOOR already computed for supported_pg, just re-emitted as a bare scalar like newest_pg) and drive both old_pg and new_pg from those single sources of truth instead of separately hardcoded literals. With only one leg left, the matrix.old_pg == '10'/'12' conditionals that used to pick which twin-database ordering(s) to run no longer make sense - both orderings now always run unconditionally. --- .github/workflows/ci.yml | 237 ++++++++++++++++++--------------------- 1 file changed, 107 insertions(+), 130 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 15a72c0..0c17ac3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -16,22 +16,21 @@ # own job would only duplicate this job's own # per-PG-version container/checkout setup for # no added confidence. -# pg-upgrade-test -- BINARY pg_upgrade: install 0.9.6 on an OLD -# PostgreSQL major, then binary-upgrade the -# cluster to a NEWER major. Its old_pg=10 leg -# additionally compares BOTH update-vs- -# pg_upgrade orderings a real user could hit, -# via twin databases migrated by the SAME -# binary pg_upgrade call; old_pg=12 is a -# second, unrelated old-PG-floor data point -# using the simpler single-database, single- -# ordering design - see the job's own comment -# for why the ordering comparison only needs -# one floor. A smaller old_pg/new_pg matrix -# (not the full PG matrix - by far the most +# pg-upgrade-test -- BINARY pg_upgrade: install 0.9.6 on the OLD +# PostgreSQL floor (the same META.json-derived +# floor the `changes` job's supported_pg uses - +# see its floor_pg output), then binary-upgrade +# the cluster to the newest supported major +# (changes job's newest_pg). A single leg (not +# the full PG matrix - by far the most # expensive job here, installing two full # PostgreSQL majors and running the real -# pg_upgrade binary per leg). +# pg_upgrade binary), which via twin databases +# migrated by the SAME binary pg_upgrade call +# compares BOTH update-vs-pg_upgrade orderings +# a real user could hit - see the job's own +# comment for why one floor is enough to cover +# both. # pg-tle-test -- pg_tle DEPLOYMENT: fresh install registered # through AWS pg_tle's database-backed catalog # instead of a filesystem .control file. @@ -92,8 +91,12 @@ jobs: # here instead of an edit in several jobs. `newest_pg` is the same # NEWEST constant emitted again as a bare scalar (not wrapped in the # JSON-array `supported_pg`), consumed only by the `test` job's draft-PR - # matrix reduction (see the top-of-file comment and that job's own - # comment) - so NEWEST still only needs to change in one place. + # matrix reduction and `pg-upgrade-test`'s new_pg leg (see the + # top-of-file comment and those jobs' own comments) - so NEWEST still + # only needs to change in one place. `floor_pg` is likewise the same + # FLOOR value (the META.json-derived floor, e.g. "9.4") emitted as a + # bare scalar, consumed only by `pg-upgrade-test`'s old_pg leg - so the + # oldest-supported-PG floor also only needs to change in one place. changes: name: ๐Ÿ” Detect docs-only changes & derive PG matrix runs-on: ubuntu-latest @@ -101,6 +104,7 @@ jobs: docs_only: ${{ steps.diff.outputs.docs_only }} supported_pg: ${{ steps.pg.outputs.supported_pg }} newest_pg: ${{ steps.pg.outputs.newest_pg }} + floor_pg: ${{ steps.pg.outputs.floor_pg }} steps: - name: Check out the repo uses: actions/checkout@v4 @@ -224,6 +228,16 @@ jobs: # second hardcoded "18" anywhere in this file. echo "newest_pg=$NEWEST" >> "$GITHUB_OUTPUT" + # Same idea for the OLDEST supported major: emit the exact $FLOOR + # value supported_pg above already derived from META.json, as a + # bare scalar, so pg-upgrade-test's old_pg leg can build a + # single-element list from it too - rather than hand-maintaining + # a THIRD copy of this number (the actual bug this is fixing: + # pg-upgrade-test's old_pg matrix used to hardcode ["10", "12"], + # which drifted from the real 9.4 floor the moment PR #51 + # established it here). + echo "floor_pg=$FLOOR" >> "$GITHUB_OUTPUT" + lint: name: ๐Ÿงน SQL lint runs-on: ubuntu-latest @@ -283,66 +297,55 @@ jobs: run: make verify-results TEST_LOAD_SOURCE=update # Proves count_nulls survives a BINARY pg_upgrade (in-place catalog - # migration to a newer PostgreSQL major). Installs 0.9.6 on an old - # cluster, plants a dependency guard, binary-pg_upgrades to a newer - # cluster, then runs the suite against the REAL migrated objects in - # existing mode. - # - # This job's small old_pg/new_pg matrix tests two INDEPENDENT axes, kept - # deliberately separate rather than crossed into their own matrix - # dimension (which would double this already-expensive job's cost for no - # added confidence): + # migration to a newer PostgreSQL major). Installs 0.9.6 on the oldest + # supported PostgreSQL floor, plants a dependency guard, binary- + # pg_upgrades to the newest supported major, then runs the suite against + # the REAL migrated objects in existing mode. # - # old_pg=10 -- the OLDEST PG floor this job tests, AND (via TWIN - # databases in the SAME cluster, migrated by a SINGLE - # pg_upgrade call - the same trick this file uses to cross - # TEST_SCHEMA without doubling matrix legs elsewhere) the - # leg that compares BOTH update-vs-pg_upgrade orderings a - # real user could hit: - # count_nulls_upgrade_oldest_first -- stays at 0.9.6 - # through the binary pg_upgrade, updated to current - # AFTER it (on the NEW cluster). Proves pg_upgrade - # itself correctly preserves objects that are STILL at - # the oldest supported version at the moment of the - # upgrade - a real-world scenario (a user upgrading - # their PostgreSQL major while still running an old - # extension version). - # count_nulls_upgrade_current_first -- updated to - # current BEFORE the binary pg_upgrade (on the OLD - # cluster). Proves pg_upgrade correctly migrates the - # objects count_nulls' CURRENT code actually creates - - # migrating 0.9.6's objects and updating afterward - # would instead test whether pg_upgrade can migrate a - # legacy structure frozen in the past, which isn't - # actionable on its own (that version already shipped; - # nothing to fix if it turned out fragile) - which is - # exactly why this leg exists alongside oldest_first - # rather than instead of it. - # old_pg=12 -- a second, unrelated old-PG-floor data point: a SINGLE - # database (count_nulls_upgrade) using the simpler - # update-before-pg_upgrade ordering only (the original, - # pre-ordering-comparison design). The ordering comparison - # itself is a property of pg_upgrade, not of which PG - # floor it runs from, so there's no reason to re-run it at - # a second floor once old_pg=10 has already covered both - # orderings - a second ordering-comparison leg here would - # only duplicate that proof at extra CI cost. + # A single old_pg/new_pg leg (old_pg from the changes job's floor_pg + # output, new_pg from its newest_pg output - see that job's comment; NOT + # a second/third hardcoded copy of either number), which via TWIN + # databases in the SAME cluster, migrated by a SINGLE pg_upgrade call + # (the same trick this file uses to cross TEST_SCHEMA without doubling + # matrix legs elsewhere), compares BOTH update-vs-pg_upgrade orderings a + # real user could hit: + # count_nulls_upgrade_oldest_first -- stays at 0.9.6 through the + # binary pg_upgrade, updated to current AFTER it (on the NEW + # cluster). Proves pg_upgrade itself correctly preserves objects + # that are STILL at the oldest supported version at the moment of + # the upgrade - a real-world scenario (a user upgrading their + # PostgreSQL major while still running an old extension version). + # count_nulls_upgrade_current_first -- updated to current BEFORE the + # binary pg_upgrade (on the OLD cluster). Proves pg_upgrade + # correctly migrates the objects count_nulls' CURRENT code actually + # creates - migrating 0.9.6's objects and updating afterward would + # instead test whether pg_upgrade can migrate a legacy structure + # frozen in the past, which isn't actionable on its own (that + # version already shipped; nothing to fix if it turned out + # fragile) - which is exactly why this leg exists alongside + # oldest_first rather than instead of it. + # The ordering comparison is a property of pg_upgrade itself, not of + # which PG floor it runs from, so a single floor is enough to cover + # both orderings - there was never a technical reason for a second + # old_pg floor here (the previous old_pg=12 leg predated this ordering + # comparison and had no purpose once it existed). # - # Every database on every leg still ends up at the current version by the - # time run-suite runs, so nothing downstream (the pgTAP suite, the - # dependency guard check) needs to differ - only how many databases exist - # per leg and which side(s) of the pg_upgrade call their update(s) happen - # on. No bridge-update step first, on any leg: count_nulls has always been - # pure SQL functions with no SELECT-*-over-catalog views, so it has no - # known pg_upgrade-unsafe old version to bridge past. + # Both databases still end up at the current version by the time + # run-suite runs, so nothing downstream (the pgTAP suite, the + # dependency guard check) needs to differ - only which side(s) of the + # pg_upgrade call each database's update happens on. No bridge-update + # step first: count_nulls has always been pure SQL functions with no + # SELECT-*-over-catalog views, so it has no known pg_upgrade-unsafe old + # version to bridge past. # # Deliberately not doing a stepwise every-major-in-sequence climb (one - # cluster walking 10->11->12->...->newest, vs. the single big jumps here): - # that would catch a regression specific to one particular major-to-major - # boundary, which would matter if count_nulls had views/functions touching - # catalog internals, but it doesn't - pure SQL functions over anyarray/ - # json/jsonb, nothing version-sensitive to break at a specific boundary. - # Revisit if count_nulls ever grows something catalog-touching. + # cluster walking 10->11->12->...->newest, vs. the single big jump + # here): that would catch a regression specific to one particular + # major-to-major boundary, which would matter if count_nulls had views/ + # functions touching catalog internals, but it doesn't - pure SQL + # functions over anyarray/json/jsonb, nothing version-sensitive to + # break at a specific boundary. Revisit if count_nulls ever grows + # something catalog-touching. # # Not yet crossed with TEST_SCHEMA (a later phase adds that, once it can # do so for both this job and the test job's update leg together). @@ -354,8 +357,14 @@ jobs: if: needs.changes.outputs.docs_only != 'true' && github.event.pull_request.draft != true strategy: matrix: - old_pg: ["10", "12"] - new_pg: ["18"] + # Single-element lists built from the changes job's floor_pg/ + # newest_pg scalar outputs (see that job's comment), NOT a second/ + # third hardcoded copy of either number - keeping this a `matrix:` + # (rather than plain `env:`) preserves the ${{ matrix.old_pg }}/ + # ${{ matrix.new_pg }} interpolations used throughout the steps + # below unchanged. + old_pg: ["${{ needs.changes.outputs.floor_pg }}"] + new_pg: ["${{ needs.changes.outputs.newest_pg }}"] name: ๐Ÿ”„ Binary pg_upgrade ${{ matrix.old_pg }} โ†’ ${{ matrix.new_pg }} runs-on: ubuntu-latest container: pgxn/pgxn-tools @@ -380,44 +389,28 @@ jobs: uses: actions/checkout@v4 - name: Install count_nulls into old cluster run: make install - - name: Prepare the old cluster (install + dependency guard), old_pg=10 twin databases - # old_pg=10 leg only - see the job's own header comment above for - # why this leg alone needs twin databases. Two separate databases - # (distinct names, one per ordering) so both exist in the SAME - # cluster ahead of the single pg_upgrade call below - that one - # binary upgrade migrates both at once. prepare-old installs - # count_nulls at 0.9.6 in each, then plants + proves the dependency - # guard, so a later accidental CASCADE drop anywhere in this job - # cannot silently make the eventual existing-mode run test a fresh - # install instead. - if: matrix.old_pg == '10' + - name: Prepare the old cluster (install + dependency guard), twin databases + # Two separate databases (distinct names, one per ordering) so + # both exist in the SAME cluster ahead of the single pg_upgrade + # call below - that one binary upgrade migrates both at once. + # prepare-old installs count_nulls at 0.9.6 in each, then plants + + # proves the dependency guard, so a later accidental CASCADE drop + # anywhere in this job cannot silently make the eventual + # existing-mode run test a fresh install instead. run: | bin/test_existing prepare-old count_nulls_upgrade_oldest_first "" 0.9.6 bin/test_existing prepare-old count_nulls_upgrade_current_first "" 0.9.6 - - name: Prepare the old cluster (install + dependency guard) - # old_pg=12 leg only (the original, single-database design) - see - # the job's own header comment above. prepare-old installs - # count_nulls at 0.9.6, then plants + proves the dependency guard, - # so a later accidental CASCADE drop anywhere in this job cannot - # silently make the eventual existing-mode run test a fresh install - # instead. - if: matrix.old_pg == '12' - run: bin/test_existing prepare-old count_nulls_upgrade "" 0.9.6 - name: Update to current before pg_upgrade (proves pg_upgrade preserves CURRENT objects) - # Runs on BOTH legs, but against a different database per leg - see - # the job's own header comment above. On old_pg=10 this updates - # ONLY count_nulls_upgrade_current_first (count_nulls_upgrade_oldest_first - # must NOT be touched here - it stays at 0.9.6 until AFTER - # pg_upgrade, below). On old_pg=12 this updates the single - # count_nulls_upgrade database (unchanged from the original - # design). Exercises ALTER EXTENSION UPDATE on the OLD cluster, - # BEFORE the binary pg_upgrade below, running the 0.9.6->stable - # update script, so pg_upgrade runs against already-current - # objects, not 0.9.6 ones. `make install` above already installed - # the current version's update scripts/control file into this - # (old) cluster's sharedir, so they're in place for this ALTER - # EXTENSION UPDATE to use. - run: bin/test_existing update ${{ matrix.old_pg == '10' && 'count_nulls_upgrade_current_first' || 'count_nulls_upgrade' }} + # Updates ONLY count_nulls_upgrade_current_first + # (count_nulls_upgrade_oldest_first must NOT be touched here - it + # stays at 0.9.6 until AFTER pg_upgrade, below). Exercises ALTER + # EXTENSION UPDATE on the OLD cluster, BEFORE the binary pg_upgrade + # below, running the 0.9.6->stable update script, so pg_upgrade + # runs against already-current objects, not 0.9.6 ones. `make + # install` above already installed the current version's update + # scripts/control file into this (old) cluster's sharedir, so + # they're in place for this ALTER EXTENSION UPDATE to use. + run: bin/test_existing update count_nulls_upgrade_current_first - name: Install PostgreSQL ${{ matrix.new_pg }} run: apt-get install -y postgresql-${{ matrix.new_pg }} postgresql-server-dev-${{ matrix.new_pg }} - name: Install count_nulls into new cluster @@ -445,26 +438,22 @@ jobs: -name '*.log' 2>/dev/null | sort | xargs -r tail -n +1; exit 1; } pg_ctlcluster ${{ matrix.new_pg }} test start - name: Update to current after pg_upgrade (proves pg_upgrade preserves OLDEST-version objects) - # old_pg=10 leg only - old_pg=12 has no after-upgrade update step at - # all, since its single database was already updated BEFORE the - # upgrade above. Exercises ALTER EXTENSION UPDATE AFTER the binary - # pg_upgrade above, against the NEW cluster (already started by the - # previous step, and the only cluster listening on 5432 at this - # point, since the old one was stopped in that same step), on + # Exercises ALTER EXTENSION UPDATE AFTER the binary pg_upgrade + # above, against the NEW cluster (already started by the previous + # step, and the only cluster listening on 5432 at this point, + # since the old one was stopped in that same step), on # count_nulls_upgrade_oldest_first ONLY (count_nulls_upgrade_current_first # was already updated before the upgrade, above, and must not be # updated again here) - the 0.9.6 objects prepare-old planted into # that database are thus still at 0.9.6 at the moment pg_upgrade - # itself runs, so this leg proves pg_upgrade correctly preserves + # itself runs, so this proves pg_upgrade correctly preserves # objects still frozen at the oldest supported version, not just # already-current ones. `make install` into the new cluster above # already installed the current version's update scripts/control # file into ITS sharedir, so they're in place for this ALTER # EXTENSION UPDATE to use. - if: matrix.old_pg == '10' run: bin/test_existing update count_nulls_upgrade_oldest_first - - name: Run the suite against the pg_upgraded database(s) (existing mode), old_pg=10 twin databases - # old_pg=10 leg only - see the job's own header comment above. + - name: Run the suite against the pg_upgraded database(s) (existing mode), twin databases # run-suite asserts the version, re-proves the dependency guard # still blocks a non-CASCADE drop (i.e. it survived both the update # and pg_upgrade), drops the guard, then runs the suite against the @@ -472,21 +461,9 @@ jobs: # not drop/recreate it) - a plain fresh `make test` would silently # test a fresh install instead of the migrated objects. Once per # database, since each holds an independent ordering's result. - if: matrix.old_pg == '10' run: | bin/test_existing run-suite count_nulls_upgrade_oldest_first "" bin/test_existing run-suite count_nulls_upgrade_current_first "" - - name: Run the suite against the pg_upgraded database (existing mode) - # old_pg=12 leg only (the original, single-database design) - see - # the job's own header comment above. run-suite asserts the - # version, re-proves the dependency guard still blocks a - # non-CASCADE drop (i.e. it survived both the update and - # pg_upgrade), drops the guard, then runs the suite against the - # REAL pg_upgraded database via --use-existing (so pg_regress does - # not drop/recreate it) - a plain fresh `make test` would silently - # test a fresh install instead of the migrated objects. - if: matrix.old_pg == '12' - run: bin/test_existing run-suite count_nulls_upgrade "" pg-tle-test: needs: [changes] From 937a7acab25c2967cc24da6721130dcc682b0932 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Fri, 7 Aug 2026 16:23:59 -0500 Subject: [PATCH 7/9] ci.yml: describe changes job outputs by current purpose, not by naming consumers The comments explaining supported_pg/newest_pg/floor_pg (and the newest_pg bare-scalar rationale) tied each output's reason for existing to a specific downstream job/feature (test's draft-PR matrix reduction, pg-upgrade-test's legs) and, for floor_pg, narrated the PR #51 history of the bug it fixed. Rework them to just state what each value is and its general purpose, so the comment doesn't need updating every time a new consumer is added or removed. --- .github/workflows/ci.yml | 49 ++++++++++++++++++---------------------- 1 file changed, 22 insertions(+), 27 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0c17ac3..0ab6ce0 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -83,20 +83,19 @@ jobs: # would never report on doc-only pushes and get stuck Pending in branch # protection. # - # Also derives the supported-PostgreSQL-major list the test job consumes, - # from a single NEWEST constant plus META.json's own declared minimum - # (not a second hand-maintained floor constant): every job that cares - # which majors are supported reads the SAME list, so they can't silently - # drift onto different sets, and adding a new major is a one-line change - # here instead of an edit in several jobs. `newest_pg` is the same - # NEWEST constant emitted again as a bare scalar (not wrapped in the - # JSON-array `supported_pg`), consumed only by the `test` job's draft-PR - # matrix reduction and `pg-upgrade-test`'s new_pg leg (see the - # top-of-file comment and those jobs' own comments) - so NEWEST still - # only needs to change in one place. `floor_pg` is likewise the same - # FLOOR value (the META.json-derived floor, e.g. "9.4") emitted as a - # bare scalar, consumed only by `pg-upgrade-test`'s old_pg leg - so the - # oldest-supported-PG floor also only needs to change in one place. + # Also derives the supported-PostgreSQL-major list every other job reads, + # from a single NEWEST constant plus META.json's own declared minimum (not + # a second hand-maintained floor constant) - every job that cares which + # majors are supported reads the SAME values, so they can't silently drift + # onto different sets, and adding a new major is a one-line change here + # instead of an edit in several jobs. + # - supported_pg: the full descending list (NEWEST down to the + # META.json-derived floor), as a JSON array, for jobs that need every + # supported major. + # - newest_pg: just the single newest major, as a bare scalar, for + # anything that needs one value instead of the full array. + # - floor_pg: just the single oldest (META.json-derived) major, as a + # bare scalar, for the same reason. changes: name: ๐Ÿ” Detect docs-only changes & derive PG matrix runs-on: ubuntu-latest @@ -174,9 +173,9 @@ jobs: id: pg run: | # A dozen-odd lines to replace what looks like a handful of version - # references, but it buys CONSISTENCY: the fresh-install/update - # `test` matrix derives its PostgreSQL set from this ONE source, so - # it cannot silently drift onto a different list. Adding a new + # references, but it buys CONSISTENCY: every job that needs the + # supported-major list derives it from this ONE source, so none of + # them can silently drift onto a different list. Adding a new # major is a one-line NEWEST bump here, not an edit in N places. # # The floor itself is NOT a second hardcoded constant here: it's @@ -222,20 +221,16 @@ jobs: echo "supported_pg=$(json)" >> "$GITHUB_OUTPUT" - # Also emitted as a bare scalar (not a JSON array) so the `test` - # job's draft-PR matrix reduction (see its own comment) can build a - # single-element list from it via fromJSON(format(...)) without a - # second hardcoded "18" anywhere in this file. + # Also emitted as a bare scalar (not a JSON array), for anything + # that needs just the single newest major instead of the full + # array - so NEWEST still only needs to change in one place. echo "newest_pg=$NEWEST" >> "$GITHUB_OUTPUT" # Same idea for the OLDEST supported major: emit the exact $FLOOR # value supported_pg above already derived from META.json, as a - # bare scalar, so pg-upgrade-test's old_pg leg can build a - # single-element list from it too - rather than hand-maintaining - # a THIRD copy of this number (the actual bug this is fixing: - # pg-upgrade-test's old_pg matrix used to hardcode ["10", "12"], - # which drifted from the real 9.4 floor the moment PR #51 - # established it here). + # bare scalar, for anything that needs just the single oldest + # major instead of the full array - rather than hand-maintaining a + # THIRD copy of this number. echo "floor_pg=$FLOOR" >> "$GITHUB_OUTPUT" lint: From 7cc1d1f3d7d65987ed924b29de9e7dd16b5701fe Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Fri, 7 Aug 2026 16:48:16 -0500 Subject: [PATCH 8/9] CI: skip the old cluster's create/destroy/recreate cycle, shorten twin db names pg-start's own NO_CLUSTER option lets us install PostgreSQL and create the old pg_upgrade-test cluster with --data-checksums in one step, instead of starting a default cluster only to immediately stop/drop/recreate it. Also drop the redundant count_nulls_ prefix from the twin database names (upgrade_oldest_first/upgrade_current_first) - we're already inside the count_nulls repo/test suite. --- .github/workflows/ci.yml | 36 +++++++++++++++++------------------- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0ab6ce0..ef7d11a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -304,13 +304,13 @@ jobs: # (the same trick this file uses to cross TEST_SCHEMA without doubling # matrix legs elsewhere), compares BOTH update-vs-pg_upgrade orderings a # real user could hit: - # count_nulls_upgrade_oldest_first -- stays at 0.9.6 through the + # upgrade_oldest_first -- stays at 0.9.6 through the # binary pg_upgrade, updated to current AFTER it (on the NEW # cluster). Proves pg_upgrade itself correctly preserves objects # that are STILL at the oldest supported version at the moment of # the upgrade - a real-world scenario (a user upgrading their # PostgreSQL major while still running an old extension version). - # count_nulls_upgrade_current_first -- updated to current BEFORE the + # upgrade_current_first -- updated to current BEFORE the # binary pg_upgrade (on the OLD cluster). Proves pg_upgrade # correctly migrates the objects count_nulls' CURRENT code actually # creates - migrating 0.9.6's objects and updating afterward would @@ -368,15 +368,13 @@ jobs: # refuses to run. INITDB_OPTS: --data-checksums --auth trust steps: - - name: Start PostgreSQL ${{ matrix.old_pg }} - run: pg-start ${{ matrix.old_pg }} - - name: Recreate old cluster with data checksums enabled + - name: Install PostgreSQL ${{ matrix.old_pg }} (cluster created next, with the right initdb options from the start) + run: NO_CLUSTER=1 pg-start ${{ matrix.old_pg }} + - name: Create old cluster with data checksums enabled run: | - pg_ctlcluster ${{ matrix.old_pg }} test stop - pg_dropcluster ${{ matrix.old_pg }} test - # -p 5432: pg_createcluster assigns the next available port, which - # may not be 5432 after pg-start has claimed and released it. - # Force 5432 so subsequent psql/createdb calls connect without -p. + # -p 5432: pg_createcluster would otherwise assign the next + # available port; force 5432 so subsequent psql/createdb calls + # connect without -p. pg_createcluster -p 5432 ${{ matrix.old_pg }} test -- $INITDB_OPTS pg_ctlcluster ${{ matrix.old_pg }} test start pg_isready -t 30 @@ -393,11 +391,11 @@ jobs: # anywhere in this job cannot silently make the eventual # existing-mode run test a fresh install instead. run: | - bin/test_existing prepare-old count_nulls_upgrade_oldest_first "" 0.9.6 - bin/test_existing prepare-old count_nulls_upgrade_current_first "" 0.9.6 + bin/test_existing prepare-old upgrade_oldest_first "" 0.9.6 + bin/test_existing prepare-old upgrade_current_first "" 0.9.6 - name: Update to current before pg_upgrade (proves pg_upgrade preserves CURRENT objects) - # Updates ONLY count_nulls_upgrade_current_first - # (count_nulls_upgrade_oldest_first must NOT be touched here - it + # Updates ONLY upgrade_current_first + # (upgrade_oldest_first must NOT be touched here - it # stays at 0.9.6 until AFTER pg_upgrade, below). Exercises ALTER # EXTENSION UPDATE on the OLD cluster, BEFORE the binary pg_upgrade # below, running the 0.9.6->stable update script, so pg_upgrade @@ -405,7 +403,7 @@ jobs: # install` above already installed the current version's update # scripts/control file into this (old) cluster's sharedir, so # they're in place for this ALTER EXTENSION UPDATE to use. - run: bin/test_existing update count_nulls_upgrade_current_first + run: bin/test_existing update upgrade_current_first - name: Install PostgreSQL ${{ matrix.new_pg }} run: apt-get install -y postgresql-${{ matrix.new_pg }} postgresql-server-dev-${{ matrix.new_pg }} - name: Install count_nulls into new cluster @@ -437,7 +435,7 @@ jobs: # above, against the NEW cluster (already started by the previous # step, and the only cluster listening on 5432 at this point, # since the old one was stopped in that same step), on - # count_nulls_upgrade_oldest_first ONLY (count_nulls_upgrade_current_first + # upgrade_oldest_first ONLY (upgrade_current_first # was already updated before the upgrade, above, and must not be # updated again here) - the 0.9.6 objects prepare-old planted into # that database are thus still at 0.9.6 at the moment pg_upgrade @@ -447,7 +445,7 @@ jobs: # already installed the current version's update scripts/control # file into ITS sharedir, so they're in place for this ALTER # EXTENSION UPDATE to use. - run: bin/test_existing update count_nulls_upgrade_oldest_first + run: bin/test_existing update upgrade_oldest_first - name: Run the suite against the pg_upgraded database(s) (existing mode), twin databases # run-suite asserts the version, re-proves the dependency guard # still blocks a non-CASCADE drop (i.e. it survived both the update @@ -457,8 +455,8 @@ jobs: # test a fresh install instead of the migrated objects. Once per # database, since each holds an independent ordering's result. run: | - bin/test_existing run-suite count_nulls_upgrade_oldest_first "" - bin/test_existing run-suite count_nulls_upgrade_current_first "" + bin/test_existing run-suite upgrade_oldest_first "" + bin/test_existing run-suite upgrade_current_first "" pg-tle-test: needs: [changes] From c094b680df16d12f21e3089b4d00cb1c368b4699 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Fri, 7 Aug 2026 16:58:25 -0500 Subject: [PATCH 9/9] bin/test_existing: fix assert_version's false-test-under-set-e abort [ "$expected" = current ] && expected=$(current_version) aborts the whole script silently whenever $expected isn't literally "current": under `set -e`, a false left side of && makes the compound command's exit status non-zero, which is fatal. Switch to `if`, matching the same fix already applied to update_ext() a few lines below. --- bin/test_existing | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/test_existing b/bin/test_existing index f0b12f5..b43c987 100755 --- a/bin/test_existing +++ b/bin/test_existing @@ -122,7 +122,7 @@ drop_guard() { assert_version() { local db=$1 expected=$2 installed - [ "$expected" = current ] && expected=$(current_version) + if [ "$expected" = current ]; then expected=$(current_version); fi installed=$(installed_version "$db") echo "version check '$db': installed='$installed' expected='$expected'" if [ -z "$installed" ] || [ -z "$expected" ] || [ "$installed" != "$expected" ]; then