diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3088ee2..75ea629 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -94,24 +94,69 @@ jobs: - name: Test on PostgreSQL ${{ matrix.pg }} run: make test + # Proves cat_tools survives a BINARY pg_upgrade (in-place catalog migration to a + # newer PostgreSQL major), not just an in-place extension update. Installs an old + # cat_tools 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. Catches view/catalog breakage + # that only surfaces when objects created on an old server are read on a new one. pg-upgrade-test: needs: [changes] if: needs.changes.outputs.docs_only != 'true' strategy: matrix: include: + # old_pg=10: install an old cat_tools on the oldest PostgreSQL, then + # BRIDGE-update to 0.2.3 (the current version) on PG10 BEFORE pg_upgrade + # -- modelling a user who updates the extension, then binary-upgrades to + # a newer major. + # + # old_install SHOULD be 0.2.0: proving that a database installed at the + # OLDEST version can update to 0.2.3 and then survive binary pg_upgrade + # is the whole point of this leg. It is TEMPORARILY pinned to a FRESH + # 0.2.2 because a 0.2.0-origin database cannot yet survive the journey -- + # the shipped 0.2.x update scripts have fresh-vs-update divergences that + # break on newer PostgreSQL. Specifically cat_tools.trigger__parse as + # produced by the update scripts hardcodes `EXECUTE PROCEDURE` (PG11+ + # needs an `EXECUTE (?:PROCEDURE|FUNCTION)` match) and omits the + # empty-args guard, so it errors with "cannot determine type of empty + # array" when the suite parses a trigger (surfaces on PG18). (The + # pg_class_v view divergence is already repaired by 0.2.2->0.2.3.) A + # FRESH 0.2.2 has the corrected trigger__parse and already strips + # relhasoids/relhaspkey (fixed `!= ALL` omit_column), so it is both + # pg_upgrade-safe and free of that divergence. + # + # Restore old_install=0.2.0 once the 0.2.2->0.2.3 repair also fixes + # trigger__parse (and any remaining divergence) -- that work is in + # progress. Meanwhile the 0.2.0-only install and its 0.2.2->0.2.3 view + # rebuild stay covered by extension-update-test on PG10. - old_pg: "10" new_pg: "11" + old_install: "0.2.2" + bridge_to: "0.2.3" - old_pg: "10" new_pg: "18" + old_install: "0.2.2" + bridge_to: "0.2.3" + # old_pg>=11: such users are already on 0.2.2+. A FRESH 0.2.2 install + # already strips relhasoids/relhaspkey (fixed `!= ALL` omit_column), so + # it is pg_upgrade-safe with no bridge. pg_upgrade, then update to current. - old_pg: "11" new_pg: "12" + old_install: "0.2.2" + bridge_to: "" - old_pg: "11" new_pg: "18" + old_install: "0.2.2" + bridge_to: "" - old_pg: "12" new_pg: "13" + old_install: "0.2.2" + bridge_to: "" - old_pg: "12" new_pg: "18" + old_install: "0.2.2" + bridge_to: "" name: 🔄 Binary pg_upgrade ${{ matrix.old_pg }} → ${{ matrix.new_pg }} runs-on: ubuntu-latest container: pgxn/pgxn-tools @@ -138,8 +183,17 @@ jobs: run: apt-get install -y rsync - name: Install cat_tools into old cluster run: make install - - name: Install cat_tools extension into upgrade test database - run: psql -c "CREATE EXTENSION cat_tools" + - name: Prepare the old cluster (install + bridge + dependency guard) + # prepare-old installs cat_tools at old_install and, when bridge_to is set + # (old_pg=10), ALTER EXTENSION UPDATEs to it BEFORE pg_upgrade -- the + # bridge that makes the views pg_upgrade-safe (see prepare-old in the + # script). It then plants + proves the dependency guard so the later + # existing-mode run cannot silently drop+reinstall and test a fresh + # install instead. The extension is always older than or equal to + # current, so the post-upgrade ALTER EXTENSION UPDATE is exercised too. + run: >- + bin/test_existing prepare-old cat_tools_upgrade + "${{ matrix.old_install }}" "${{ matrix.bridge_to }}" - name: Install PostgreSQL ${{ matrix.new_pg }} run: apt-get install -y postgresql-${{ matrix.new_pg }} postgresql-server-dev-${{ matrix.new_pg }} - name: Install cat_tools into new cluster @@ -166,38 +220,42 @@ 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: Verify extension version after upgrade - run: | - INSTALLED=$(psql -tAc "SELECT extversion FROM pg_extension WHERE extname = 'cat_tools'") - EXPECTED=$(make -s print-PGXNVERSION 2>/dev/null | sed -n 's/.*set to "\(.*\)"$/\1/p') - echo "installed=$INSTALLED expected=$EXPECTED" - # Assert installed version (reported by pg_extension) matches what's specified in META.in.json - if [ -z "$INSTALLED" ] || [ -z "$EXPECTED" ] || [ "$INSTALLED" != "$EXPECTED" ]; then - echo "FAIL: installed='$INSTALLED' expected='$EXPECTED'"; exit 1 - fi - - name: Run test suite on upgraded cluster - run: make test - # TODO: also test ALTER EXTENSION cat_tools UPDATE here, once the - # pg_upgrade source versions (e.g. 0.2.0) can install on the new_pg - # version. Currently the pre-0.2.2 install scripts fail on PG11+ so - # we cannot pg_upgrade from a cluster that has them installed. + - 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 + # version-to-version scripts up to the current version. A no-op when the + # bridge already reached current (old_pg=10), which still verifies UPDATE + # does not error on pg_upgraded objects. + run: bin/test_existing update cat_tools_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), 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 cat_tools_upgrade + # Proves the in-place extension update path: CREATE EXTENSION at an OLD cat_tools + # version then ALTER EXTENSION UPDATE (no pg_upgrade, same PostgreSQL). On PG12+ + # it updates 0.2.2 -> current and runs the FULL suite against the updated + # database (same expected output as a fresh install, so an updated DB must behave + # identically). The PG10 leg only exercises the pre-0.2.2 update scripts, the + # sole version where they still load. Complements pg-upgrade-test, which covers + # the cross-major-version binary upgrade instead. extension-update-test: needs: [changes] if: needs.changes.outputs.docs_only != 'true' strategy: matrix: - # Restricted to PG10 only because the pre-0.2.2 install scripts use - # unqualified SELECT * in views over pg_attribute/pg_constraint: - # - PG11 added attmissingval (pseudo-type anyarray) to pg_attribute, - # so cat_tools--0.2.0.sql and cat_tools--0.2.1.sql fail on PG11+ - # with "column attmissingval has pseudo-type anyarray". - # - PG12 made the `oid` system column visible in SELECT *, causing - # "column oid specified more than once" on PG12+. - # PG10 is the only version where both old scripts install cleanly. - # TODO: when a future version's install script works on PG11+, expand - # this matrix and add tests for the update path from 0.2.2 onwards. - pg: [10] + # The update-scenario legs (0.2.2 -> current + full suite, existing mode) + # run on PG12-18. 0.2.2 is the update-from floor: the 0.2.0/0.2.1 install + # scripts fail on PG11+/PG12+. + # PG10 is the ONLY version where the pre-0.2.2 install scripts still load, + # so it is the only place the 0.2.0->0.2.2 / 0.2.1->0.2.2 update scripts + # and the 0.2.2->0.2.3 view rebuild on the broken path can be exercised. + # It runs only those legacy checks (no current-version suite; that is + # covered on PG10 by the `test` job). See the per-step `if` guards. + pg: [18, 17, 16, 15, 14, 13, 12, 10] name: ⬆️ Extension update test on PostgreSQL ${{ matrix.pg }} runs-on: ubuntu-latest container: pgxn/pgxn-tools @@ -210,40 +268,37 @@ jobs: run: apt-get install -y rsync - name: Install cat_tools (all versions) run: make install - - name: Test update from 0.2.0 to current + - name: Test pre-0.2.2 update scripts + 0.2.2→0.2.3 rebuild (PG10 only) + # 0.2.0/0.2.1 install only on PG10; their update scripts target 0.2.2 (not + # the current version) and are otherwise never exercised. PG takes the + # shortest path, so 0.2.0 → 0.2.2 uses the direct 0.2.0--0.2.2 script; + # starting at 0.2.1 exercises the 0.2.1--0.2.2 script directly. update-check + # asserts each lands on 0.2.2. + # + # The third check exercises the REAL broken path end to end: a 0.2.0 + # install on PG10 leaves relhasoids in _cat_tools.pg_class_v (the buggy + # 0.2.0--0.2.2 omit_column no-op), and updating to 0.2.3 routes through + # 0.2.2--0.2.3 (shortest path 0.2.0--0.2.2 then 0.2.2--0.2.3), firing the + # conditional rebuild that strips relhasoids. On PG12+ relhasoids never + # existed, so only PG10 exercises the rebuild. The psql assertion fails + # loudly if the rebuild did not fire (relhasoids still present) -- + # complementing the stronger 10→18 pg_upgrade bridge leg. `$$` is escaped + # as `\$\$` so the shell passes literal dollar quotes through to psql. + if: matrix.pg == '10' run: | - psql -c "CREATE EXTENSION cat_tools VERSION '0.2.0'" - psql -c "ALTER EXTENSION cat_tools UPDATE" - INSTALLED=$(psql -tAc "SELECT extversion FROM pg_extension WHERE extname = 'cat_tools'") - EXPECTED=$(make -s print-PGXNVERSION 2>/dev/null | sed -n 's/.*set to "\(.*\)"$/\1/p') - echo "installed=$INSTALLED expected=$EXPECTED" - # 0.2.0 -> current; the 0.2.1->0.2.2 script is covered by the next step. - if [ -z "$INSTALLED" ] || [ -z "$EXPECTED" ] || [ "$INSTALLED" != "$EXPECTED" ]; then - echo "FAIL: installed='$INSTALLED' expected='$EXPECTED'"; exit 1 - fi - # Verify the 0.2.2->0.2.3 view rebuild actually FIRED (not just ran). On - # PG10 the 0.2.0->0.2.2 path leaves _cat_tools.pg_class_v carrying - # relhasoids (the buggy `!= ANY` no-op never stripped it), and the - # conditional 0.2.2->0.2.3 repair must detect that and rebuild the view - # without it. This is the REAL broken path: on PG12+ relhasoids never - # existed, so only PG10 exercises the rebuild. ON_ERROR_STOP makes the - # RAISE EXCEPTION fail the step. - psql -v ON_ERROR_STOP=1 -c "DO \$\$ BEGIN IF EXISTS (SELECT 1 FROM pg_attribute WHERE attrelid = '_cat_tools.pg_class_v'::regclass AND attname = 'relhasoids' AND NOT attisdropped AND attnum > 0) THEN RAISE EXCEPTION 'pg_class_v still exposes relhasoids after update to current -- 0.2.2->0.2.3 rebuild did not fire'; END IF; END \$\$" - - name: Test update 0.2.1 → 0.2.2 - run: | - # PG takes the shortest update path, so the 0.2.0->current test above goes - # direct 0.2.0->0.2.2 and never runs the 0.2.1->0.2.2 script. Start at 0.2.1 - # and target 0.2.2 explicitly so that script is exercised. - createdb cat_tools_from_021 - psql -d cat_tools_from_021 -c "CREATE EXTENSION cat_tools VERSION '0.2.1'" - psql -d cat_tools_from_021 -c "ALTER EXTENSION cat_tools UPDATE TO '0.2.2'" - INSTALLED=$(psql -d cat_tools_from_021 -tAc "SELECT extversion FROM pg_extension WHERE extname = 'cat_tools'") - echo "installed=$INSTALLED" - if [ -z "$INSTALLED" ] || [ "$INSTALLED" != "0.2.2" ]; then - echo "FAIL: expected 0.2.2, got '$INSTALLED'"; exit 1 - fi - - name: Run test suite on updated extension - run: make test + bin/test_existing update-check cat_tools_from_020 0.2.0 0.2.2 + bin/test_existing update-check cat_tools_from_021 0.2.1 0.2.2 + bin/test_existing update-check cat_tools_rebuild_020 0.2.0 0.2.3 + psql -d cat_tools_rebuild_020 -v ON_ERROR_STOP=1 -c "DO \$\$ BEGIN IF EXISTS (SELECT 1 FROM pg_attribute WHERE attrelid='_cat_tools.pg_class_v'::regclass AND attname='relhasoids' AND NOT attisdropped AND attnum>0) THEN RAISE EXCEPTION 'pg_class_v still exposes relhasoids after update through 0.2.2->0.2.3 -- rebuild did not fire'; END IF; END \$\$" + - name: Update 0.2.2 → current and run the suite (existing mode, PG12+) + # update-scenario creates a real database at 0.2.2, plants + proves the + # dependency guard, ALTER EXTENSION UPDATEs to the current version, and + # runs the suite against that updated database in existing mode (asserting + # the version and that the guard still blocks a drop). Reusing the SAME + # suite and expected output asserts the updated database behaves + # identically to a fresh install. + if: matrix.pg != '10' + run: bin/test_existing update-scenario cat_tools_update 0.2.2 # A single stable check name for use as a required status check in branch # protection rules. Matrix jobs produce check names like "🐘 PostgreSQL 14" diff --git a/.gitignore b/.gitignore index 6fc6141..8dfbc48 100644 --- a/.gitignore +++ b/.gitignore @@ -19,9 +19,6 @@ control.mk # built targets # Note: Version-specific files (sql/*--*.sql) are now tracked in git and should be committed -# test targets -/test/.build/ - # Test artifacts results/ regression.diffs diff --git a/Makefile b/Makefile index b7774e5..c0e86e7 100644 --- a/Makefile +++ b/Makefile @@ -1,5 +1,65 @@ testdeps: $(wildcard test/*.sql test/helpers/*.sql) # Be careful not to include directories in this +# Committed-once install of the extension + test roles. +# +# test/install/load.sql is the ONE place that installs everything the pgTAP +# suite depends on (the extension and the test roles + grants). pgxntool's +# native test/install feature runs it COMMITTED, before the suite, in its own +# pg_regress session; the state persists into every (rolled-back) test. So the +# per-test files no longer each reinstall -- a real time saver. We enable it in +# BOTH modes (install always happens via test/install), so it must be `yes` +# unconditionally here. +PGXNTOOL_ENABLE_TEST_INSTALL = yes +# +# TEST_LOAD_SOURCE selects how load.sql installs the extension: +# - fresh (default): CREATE EXTENSION cat_tools (current version). +# - update: CREATE EXTENSION at TEST_UPDATE_FROM (default 0.2.2, the +# backward-compat floor) then ALTER EXTENSION UPDATE -- to TEST_UPDATE_TO if +# set, otherwise to the current version. Running the SAME suite with the SAME +# expected output against the updated database verifies it behaves +# identically to a fresh install. +# - existing: the extension is ALREADY installed in the target database (by a +# binary pg_upgrade, or an ALTER EXTENSION UPDATE done outside the suite). +# load.sql does not touch it; it only asserts presence + current version and +# creates the test roles. Pair with CONTRIB_TESTDB= and +# EXTRA_REGRESS_OPTS=--use-existing so pg_regress runs against that database +# instead of dropping and recreating a throwaway one. +# +# The mode (and the update from/to versions) are signalled to load.sql by +# placeholder GUCs. pg_regress does not forward make variables, but the psql +# processes it spawns inherit the environment, so PGOPTIONS reaches load.sql. +# +# The GUCs are exported UNCONDITIONALLY, so load.sql can read them WITHOUT +# missing_ok and fail loudly if they did not propagate. Relying on an absent GUC +# to mean "fresh" is unsafe: a silent break anywhere in the +# make -> PGOPTIONS -> env -> psql chain would quietly run the wrong mode. +# +# TEST_LOAD_SOURCE must be exactly `fresh`, `update` or `existing`; anything else +# is a hard error at parse time (so e.g. `make test TEST_LOAD_SOURCE=typo` fails +# fast rather than defaulting). +# +# The install runs COMMITTED (in test/install, not in deps.sql's per-test +# transaction) so it happens once, before the suite, and persists into every +# rolled-back test. See test/install/load.sql. +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 + +# update-mode version range (read by load.sql only in update mode). Empty +# TEST_UPDATE_TO means "update to the current default_version". +TEST_UPDATE_FROM ?= 0.2.2 +TEST_UPDATE_TO ?= + +export PGOPTIONS := $(PGOPTIONS) -c cat_tools.test_load_mode=$(TEST_LOAD_SOURCE) -c cat_tools.test_update_from=$(TEST_UPDATE_FROM) -c cat_tools.test_update_to=$(TEST_UPDATE_TO) + +# 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 + # Versioned SQL is generated from .sql.in at build time. That generation and the # DATA list that installs it live in sql.mk, which also owns # `include pgxntool/base.mk` (base.mk has no include guard, so it must be @@ -7,45 +67,15 @@ testdeps: $(wildcard test/*.sql test/helpers/*.sql) # Be careful not to include # GNU Make two-phase (parse vs. recipe) hazards involved (e.g. # https://github.com/Postgres-Extensions/cat_tools/issues/28) and relies on # base.mk/control.mk/PGXS vars (EXTENSION_VERSION_FILES, PG_CONFIG, MAJORVER, -# datadir, ...). +# datadir, ...). The PGXNTOOL_ENABLE_TEST_INSTALL / TEST_LOAD_SOURCE vars above +# are set before this include so base.mk (pulled in by sql.mk) sees them. include sql.mk -# Support for upgrade test -# -# TODO: Instead of all of this stuff figure out how to pass something to -# pg_regress that will alter the behavior of the test instead. -TEST_BUILD_DIR = test/.build -testdeps: $(TEST_BUILD_DIR)/dep.mk $(TEST_BUILD_DIR)/active.sql --include $(TEST_BUILD_DIR)/dep.mk - -# Ensure dep.mk exists. -$(TEST_BUILD_DIR)/dep.mk: $(TEST_BUILD_DIR) - echo 'TEST_LOAD_SOURCE = new' > $(TEST_BUILD_DIR)/dep.mk - -.PHONY: set-test-new -set-test-new: $(TEST_BUILD_DIR) - echo 'TEST_LOAD_SOURCE = new' > $(TEST_BUILD_DIR)/dep.mk - -.PHONY: test-upgrade -set-test-upgrade: $(TEST_BUILD_DIR) - echo 'TEST_LOAD_SOURCE = upgrade' > $(TEST_BUILD_DIR)/dep.mk - - -$(TEST_BUILD_DIR)/active.sql: $(TEST_BUILD_DIR)/dep.mk $(TEST_BUILD_DIR)/$(TEST_LOAD_SOURCE).sql - ln -sf $(TEST_LOAD_SOURCE).sql $@ - -$(TEST_BUILD_DIR)/upgrade.sql: test/load_upgrade.sql $(TEST_BUILD_DIR) old_version - (echo @generated@ && cat $< && echo @generated@) | sed -e 's#@generated@#-- GENERATED FILE! DO NOT EDIT! See $<#' > $@.tmp - mv $@.tmp $@ - -$(TEST_BUILD_DIR)/new.sql: test/load_new.sql $(TEST_BUILD_DIR) - (echo @generated@ && cat $< && echo @generated@) | sed -e 's#@generated@#-- GENERATED FILE! DO NOT EDIT! See $<#' > $@.tmp - mv $@.tmp $@ - -# TODO: figure out vpath -EXTRA_CLEAN += $(TEST_BUILD_DIR)/ -$(TEST_BUILD_DIR): - [ -d $@ ] || mkdir -p $@ +# Clean the cruft pg_regress writes into test/install/ (the self-comparing +# result .out and its diff), which is listed in test/install/.gitignore. This is +# the pgxntool test/install feature configured above (not SQL generation), so +# its cleanup stays here rather than in sql.mk. +EXTRA_CLEAN += $(addprefix test/install/,$(shell grep -v '^\#' test/install/.gitignore 2>/dev/null)) .PHONY: old_version old_version: $(DESTDIR)$(datadir)/extension/cat_tools--0.2.0.sql diff --git a/bin/test_existing b/bin/test_existing new file mode 100755 index 0000000..42f3862 --- /dev/null +++ b/bin/test_existing @@ -0,0 +1,260 @@ +#!/usr/bin/env bash +# +# Exercise the cat_tools test suite against a REAL database whose extension was +# installed/updated/pg_upgraded OUTSIDE the suite ("existing" mode). Both the +# pg-upgrade legs and the extension-update scenarios in .github/workflows/ci.yml +# repeat the same sequence: +# +# install -> plant dependency guard -> update/upgrade -> assert version +# -> run the suite in existing mode +# +# so it lives here once instead of being duplicated as inline YAML. It is NOT +# CI-only: a developer can run any subcommand locally against a scratch database. +# +# USAGE: bin/test_existing [args] +# +# plant-guard DB +# Plant + prove the dependency guard (extension must already be >= 0.2.2). +# +# update DB [TO_VERSION] +# ALTER EXTENSION cat_tools UPDATE [TO 'TO_VERSION'] (empty => current). +# +# prepare-old DB INSTALL_VERSION [BRIDGE_TO] +# Old-cluster prep for pg-upgrade-test: create DB + extension at +# INSTALL_VERSION, optionally bridge-update to BRIDGE_TO, then plant guard. +# +# run-suite DB +# Run the suite in existing mode (extension must be at the current version). +# +# update-scenario DB FROM_VERSION +# Create DB + extension at FROM_VERSION, plant guard, update to current, +# run the suite against that real updated database. +# +# update-check DB FROM_VERSION TO_VERSION +# Lightweight check that a specific update script applies (no suite). +# +# Run `bin/test_existing` with no subcommand to print usage. +# +# Why the dependency guard: "existing" mode must run the suite against the ACTUAL +# upgraded/updated objects. If anything silently dropped + reinstalled the +# extension, the suite would test a FRESH install and hide a regression. As +# belt-and-suspenders to the load.sql guarantee (existing mode never drops the +# extension), we plant an object that HARD-references a cat_tools member so a +# non-CASCADE DROP EXTENSION fails, and we actively PROVE that here (see +# plant_guard): if the drop unexpectedly succeeds, this script fails CI. +# +# See https://github.com/Postgres-Extensions/cat_tools/pull/16 for context. +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 (no dependency on being inside a git checkout). readlink -f +# resolves any path the script was invoked through. +cd "$(dirname "$(readlink -f "$0")")/.." + +# A view whose output column has a cat_tools enum type creates a pg_depend edge +# to that extension member, so a non-CASCADE DROP EXTENSION cannot succeed. The +# enum is only ever extended (ALTER TYPE ... ADD VALUE) by the update scripts, +# never dropped, so the guard survives 0.2.2 -> current updates and binary +# pg_upgrade. +GUARD_SCHEMA=cat_tools_drop_guard +GUARD_VIEW=guard + +# --------------------------------------------------------------------------- +# psql helpers +# --------------------------------------------------------------------------- + +# Capture the single-value (-tAc) output of a query against a database. Used for +# the many "read one value back out" calls below so the psql flags live in one +# place. +psql_value() { + local db=$1 sql=$2 + psql -d "$db" -tAc "$sql" +} + +# Run SQL that must succeed, aborting the whole script on any error +# (ON_ERROR_STOP). Extra args pass through to psql, so callers use either +# `psql_do DB -c '...'` or a heredoc (`psql_do DB </dev/null | sed -n 's/.*set to "\(.*\)"$/\1/p' +} + +installed_version() { + psql_value "$1" \ + "SELECT extversion FROM pg_extension WHERE extname = 'cat_tools'" +} + +guard_present() { + test "$(psql_value "$1" \ + "SELECT count(*) FROM pg_views WHERE schemaname = '$GUARD_SCHEMA' AND viewname = '$GUARD_VIEW'")" = 1 +} + +# 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 + psql_do "$db" </dev/null 2>&1; then + echo "FAIL: DROP EXTENSION cat_tools (non-CASCADE) unexpectedly SUCCEEDED in '$db' -- dependency guard is ineffective" >&2 + exit 1 + fi + guard_present "$db" \ + || { echo "FAIL: dependency guard missing from '$db' after the drop attempt" >&2; exit 1; } + test -n "$(installed_version "$db")" \ + || { echo "FAIL: cat_tools extension missing from '$db' after the drop attempt" >&2; exit 1; } + echo "OK: non-CASCADE DROP EXTENSION is blocked in '$db' (dependency guard effective)" +} + +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: cat_tools 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 the script. + if [ -n "$to" ]; then to="TO '$to'"; fi + psql_do "$db" -c "ALTER EXTENSION cat_tools UPDATE $to" +} + +# --------------------------------------------------------------------------- +# Subcommand implementations +# --------------------------------------------------------------------------- + +# prepare-old DB INSTALL_VERSION [BRIDGE_TO] +# Old-cluster preparation for pg-upgrade-test. Create the database and the +# extension at INSTALL_VERSION; if BRIDGE_TO is given, ALTER EXTENSION UPDATE +# TO it first. That "bridge" models the real migration path a user on an OLD +# PostgreSQL + OLD cat_tools must take: e.g. on PG10, install 0.2.0 then update +# to 0.2.3 BEFORE pg_upgrade. The shipped 0.2.0->0.2.2 / 0.2.1->0.2.2 scripts do +# NOT fix the views (their omit_column used the no-op `!= ANY`, leaving +# relhasoids/relhaspkey in _cat_tools.pg_class_v); the pg_class_v DROP+CREATE +# rebuild that strips those columns lives in the 0.2.2->0.2.3 update, so the +# bridge must reach 0.2.3 (the current version). The raw 0.2.0 views reference +# catalog columns removed in newer PostgreSQL and would break binary pg_upgrade +# otherwise. Then plant + prove the dependency guard (at the bridged version, +# so cat_tools.relation_type exists). +prepare_old() { + local db=$1 install=$2 bridge=${3:-} + createdb "$db" + psql_do "$db" -c "CREATE EXTENSION cat_tools VERSION '$install'" + # Use `if`, not `&&`: under `set -e` a false `[ -n ... ] && ...` would abort. + if [ -n "$bridge" ]; then update_ext "$db" "$bridge"; fi + plant_guard "$db" +} + +# update-scenario DB FROM_VERSION +# Full extension-update flow that runs the existing-mode suite: create the DB +# and extension at FROM_VERSION, plant + prove the guard, update to the current +# version, then run the suite against that real updated database. +update_scenario() { + local db=$1 from=$2 + createdb "$db" + psql_do "$db" -c "CREATE EXTENSION cat_tools VERSION '$from'" + plant_guard "$db" + update_ext "$db" + run_suite "$db" +} + +# update-check DB FROM_VERSION TO_VERSION +# Lightweight check that a specific update script applies (no suite): used for +# the pre-0.2.2 scripts, which only install on PG10 and target 0.2.2 (not the +# current version, so the suite cannot run against them). +update_check() { + local db=$1 from=$2 to=$3 + createdb "$db" + psql_do "$db" -c "CREATE EXTENSION cat_tools VERSION '$from'" + update_ext "$db" "$to" + assert_version "$db" "$to" +} + +# Run the pgTAP suite against an already-populated database in existing mode. +# Verifies the extension is at the current version, re-proves the guard still +# blocks a drop (i.e. it survived the update/upgrade), runs the suite via +# --use-existing so pg_regress does NOT drop/recreate the database, then confirms +# the guard is still present (a CASCADE drop+reinstall would have removed it). +run_suite() { + local db=$1 + assert_version "$db" current + assert_drop_blocked "$db" + # In existing mode pg_regress runs against $db via --use-existing and must NOT + # create/drop its own database. Two consequences drive the make args below: + # 1. PGXNTOOL_ENABLE_TEST_BUILD=no: base.mk auto-enables the test-build sanity + # check whenever test/build/*.sql exist, adding it as a `test` prerequisite. + # test-build spawns a recursive `installcheck` that INHERITS this call's + # --use-existing (a command-line var propagates to sub-makes) but targets a + # fresh `regression` DB it cannot create under --use-existing, so it dies + # with "database regression does not exist". test-build is a fresh-install + # check already run by the fresh `test` job on every PG version, so it adds + # nothing here -- disable it. + # 2. verify-results depends on `test`, so it re-runs the suite; it must carry + # the SAME existing-mode overrides or it would re-run FRESH (against a new + # regression DB) instead of verifying THIS existing database. + local existing_args="TEST_LOAD_SOURCE=existing CONTRIB_TESTDB=$db EXTRA_REGRESS_OPTS=--use-existing PGXNTOOL_ENABLE_TEST_BUILD=no" + make test $existing_args + make verify-results $existing_args + # Post-suite guard assertion: a CASCADE drop+reinstall during the run would + # have removed the guard view, meaning the suite tested a fresh install, not $db. + guard_present "$db" \ + || { echo "FAIL: dependency guard vanished during the suite run on '$db' -- extension was dropped+reinstalled (CASCADE)?" >&2; exit 1; } +} + +usage() { + echo "usage: bin/test_existing [args]" >&2 + echo " plant-guard DB" >&2 + echo " update DB [TO_VERSION]" >&2 + echo " prepare-old DB INSTALL_VERSION [BRIDGE_TO]" >&2 + echo " run-suite DB" >&2 + echo " update-scenario DB FROM_VERSION" >&2 + echo " update-check DB FROM_VERSION TO_VERSION" >&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 + plant-guard) plant_guard "$@" ;; + update) update_ext "$@" ;; + prepare-old) prepare_old "$@" ;; + run-suite) run_suite "$@" ;; + update-scenario) update_scenario "$@" ;; + update-check) update_check "$@" ;; + *) usage ;; + esac +} + +main "$@" diff --git a/test/build/build.sql b/test/build/build.sql new file mode 100644 index 0000000..ef6210a --- /dev/null +++ b/test/build/build.sql @@ -0,0 +1,21 @@ +\set ECHO none + +\i test/pgxntool/psql.sql +\t + +BEGIN; +CREATE SCHEMA cat_tools; + +/* + * The install script no longer sets client_min_messages itself (that is the + * caller's job; deps.sql does the same for the main suite). Suppress NOTICEs + * here so build.out does not capture verbose, version-specific messages (e.g. + * "%TYPE converted to regclass" with a source-file LOCATION line). + */ +SET client_min_messages = WARNING; + +\i sql/cat_tools.sql + +\echo # TRANSACTION INTENTIONALLY LEFT OPEN! + +-- vi: expandtab sw=2 ts=2 diff --git a/test/expected/zzz_build.out b/test/build/expected/build.out similarity index 98% rename from test/expected/zzz_build.out rename to test/build/expected/build.out index dbe9615..8710271 100644 --- a/test/expected/zzz_build.out +++ b/test/build/expected/build.out @@ -100,6 +100,8 @@ + + diff --git a/test/deps.sql b/test/deps.sql index f7fe4d9..09a7356 100644 --- a/test/deps.sql +++ b/test/deps.sql @@ -1,20 +1,17 @@ -- IF NOT EXISTS will emit NOTICEs, which is annoying SET client_min_messages = WARNING; --- Add any test dependency statements here --- Note: pgTap is loaded by setup.sql ---CREATE EXTENSION IF NOT EXISTS ...; - -\i test/.build/active.sql - --- Used by several unit tests +/* + * The extension and the test roles are installed ONCE, COMMITTED, before the + * suite by test/install/load.sql (pgxntool's test/install feature), instead of + * per-test here. So this file no longer runs CREATE EXTENSION or CREATE ROLE: + * all it does is (re)set the psql variables the suite references (setup.sql and + * the test/sql/ files). psql variables are session-local, not committed DB + * state, so they must still be (re)set per test file. + * + * The role names are also set by test/install/load.sql (which creates the + * roles); keep the two in sync. Add any OTHER per-test dependency + * \set/statements here; committed dependencies belong in test/install/load.sql. + */ \set no_use_role cat_tools_testing__no_use_role \set use_role cat_tools_testing__use_role -CREATE ROLE :no_use_role; -CREATE ROLE :use_role; - -GRANT cat_tools__usage TO :use_role; --- PG15+ removed CREATE on public schema from PUBLIC; grant it explicitly for tests --- that need to create shadow names in public to test catalog lookup correctness. -GRANT CREATE ON SCHEMA public TO :use_role; - diff --git a/test/install/.gitignore b/test/install/.gitignore new file mode 100644 index 0000000..eb02f57 --- /dev/null +++ b/test/install/.gitignore @@ -0,0 +1,6 @@ +# pg_regress writes the install step's result (and any diff) here, because the +# install schedule references tests as ../install/. The install output is +# self-comparing (pg_regress resolves both the expected and result paths to this +# directory), so it is never asserted and must not be tracked. +load.out +install.out.diff diff --git a/test/install/load.sql b/test/install/load.sql new file mode 100644 index 0000000..38d3ccd --- /dev/null +++ b/test/install/load.sql @@ -0,0 +1,197 @@ +/* + * Single, committed-once installer for the test suite's dependencies: the + * cat_tools extension (see the modes below) and the test roles + grants. + * + * pgxntool's test/install feature runs this file COMMITTED, in its own + * pg_regress session, BEFORE the main pgTAP suite. Because its state is + * committed it persists into every test and runs ONCE instead of per-test + * (pgTAP rolls back each test/sql/ file, so tests read these objects but never + * modify them). Committing also mirrors a real production update: ALTER + * EXTENSION UPDATE commits, then later transactions use the updated objects. + * deps.sql (run per-test) installs nothing; it only sets the psql variables the + * suite references. + * + * Three modes, selected by the cat_tools.test_load_mode placeholder GUC, which + * the Makefile TEST_LOAD_SOURCE block sets via PGOPTIONS (fresh is the default): + * - fresh (default): plain CREATE EXTENSION cat_tools (current version). + * - update: CREATE EXTENSION at an older version (cat_tools.test_update_from, + * default 0.2.2) then ALTER EXTENSION UPDATE -- to cat_tools.test_update_to + * when that GUC is non-empty, otherwise to the current default_version. + * Reusing the SAME suite and expected output asserts an updated database + * behaves identically to a fresh install. + * - existing: the extension is ALREADY installed (by binary pg_upgrade, or an + * ALTER EXTENSION UPDATE performed outside the suite). load.sql must NOT + * drop/create/update it -- that would destroy exactly what the suite + * validates. It only asserts presence + current version, then creates the + * test roles. + * + * Version floor: + * - 0.2.2 is the default update-from floor: it is the OLDEST cat_tools + * version that installs cleanly on PG11+ (the 0.2.0/0.2.1 install scripts + * fail on PG11+/PG12+), so it is the WIDEST update path this harness can + * exercise across the supported PostgreSQL range -- not a claim that we + * only care about 0.2.2. + */ +SET client_min_messages = WARNING; + +/* + * The test-role names, set here and mirrored per-test by test/deps.sql (psql + * variables are session-local, so both this committed installer and each + * rolled-back test file must set them). Each :var holds the identifier; the + * pg_temp helper functions below receive it as a text literal (:'use_role') and + * quote it via format(%I). + */ +\set no_use_role cat_tools_testing__no_use_role +\set use_role cat_tools_testing__use_role + +/* + * Mode selection. The Makefile always exports cat_tools.test_load_mode via + * PGOPTIONS. Read it WITHOUT missing_ok: if the GUC did not propagate (a break + * anywhere in make -> PGOPTIONS -> env -> psql), current_setting errors here and + * the whole install step fails loudly, instead of silently falling back to a + * default and running the wrong suite. The DO block then rejects any value + * other than fresh/update/existing with a clear message. + */ +SELECT current_setting('cat_tools.test_load_mode') AS cat_tools_test_load_mode +\gset + +DO $DO$ +BEGIN + IF current_setting('cat_tools.test_load_mode') NOT IN ('fresh', 'update', 'existing') THEN + RAISE EXCEPTION + 'cat_tools.test_load_mode must be ''fresh'', ''update'' or ''existing'', got ''%''' + , current_setting('cat_tools.test_load_mode') + ; + END IF; +END +$DO$; + +SELECT + :'cat_tools_test_load_mode' = 'update' AS cat_tools_mode_update + , :'cat_tools_test_load_mode' = 'existing' AS cat_tools_mode_existing +\gset + +\if :cat_tools_mode_existing +/* + * existing mode: do NOT touch the extension. Assert it is installed and at the + * current default_version -- the pg_upgrade / external update the database just + * went through is exactly what the suite is validating, so dropping or + * reinstalling it would defeat the test. Fail loudly on absence or mismatch. + * (CI additionally plants a dependency guard so a stray non-CASCADE drop would + * error rather than silently reinstall; see bin/test_existing.) + */ +DO $DO$ +DECLARE + v_installed text := (SELECT extversion FROM pg_extension WHERE extname = 'cat_tools'); + v_default text := (SELECT default_version FROM pg_available_extensions WHERE name = 'cat_tools'); +BEGIN + IF v_installed IS NULL THEN + RAISE EXCEPTION 'test_load_mode=existing but the cat_tools extension is not installed'; + END IF; + IF v_installed IS DISTINCT FROM v_default THEN + RAISE EXCEPTION + 'cat_tools is installed at version % but the current default_version is %' + , v_installed, v_default + ; + END IF; +END +$DO$; +\else +/* + * fresh / update: (re)install from scratch. Drop-first so a re-run on a + * persistent cluster installs the newest build instead of reusing stale + * objects. Drop the extension, then the roles. + * + * DROP EXTENSION does not remove cat_tools__usage: the extension scripts create + * it with CREATE ROLE, and roles are global objects, not extension members, so + * they survive DROP EXTENSION. The 0.2.2 install script uses a bare CREATE ROLE + * (unlike the current version's duplicate-tolerant DO block), so a leftover role + * would break a re-run in update mode. Drop all three roles explicitly via + * pg_temp.drop_role(): DROP OWNED BY first strips privileges granted TO the role + * (e.g. CREATE on public, cat_tools__usage membership) so DROP ROLE cannot fail + * with a dependency error; the pg_roles guard skips a not-yet-existing role + * (DROP OWNED BY errors on one), and format(%I) quotes the name correctly. + */ +DROP EXTENSION IF EXISTS cat_tools CASCADE; + +CREATE FUNCTION pg_temp.drop_role( + role_name text +) RETURNS void LANGUAGE plpgsql AS $$ +BEGIN + IF EXISTS (SELECT 1 FROM pg_roles WHERE rolname = role_name) THEN + EXECUTE format('DROP OWNED BY %I', role_name); + EXECUTE format('DROP ROLE IF EXISTS %I', role_name); + END IF; +END +$$; + +SELECT pg_temp.drop_role(:'use_role'); +SELECT pg_temp.drop_role(:'no_use_role'); +SELECT pg_temp.drop_role('cat_tools__usage'); + +\if :cat_tools_mode_update +/* + * update mode: install an older version, then ALTER EXTENSION UPDATE. The + * from/to versions come from the Makefile (TEST_UPDATE_FROM / TEST_UPDATE_TO, + * exported as GUCs). An empty test_update_to means "update to the current + * default_version" (the widest path); a non-empty value targets a specific + * version (e.g. the explicit 0.2.1 -> 0.2.2 script). + */ +SELECT current_setting('cat_tools.test_update_from') AS cat_tools_test_update_from \gset +SELECT current_setting('cat_tools.test_update_to') AS cat_tools_test_update_to \gset +/* + * Build the optional target clause once so a SINGLE ALTER EXTENSION covers both + * cases: an empty test_update_to yields '' (update to the current + * default_version -- the widest path); a non-empty value yields "TO ''". + * format(%L) quotes the version literal safely; the bare :clause interpolation + * below then drops it in verbatim. + */ +SELECT CASE WHEN :'cat_tools_test_update_to' = '' THEN '' + ELSE format('TO %L', :'cat_tools_test_update_to') END + AS cat_tools_update_to_clause \gset + +CREATE EXTENSION cat_tools VERSION :'cat_tools_test_update_from'; +/* + * Suppress the deprecation NOTICEs the update scripts emit, matching the + * approach used by test/build/upgrade.sql. + */ +SET client_min_messages = ERROR; +ALTER EXTENSION cat_tools UPDATE :cat_tools_update_to_clause; +SET client_min_messages = WARNING; +\else +CREATE EXTENSION cat_tools; +\endif +-- end \if :cat_tools_mode_update (fresh vs. update install branch) +\endif +-- end \if :cat_tools_mode_existing (existing mode skips the whole (re)install block) + +/* + * Roles and grants the test suite depends on. Formerly created per-test in + * deps.sql; now committed here once. Created idempotently because the source + * differs by mode: fresh/update dropped them just above, while a freshly + * pg_upgraded database (existing mode) never had them (roles are global, and + * only the extension -- not the test roles -- is created before the pg_upgrade). + * GRANT is idempotent, so re-applying the grants is always safe. + */ +CREATE FUNCTION pg_temp.create_role( + role_name text +) RETURNS void LANGUAGE plpgsql AS $$ +BEGIN + IF NOT EXISTS (SELECT 1 FROM pg_roles WHERE rolname = role_name) THEN + EXECUTE format('CREATE ROLE %I', role_name); + END IF; +END +$$; + +SELECT pg_temp.create_role(:'no_use_role'); +SELECT pg_temp.create_role(:'use_role'); + +GRANT cat_tools__usage TO :use_role; +/* + * PG15+ removed CREATE on the public schema from PUBLIC; grant it explicitly + * for tests that create shadow names in public to check catalog-lookup + * correctness. + */ +GRANT CREATE ON SCHEMA public TO :use_role; + +-- vi: expandtab ts=2 sw=2 diff --git a/test/load_new.sql b/test/load_new.sql deleted file mode 100644 index d87ae34..0000000 --- a/test/load_new.sql +++ /dev/null @@ -1,6 +0,0 @@ -/* - * Now load our extension. We don't use IF NOT EXISTs here because we want an - * error if the extension is already loaded (because we want to ensure we're - * getting the very latest version). - */ -CREATE EXTENSION cat_tools; diff --git a/test/load_upgrade.sql b/test/load_upgrade.sql deleted file mode 100644 index 5507205..0000000 --- a/test/load_upgrade.sql +++ /dev/null @@ -1,4 +0,0 @@ --- NOTE: To set this prior to 0.2.0 you need to hack the installed extension file to not create the cat_tools schema, or hack the current control file to not specify it! -CREATE EXTENSION cat_tools VERSION '0.2.0'; -ALTER EXTENSION cat_tools UPDATE; - diff --git a/test/sql/zzz_build.sql b/test/sql/zzz_build.sql deleted file mode 100644 index e9a7f60..0000000 --- a/test/sql/zzz_build.sql +++ /dev/null @@ -1,13 +0,0 @@ -\set ECHO none - -\i test/pgxntool/psql.sql -\t - -BEGIN; -CREATE SCHEMA cat_tools; - -\i sql/cat_tools.sql - -\echo # TRANSACTION INTENTIONALLY LEFT OPEN! - --- vi: expandtab sw=2 ts=2