Real pg_upgrade support via a reduced bin/test_existing

.github/workflows/ci.yml

@@ -16,6 +16,19 @@
 #                              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, 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.
@@ -202,6 +215,114 @@ 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). 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.
+  #
+  # 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: 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
+        # 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: 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
+        # 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:
     needs: [changes]
     if: needs.changes.outputs.docs_only != 'true'
@@ -332,7 +453,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:

bin/test_existing

@@ -0,0 +1,216 @@
+#!/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) -> 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. 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
+# 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 <subcommand> [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 <subcommand> [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 "$@"

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
+$$;