Run bin/compare_fresh_vs_update against the real pg_upgraded database too

Makefile

@@ -25,3 +25,35 @@ testdeps: $(wildcard test/*/*.sql) $(wildcard test/*.sql) # Be careful not to in
 # still fails loudly instead of looking identical to a deliberately empty one.
 TEST_SCHEMA ?=
 export PGOPTIONS := $(PGOPTIONS) -c count_nulls.test_schema=$(TEST_SCHEMA)
+
+# TEST_LOAD_SOURCE selects how test/install/load.sql installs count_nulls
+# for the WHOLE test run:
+#   - fresh (default): CREATE EXTENSION count_nulls (current version).
+#   - update: CREATE EXTENSION at the oldest version we still ship a full
+#     install script for (0.9.6), then ALTER EXTENSION UPDATE to current -
+#     committed, since test/install runs outside any per-test rolled-back
+#     transaction (see pgxntool/README.asc's Update & Upgrade (U&U) Testing
+#     section for why the commit matters).
+#   - existing: count_nulls is already installed (a real `pg_upgrade` run,
+#     external to this invocation) - test/install only asserts it's present
+#     and current, it does not drop/create/update anything. Meant to be run
+#     with CONTRIB_TESTDB=<db> EXTRA_REGRESS_OPTS=--use-existing against a
+#     real database, not via a make wrapper here.
+#
+# "update" (this) is extension-level (ALTER EXTENSION UPDATE); "upgrade" is
+# cluster-level (pg_upgrade) - 'existing' is how that axis is exercised.
+#
+# Propagated the same way as TEST_SCHEMA: via the count_nulls.test_load_mode
+# GUC, exported unconditionally through PGOPTIONS, read without missing_ok.
+TEST_LOAD_SOURCE ?= fresh
+ifeq ($(filter $(TEST_LOAD_SOURCE),fresh update existing),)
+$(error TEST_LOAD_SOURCE must be 'fresh', 'update' or 'existing', got '$(TEST_LOAD_SOURCE)')
+endif
+export PGOPTIONS := $(PGOPTIONS) -c count_nulls.test_load_mode=$(TEST_LOAD_SOURCE)
+
+# Convenience wrapper: `make test-update` == `make test TEST_LOAD_SOURCE=update`.
+# Must recurse (a fresh $(MAKE)) rather than depend on `test`, so the
+# parse-time TEST_LOAD_SOURCE conditional above re-evaluates with update set.
+.PHONY: test-update
+test-update:
+	$(MAKE) test TEST_LOAD_SOURCE=update

bin/compare_fresh_vs_update

@@ -0,0 +1,126 @@
+#!/usr/bin/env bash
+#
+# compare_fresh_vs_update - structurally compare every object the count_nulls
+# extension owns between a FRESH install (CREATE EXTENSION at current) and an
+# UPDATED one (CREATE EXTENSION at FROM_VERSION, then ALTER EXTENSION
+# UPDATE), in the same schema. A fixed pgTAP expected-output suite (see
+# TEST_LOAD_SOURCE in the Makefile) only proves the specific behaviors it
+# happens to assert still work - it does not prove an update script left
+# object definitions/comments/ACLs BYTE-FOR-BYTE identical to a fresh
+# install of the same version. This catches divergence classes a fixed
+# suite doesn't already know to test for.
+#
+# Modeled on the manual technique used in Postgres-Extensions/cat_tools#46,
+# which found a real bug this way (a pre-0.2.2 update path left
+# `EXECUTE PROCEDURE` hardcoded in a trigger body that fresh installs had
+# already updated to `EXECUTE FUNCTION`). Unlike that PR, this is committed,
+# reusable tooling rather than a one-off manual diff.
+#
+# What's compared, per object the extension owns (discovered live via
+# pg_depend - see extension_members(), not a hardcoded object list, so a
+# newly added function is automatically covered without editing this
+# script): pg_get_functiondef() (full definition: schema, args, body,
+# volatility, strictness - everything), its comment (obj_description), and
+# its ACL (proacl). count_nulls currently ships only functions/triggers, no
+# views - the doc this is modeled on also compares pg_get_viewdef/type
+# labels/extension membership for extensions that have those; add a query
+# for the relevant catalog (pg_class for views, pg_type for types, ...) the
+# same way if count_nulls ever grows one.
+#
+# USAGE: bin/compare_fresh_vs_update [SCHEMA] [FROM_VERSION] [EXISTING_DB]
+#   SCHEMA       - schema both installs target (default: unqualified, same
+#                  as TEST_SCHEMA empty - see the Makefile). Both installs
+#                  use the SAME schema, since the point is comparing object
+#                  definitions, not exercising schema-qualification (that's
+#                  TEST_SCHEMA's job in the regular suite).
+#   FROM_VERSION - the update origin (default: 0.9.6, the oldest version
+#                  count_nulls still ships a full install script for).
+#                  Ignored when EXISTING_DB is given - that database's
+#                  history is whatever already produced it.
+#   EXISTING_DB  - compare against this ALREADY-POPULATED database instead
+#                  of creating+updating a scratch one (default: none, create
+#                  our own scratch "updated" database as before). Lets
+#                  callers that produced their update/upgrade result some
+#                  other way - e.g. bin/test_existing's real binary
+#                  pg_upgrade path - reuse this same comparison without this
+#                  script re-deriving that database itself. The caller owns
+#                  EXISTING_DB's lifecycle: it is never created, updated, or
+#                  dropped here, only queried.
+#
+# Exits nonzero (and prints a real diff) on ANY difference. Scratch
+# database(s) this script created itself are dropped on exit regardless of
+# outcome; an EXISTING_DB passed in is left untouched.
+set -euo pipefail
+
+cd "$(dirname "$(readlink -f "$0")")/.."
+
+schema=${1:-}
+from_version=${2:-0.9.6}
+existing_db=${3:-}
+
+fresh_db=compare_fresh_vs_update_fresh
+update_db=${existing_db:-compare_fresh_vs_update_updated}
+fresh_snapshot=$(mktemp)
+update_snapshot=$(mktemp)
+
+cleanup() {
+  dropdb --if-exists "$fresh_db"
+  # Only drop update_db if we created it ourselves - an EXISTING_DB belongs
+  # to the caller (e.g. the real pg_upgraded database bin/test_existing is
+  # still using) and must survive this script running.
+  if [ -z "$existing_db" ]; then
+    dropdb --if-exists "$update_db"
+  fi
+  rm -f "$fresh_snapshot" "$update_snapshot"
+}
+trap cleanup EXIT
+
+# extension_members(): every object pg_depend records as owned by the
+# count_nulls extension (deptype 'e'), restricted to pg_proc for now (see
+# the header comment on extending this). Ordered by name/args so the two
+# snapshots line up for a textual diff regardless of OID assignment order,
+# which differs between a fresh install and an update.
+query() {
+  cat <<'SQL'
+SELECT
+  '-- ' || p.oid::regprocedure::text || E'\n'
+  || pg_get_functiondef(p.oid) || E'\n'
+  || '-- comment: ' || coalesce(obj_description(p.oid, 'pg_proc'), '(none)') || E'\n'
+  || '-- acl: ' || coalesce(p.proacl::text, '(default)') || E'\n'
+FROM pg_depend d
+JOIN pg_extension x ON d.refobjid = x.oid AND x.extname = 'count_nulls'
+JOIN pg_proc p ON d.objid = p.oid AND d.classid = 'pg_proc'::regclass
+WHERE d.deptype = 'e'
+ORDER BY p.proname, p.oid::regprocedure::text;
+SQL
+}
+
+install_in_schema() {
+  local sql=""
+  if [ -n "$schema" ]; then
+    sql="CREATE SCHEMA IF NOT EXISTS \"$schema\"; SET search_path = \"$schema\"; "
+  fi
+  echo "$sql"
+}
+
+createdb "$fresh_db"
+psql -d "$fresh_db" -v ON_ERROR_STOP=1 -c "$(install_in_schema)CREATE EXTENSION count_nulls"
+
+if [ -z "$existing_db" ]; then
+  createdb "$update_db"
+  psql -d "$update_db" -v ON_ERROR_STOP=1 -c "$(install_in_schema)CREATE EXTENSION count_nulls VERSION '$from_version'"
+  psql -d "$update_db" -v ON_ERROR_STOP=1 -c "SET client_min_messages = WARNING; ALTER EXTENSION count_nulls UPDATE"
+fi
+
+psql -d "$fresh_db"  -tA -v ON_ERROR_STOP=1 -c "$(query)" > "$fresh_snapshot"
+psql -d "$update_db" -tA -v ON_ERROR_STOP=1 -c "$(query)" > "$update_snapshot"
+
+source_desc="$from_version->current update"
+[ -n "$existing_db" ] && source_desc="'$existing_db'"
+
+if diff -u "$fresh_snapshot" "$update_snapshot"; then
+  echo "OK: fresh install and $source_desc produce IDENTICAL object definitions/comments/ACLs"
+else
+  echo "FAIL: $source_desc diverges from a fresh install of the same version - see diff above" >&2
+  exit 1
+fi

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 <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 "$@"