From cc75a04206d56a118cb320469d17fc3ef985e8df Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Thu, 6 Aug 2026 17:41:08 -0500 Subject: [PATCH 1/5] Migrate zzz_build.sql to pgxntool's native test/build feature zzz_build.sql hand-rolled exactly what pgxntool's test/build/ already does natively: run the raw install script directly (not via CREATE EXTENSION) for better error messages than a bare CREATE EXTENSION failure. Postgres-Extensions/cat_tools already made this exact move. Also fixes the flaky expected-output problem that came with the old approach: zzz_build.sql ran client_min_messages unsuppressed, so its expected output captured verbose, PG-minor-version-dependent NOTICEs (e.g. "%TYPE converted to ..." with a source-file LOCATION line) -- any environment/PG-version drift showed as a spurious diff. test/build runs as its own separate installcheck invocation with client_min_messages = WARNING (matching cat_tools's own build.sql), so the expected output is stable and empty. PGXNTOOL_ENABLE_TEST_BUILD set explicitly (yes) rather than left to auto-detect, so an accidental future deletion of test/build/'s contents is a hard error instead of the check silently vanishing. --- Makefile | 5 +++++ test/build/build.sql | 20 +++++++++++++++++++ .../expected/build.out} | 2 -- test/sql/zzz_build.sql | 12 ----------- 4 files changed, 25 insertions(+), 14 deletions(-) create mode 100644 test/build/build.sql rename test/{expected/zzz_build.out => build/expected/build.out} (96%) delete mode 100644 test/sql/zzz_build.sql diff --git a/Makefile b/Makefile index 07a0f45..cce47d3 100644 --- a/Makefile +++ b/Makefile @@ -1,5 +1,10 @@ include pgxntool/base.mk +# Explicit rather than relying on auto-detect (which enables this whenever +# test/build/*.sql exists) so an accidental deletion of test/build/'s +# contents is a hard error instead of the check silently disappearing. +PGXNTOOL_ENABLE_TEST_BUILD = yes + testdeps: test_extension test_extension: $(DESTDIR)$datadir)/extension/extension_drop_test.control $(wildcard $(TESTDIR)/*) $(DESTDIR)$datadir)/extension/extension_drop_test.control: diff --git a/test/build/build.sql b/test/build/build.sql new file mode 100644 index 0000000..ac3fbb6 --- /dev/null +++ b/test/build/build.sql @@ -0,0 +1,20 @@ +\set ECHO none +BEGIN; +\i test/pgxntool/psql.sql + +CREATE EXTENSION IF NOT EXISTS cat_tools; + +/* + * Suppress NOTICEs from the raw install script itself (e.g. "%TYPE converted + * to ..." with a version-specific source-file LOCATION line) so this file's + * expected output stays stable across PostgreSQL minor versions instead of + * capturing verbose, version-dependent messages. + */ +SET client_min_messages = WARNING; + +\echo +\echo INSTALL +\t +\i sql/extension_drop.sql + +\echo # TRANSACTION INTENTIONALLY LEFT OPEN diff --git a/test/expected/zzz_build.out b/test/build/expected/build.out similarity index 96% rename from test/expected/zzz_build.out rename to test/build/expected/build.out index c66ee3c..44215eb 100644 --- a/test/expected/zzz_build.out +++ b/test/build/expected/build.out @@ -19,6 +19,4 @@ INSTALL - - # TRANSACTION INTENTIONALLY LEFT OPEN diff --git a/test/sql/zzz_build.sql b/test/sql/zzz_build.sql deleted file mode 100644 index 354c480..0000000 --- a/test/sql/zzz_build.sql +++ /dev/null @@ -1,12 +0,0 @@ -\set ECHO none -BEGIN; -\i test/pgxntool/psql.sql - -CREATE EXTENSION IF NOT EXISTS cat_tools; - -\echo -\echo INSTALL -\t -\i sql/extension_drop.sql - -\echo # TRANSACTION INTENTIONALLY LEFT OPEN From 7f5a78c60906b13c349cd45f8e5c2f649608271a Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Tue, 4 Aug 2026 15:15:23 -0500 Subject: [PATCH 2/5] Add test/install foundation: TEST_LOAD_SOURCE modes, dependency guard, quoting-requiring schema test Builds the U&U (update & upgrade) test infrastructure that doesn't require a real second extension_drop version or pg_upgrade CI to already exist: - PGXNTOOL_ENABLE_TEST_INSTALL = yes, with test/install/load.sql as the committed-once installer for the extension (no test roles exist for this extension, so unlike cat_tools there's nothing role-related to add). - TEST_LOAD_SOURCE (fresh/update/existing) GUC/make-var switch, parse-time validated, exported unconditionally, read in load.sql without missing_ok. `existing` mode is fully exercised locally (verified against a real, already-installed database, including the failure path when the extension is genuinely absent). `update` mode is wired up and structurally verified end-to-end, but extension_drop has no real prior released version to update FROM yet -- the Makefile refuses to run it without TEST_UPDATE_FROM set explicitly, and no CI leg exercises it in this repo today. - Dependency guard (test/sql/dependency_guard.sql): a view depending on extension_drop__commands' row type blocks a non-CASCADE DROP EXTENSION; proven by actually attempting the drop and asserting failure, not assumed. - test/sql/schema.sql's custom-schema test names renamed to mixed case (requires identifier quoting), reusing its existing coverage rather than adding a new schema-testing dimension. - ci.yml: run `make test && make verify-results` instead of pg-build-test, so a real regression actually fails the build (pgxntool's .IGNORE: installcheck otherwise reports green regardless of test results, per RELEASE.md's existing note about PRs #6/#7). Moving the extension's own installation into test/install/load.sql required adapting every test file that used to install it per-test in a rolled-back transaction (test/deps.sql, test/sql/simple.sql, test/sql/schema.sql, test/sql/zzz_build.sql) to work against the new committed-once install instead, since an extension name is a database-wide singleton. Co-Authored-By: Claude Sonnet 5 --- .github/workflows/ci.yml | 10 +- Makefile | 59 +++++++++++ test/deps.sql | 32 ++---- test/expected/dependency_guard.out | 6 ++ test/expected/schema.out | 10 +- test/install/.gitignore | 14 +++ test/install/load.sql | 162 +++++++++++++++++++++++++++++ test/sql/dependency_guard.sql | 66 ++++++++++++ test/sql/schema.sql | 32 +++++- test/sql/simple.sql | 18 ++-- 10 files changed, 368 insertions(+), 41 deletions(-) create mode 100644 test/expected/dependency_guard.out create mode 100644 test/install/.gitignore create mode 100644 test/install/load.sql create mode 100644 test/sql/dependency_guard.sql diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index dc82d51..354b400 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -60,7 +60,15 @@ jobs: - name: Check out the repo uses: actions/checkout@v7 - name: Test on PostgreSQL ${{ matrix.pg }} - run: pg-build-test + # pgxntool's base.mk marks installcheck .IGNORE, so a plain `make + # test` (what pg-build-test itself invokes under the hood) exits 0 + # even when every pg_regress test fails -- confirmed happening for + # real on PRs #6/#7 (RELEASE.md). `make verify-results` is the + # actual pass/fail signal (it scans for raw pgTAP failures and plan + # mismatches, not just installcheck's own exit code), so run it + # explicitly after `make test` instead of relying on pg-build-test + # alone. + run: make test && make verify-results # A single stable check name for use as a required status check in branch # protection. Matrix jobs produce names like "🐘 PostgreSQL 14" that change diff --git a/Makefile b/Makefile index cce47d3..960c018 100644 --- a/Makefile +++ b/Makefile @@ -1,3 +1,62 @@ +# Run test/install/load.sql (extension install) COMMITTED, once, before the +# main pgTAP suite, via pgxntool's test/install feature. Set explicitly +# (rather than left to auto-detect) so an accidentally emptied test/install/ +# is a hard build error instead of silently falling back to "disabled". +# Must be set before `include pgxntool/base.mk` below -- base.mk reads it +# while parsing. +PGXNTOOL_ENABLE_TEST_INSTALL = yes + +# TEST_LOAD_SOURCE selects how test/install/load.sql installs extension_drop: +# - fresh (default): CREATE EXTENSION extension_drop (current version). +# - update: CREATE EXTENSION at TEST_UPDATE_FROM, then ALTER EXTENSION +# UPDATE -- to TEST_UPDATE_TO if set, otherwise to the current version. +# Running the SAME suite/expected output against the result asserts +# update behaves identically to a fresh install. NOTE: extension_drop has +# never had a real second released version (PGXN's only listing is +# 0.1.x from 2017, predating the current SQL entirely -- see HISTORY.asc +# and RELEASE.md), so TEST_UPDATE_FROM has no safe default; this mode is +# wired up and structurally ready, but there is nothing real to update +# FROM yet, and so no CI leg exercises it in this repo today. +# - existing: the extension is ALREADY installed (a real pg_upgrade, or an +# ALTER EXTENSION UPDATE done outside the suite). load.sql does not +# touch it; it only asserts presence + current version. Pair with +# CONTRIB_TESTDB= and EXTRA_REGRESS_OPTS=--use-existing to point +# pg_regress at that database instead of a throwaway one. +# +# Propagated to load.sql as a GUC: pg_regress doesn't forward make variables, +# but the psql processes it spawns inherit the environment, so PGOPTIONS +# reaches load.sql. Exported UNCONDITIONALLY so load.sql can read it without +# missing_ok and fail loudly if it didn't propagate, rather than silently +# defaulting to the wrong mode. The mode is also validated here at +# make-parse-time, so a typo like `TEST_LOAD_SOURCE=fresh ` or +# `TEST_LOAD_SOURCE=typo` fails immediately instead of quietly running the +# default. +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 (load.sql only reads these in update mode). +# Empty TEST_UPDATE_TO means "update to the current default_version". There +# is no safe default for TEST_UPDATE_FROM (see above) -- require it +# explicitly rather than pointing it at a version that doesn't exist. +TEST_UPDATE_FROM ?= +TEST_UPDATE_TO ?= +ifeq ($(TEST_LOAD_SOURCE),update) + ifeq ($(strip $(TEST_UPDATE_FROM)),) +$(error TEST_UPDATE_FROM must be set when TEST_LOAD_SOURCE=update -- extension_drop has no prior released version yet to default it to) + endif +endif + +export PGOPTIONS := $(PGOPTIONS) -c extension_drop.test_load_mode=$(TEST_LOAD_SOURCE) -c extension_drop.test_update_from=$(TEST_UPDATE_FROM) -c extension_drop.test_update_to=$(TEST_UPDATE_TO) + +# 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 + include pgxntool/base.mk # Explicit rather than relying on auto-detect (which enables this whenever diff --git a/test/deps.sql b/test/deps.sql index bc5e06d..f53733f 100644 --- a/test/deps.sql +++ b/test/deps.sql @@ -1,32 +1,16 @@ --- 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 --- Re-enable notices -SET client_min_messages = NOTICE; +/* + * extension_drop itself used to be (re)installed here, per test file. It's + * now installed ONCE, COMMITTED, by test/install/load.sql (pgxntool's + * test/install feature) before this suite runs at all -- this file no + * longer touches it. test/sql/schema.sql is the one test that actually + * drops/recreates the extension itself (that's what it's testing); every + * other test file just uses the extension load.sql already installed. + */ \set TT extension_drop_test_table CREATE TEMP TABLE :TT (i int); -CREATE SCHEMA :TEST_SCHEMA; -SET search_path = :TEST_SCHEMA, tap, "$user"; - -/* - * 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). - */ -SET client_min_messages = WARNING; -- Squelch notice from CASCADE -DO $$ BEGIN - IF current_setting('server_version_num')::int < 100000 THEN - CREATE EXTENSION IF NOT EXISTS cat_tools; - CREATE EXTENSION extension_drop ; - ELSE - EXECUTE $exec$CREATE EXTENSION extension_drop CASCADE$exec$; - END IF; -END$$; -SET client_min_messages = NOTICE; - -- vi: expandtab ts=2 sw=2 diff --git a/test/expected/dependency_guard.out b/test/expected/dependency_guard.out new file mode 100644 index 0000000..5d22292 --- /dev/null +++ b/test/expected/dependency_guard.out @@ -0,0 +1,6 @@ +\set ECHO none +1..3 +ok 1 - Non-CASCADE DROP EXTENSION extension_drop is blocked by the dependency guard +ok 2 - extension_drop is still installed after the blocked drop attempt +ok 3 - Dependency guard view is still present after the blocked drop attempt +# TRANSACTION INTENTIONALLY LEFT OPEN! diff --git a/test/expected/schema.out b/test/expected/schema.out index 12db609..511254e 100644 --- a/test/expected/schema.out +++ b/test/expected/schema.out @@ -4,12 +4,12 @@ ok 1 - Create test extension ok 2 - Test extension exists ok 3 - Drop test extension ok 4 - Test extension does not exist -ok 5 - Table _test_ed.extension_drop__commands should exist +ok 5 - Table "_Test_Ed".extension_drop__commands should exist ok 6 - Drop extension -ok 7 - Create extension in schema _test_ed_2 -ok 8 - Table _test_ed_2.extension_drop__commands should exist -ok 9 - Create test extension in _test_ed_2 +ok 7 - Create extension in schema _Test_Ed_2 +ok 8 - Table "_Test_Ed_2".extension_drop__commands should exist +ok 9 - Create test extension in _Test_Ed_2 ok 10 - extension_drop__update() ok 11 - Verify extension_drop__get() -ok 12 - Drop schema _test_ed without cascade succeeds +ok 12 - Drop schema _Test_Ed without cascade succeeds # TRANSACTION INTENTIONALLY LEFT OPEN! diff --git a/test/install/.gitignore b/test/install/.gitignore new file mode 100644 index 0000000..5ec5e32 --- /dev/null +++ b/test/install/.gitignore @@ -0,0 +1,14 @@ +# pg_regress writes the install step's result here, because the install +# schedule references tests as ../install/ -- one directory up from +# both test/expected/ and test/results/, which cancels back out to this same +# directory for both. So load.out is simultaneously "expected" and "actual": +# confirmed by hand (deliberately breaking load.sql's existing-mode assertion +# and seeing pg_regress still report the step "ok" while the real error text +# showed up in this file) that pg_regress can never see a diff for it here, +# regardless of what load.sql actually does. Never track it -- it would just +# be reformatted/overwritten noise on every run, not a real expectation. +load.out +# Precautionary: haven't observed pg_regress emit a *.diff for this +# self-comparing path locally, but if it ever does, it'd be equally +# meaningless to track for the same reason as load.out above. +install.out.diff diff --git a/test/install/load.sql b/test/install/load.sql new file mode 100644 index 0000000..b29636e --- /dev/null +++ b/test/install/load.sql @@ -0,0 +1,162 @@ +\set ECHO none +/* + * Committed-once installer for the test suite's one real dependency: the + * extension_drop extension itself. (No test roles exist for this extension + * -- see test/deps.sql -- so unlike cat_tools' equivalent load.sql, there is + * nothing role-related to install here.) + * + * pgxntool's test/install feature runs this file COMMITTED, in its own + * pg_regress session, BEFORE the main pgTAP suite, so the extension persists + * into every (rolled-back) test/sql/ file instead of each one re-installing + * it from scratch. test/deps.sql (run per test) no longer creates the + * extension; it only sets the psql variables the suite references. + * test/sql/schema.sql is the one exception: proving the schema-targeting + * pipeline works is its actual job, so it explicitly drops this committed + * install and recreates its own copies in schemas it chooses -- safely, + * since that all happens inside its own rolled-back transaction and never + * escapes that one file. + * + * Three modes, selected by the extension_drop.test_load_mode placeholder + * GUC, which the Makefile's TEST_LOAD_SOURCE block sets via PGOPTIONS + * (fresh is the default): + * - fresh (default): plain CREATE EXTENSION extension_drop (current + * version). + * - update: CREATE EXTENSION at an older version + * (extension_drop.test_update_from) then ALTER EXTENSION UPDATE -- to + * extension_drop.test_update_to when that GUC is non-empty, otherwise to + * the current default_version. NOTE: extension_drop has never had a + * real second released version -- PGXN's only listing (0.1.x, 2017) + * predates the current SQL entirely (see HISTORY.asc/RELEASE.md), so + * there is no version that could legitimately fill + * extension_drop.test_update_from today. This branch is wired up and + * structurally correct (the Makefile refuses to select this mode + * without TEST_UPDATE_FROM set explicitly), but has nothing real to + * update FROM yet, so it exists ready for the day a second version + * ships rather than because it's exercised in CI now. + * - existing: the extension is ALREADY installed (by a real binary + * pg_upgrade, or an ALTER EXTENSION UPDATE performed outside the + * suite). This branch must NOT drop/create/update it -- that would + * destroy exactly what "existing" mode exists to test. It only asserts + * presence + current version. + * + * Unlike cat_tools (whose control file pins schema = 'cat_tools' -- + * CREATE EXTENSION always lands in the same place, no choice), extension_drop's + * control file has no schema= line, so CREATE EXTENSION here lands wherever + * the ambient search_path resolves when this file runs -- a fresh psql + * session's default "$user", public, i.e. public in practice. That's a + * deliberate, useful default: it proves nothing in extension_drop's install + * script is hardcoded to a specific schema, the same property + * test/sql/schema.sql proves again explicitly for non-default schemas. + */ +SET client_min_messages = WARNING; + +/* + * The Makefile always exports extension_drop.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 defaulting + * and running the wrong suite. + */ +SELECT current_setting('extension_drop.test_load_mode') AS extension_drop_test_load_mode +\gset + +DO $DO$ +BEGIN + IF current_setting('extension_drop.test_load_mode') NOT IN ('fresh', 'update', 'existing') THEN + RAISE EXCEPTION + 'extension_drop.test_load_mode must be ''fresh'', ''update'' or ''existing'', got ''%''' + , current_setting('extension_drop.test_load_mode') + ; + END IF; +END +$DO$; + +SELECT + :'extension_drop_test_load_mode' = 'update' AS extension_drop_mode_update + , :'extension_drop_test_load_mode' = 'existing' AS extension_drop_mode_existing +\gset + +\if :extension_drop_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. + */ +DO $DO$ +DECLARE + v_installed text := (SELECT extversion FROM pg_extension WHERE extname = 'extension_drop'); + v_default text := (SELECT default_version FROM pg_available_extensions WHERE name = 'extension_drop'); +BEGIN + IF v_installed IS NULL THEN + RAISE EXCEPTION 'test_load_mode=existing but the extension_drop extension is not installed'; + END IF; + IF v_installed IS DISTINCT FROM v_default THEN + RAISE EXCEPTION + 'extension_drop 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 (CASCADE, matching + * cat_tools' own load.sql) so a re-run on a persistent cluster installs the + * newest build instead of reusing stale objects. + * + * extension_drop requires cat_tools. CASCADE auto-installs it on PG10+; + * event triggers exist from 9.3 but CREATE EXTENSION ... CASCADE was only + * added in PG10, so pre-PG10 needs cat_tools created explicitly first. This + * mirrors the check test/deps.sql used to do per-test before this file took + * over installing the extension. server_version_num is read once into a + * psql variable rather than a runtime DO block, so it can drive \if + * (client-side) branching around the VERSION-qualified CREATE EXTENSION + * calls below without needing psql variables interpolated inside a + * dollar-quoted DO body. + */ +DROP EXTENSION IF EXISTS extension_drop CASCADE; + +SELECT current_setting('server_version_num')::int >= 100000 AS extension_drop_pg10_plus +\gset + +\if :extension_drop_mode_update +SELECT current_setting('extension_drop.test_update_from') AS extension_drop_test_update_from \gset +SELECT current_setting('extension_drop.test_update_to') AS extension_drop_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. + */ +SELECT CASE WHEN :'extension_drop_test_update_to' = '' THEN '' + ELSE format('TO %L', :'extension_drop_test_update_to') END + AS extension_drop_update_to_clause \gset + +\if :extension_drop_pg10_plus +CREATE EXTENSION extension_drop VERSION :'extension_drop_test_update_from' CASCADE; +\else +CREATE EXTENSION IF NOT EXISTS cat_tools; +CREATE EXTENSION extension_drop VERSION :'extension_drop_test_update_from'; +\endif + +/* + * Suppress the deprecation NOTICEs an update script might emit. + */ +SET client_min_messages = ERROR; +ALTER EXTENSION extension_drop UPDATE :extension_drop_update_to_clause; +SET client_min_messages = WARNING; +\else +\if :extension_drop_pg10_plus +CREATE EXTENSION extension_drop CASCADE; +\else +CREATE EXTENSION IF NOT EXISTS cat_tools; +CREATE EXTENSION extension_drop; +\endif +\endif +-- end \if :extension_drop_mode_update (fresh vs. update install branch) +\endif +-- end \if :extension_drop_mode_existing (existing mode skips the whole (re)install block) + +-- vi: expandtab ts=2 sw=2 diff --git a/test/sql/dependency_guard.sql b/test/sql/dependency_guard.sql new file mode 100644 index 0000000..9cdec96 --- /dev/null +++ b/test/sql/dependency_guard.sql @@ -0,0 +1,66 @@ +\set ECHO none +\i test/pgxntool/setup.sql + +/* + * Dependency-guard proof. This protects a future "existing" mode CI run + * (extension_drop already installed by a real pg_upgrade, or an ALTER + * EXTENSION UPDATE done outside the suite -- see test/install/load.sql and + * the Makefile's TEST_LOAD_SOURCE machinery): nothing today stops an + * accidental CASCADE drop, a stray CI step, or a logic bug from silently + * destroying the real updated/upgraded objects that mode exists to + * validate -- after which the suite would quietly pass again against a + * fresh reinstall instead of the thing it was supposed to check. + * + * The fix is a view with a HARD pg_depend dependency on a stable + * extension_drop member: something the extension only ever extends, never + * drops or redefines. extension_drop__commands is exactly that -- it's the + * one state table every other object in this extension revolves around + * (get/add/remove/update, the sanity checks, and the event trigger all key + * off it); getting rid of it or changing its identity would be a rewrite of + * the whole extension, not a routine update. Referencing its row type + * (rather than a specific column) means the guard doesn't need updating + * even if a future release adds a column to it. extension_drop has no + * enums (unlike cat_tools' own guard, which types on an enum grown via ADD + * VALUE) -- a stable table's row type serves the same purpose here. + * + * This test PROVES the guard works instead of assuming the SQL is correct: + * it attempts the actual non-CASCADE DROP EXTENSION and asserts it fails, + * then asserts both the extension and the guard view are still present + * afterward. Everything here runs inside pgTAP's own rolled-back + * transaction, so the guard schema/view never leaks into any other test + * file. + */ +CREATE SCHEMA extension_drop_drop_guard; +CREATE VIEW extension_drop_drop_guard.guard AS + SELECT NULL::extension_drop__commands AS guarded_member; + +SELECT plan( + 0 + + 1 -- non-CASCADE drop is blocked + + 1 -- extension_drop is still installed + + 1 -- guard view still present +); + +/* + * 2BP01 = dependent_objects_still_exist: the standard error DROP ... RESTRICT + * (the implicit default for DROP EXTENSION) raises when another object + * depends on something the extension owns. throws_ok's 3-arg overload is + * (sql, message, description), not (sql, sqlstate, description) -- passing + * just the sqlstate there matches message text literally instead of + * checking the code, so the sqlstate AND the real message both need to be + * given explicitly (4-arg form) to actually check the error class. + */ +SELECT throws_ok( + $$DROP EXTENSION extension_drop$$ + , '2BP01' + , 'cannot drop extension extension_drop because other objects depend on it' + , 'Non-CASCADE DROP EXTENSION extension_drop is blocked by the dependency guard' +); + +SELECT has_extension('extension_drop', 'extension_drop is still installed after the blocked drop attempt'); + +SELECT has_view('extension_drop_drop_guard', 'guard', 'Dependency guard view is still present after the blocked drop attempt'); + +\i test/pgxntool/finish.sql + +-- vi: expandtab ts=2 sw=2 diff --git a/test/sql/schema.sql b/test/sql/schema.sql index fa70ed4..20c278e 100644 --- a/test/sql/schema.sql +++ b/test/sql/schema.sql @@ -1,8 +1,30 @@ \set ECHO none -\set TEST_SCHEMA _test_ed +\set TEST_SCHEMA _Test_Ed \i test/pgxntool/setup.sql -CREATE SCHEMA _test_ed_2; +/* + * extension_drop is already installed (test/install/load.sql, committed, + * landing wherever the ambient search_path resolves -- public in practice) + * before this suite runs. This file's actual job is proving the + * schema-targeting/quoting pipeline works, so it drops that committed + * install and recreates its own copies in schemas it chooses instead. Safe + * to drop here: this whole file runs inside pgTAP's own rolled-back + * transaction, so load.sql's committed install is back in place for the + * next test file regardless of what happens below. + * + * :TEST_SCHEMA is mixed-case, so every reference to it MUST be + * identifier-quoted (:"TEST_SCHEMA", or %I via format()) -- an unquoted + * reference would silently fold to lowercase and test a different, + * unquoted schema instead of this one, without erroring. That's + * deliberate: it turns a missing-quote bug in the code under test into a + * hard failure instead of a silent pass. + */ +DROP EXTENSION extension_drop; +CREATE SCHEMA :"TEST_SCHEMA"; +CREATE EXTENSION extension_drop SCHEMA :"TEST_SCHEMA"; +SET search_path = :"TEST_SCHEMA", tap, "$user"; + +CREATE SCHEMA "_Test_Ed_2"; SELECT plan( 0 @@ -28,7 +50,7 @@ SELECT lives_ok( , 'Drop extension' ); -\set TEST_SCHEMA_2 _test_ed_2 +\set TEST_SCHEMA_2 _Test_Ed_2 SELECT lives_ok( format( $$CREATE EXTENSION extension_drop SCHEMA %I$$, :'TEST_SCHEMA_2' ) , 'Create extension in schema ' || :'TEST_SCHEMA_2' @@ -44,11 +66,11 @@ SELECT lives_ok( SET search_path = "$user", public, tap; SELECT lives_ok( - $$SELECT _test_ed_2.extension_drop__update('extension_drop_test', 'moo')$$ + format( $$SELECT %I.extension_drop__update('extension_drop_test', 'moo')$$, :'TEST_SCHEMA_2' ) , 'extension_drop__update()' ); SELECT bag_eq( - $$SELECT * FROM _test_ed_2.extension_drop__get('extension_drop_test')$$ + format( $$SELECT * FROM %I.extension_drop__get('extension_drop_test')$$, :'TEST_SCHEMA_2' ) , $$SELECT 'extension_drop_test'::name , 'moo'::text$$ , 'Verify extension_drop__get()' ); diff --git a/test/sql/simple.sql b/test/sql/simple.sql index 721a825..b01269d 100644 --- a/test/sql/simple.sql +++ b/test/sql/simple.sql @@ -1,5 +1,4 @@ \set ECHO none -\set TEST_SCHEMA _test_ed \i test/pgxntool/setup.sql SELECT plan( @@ -48,17 +47,24 @@ SELECT lives_ok( ); /* - * Check search path for add command + * These calls used to be schema-qualified (_test_ed.extension_drop__remove + * etc.) back when this file's own per-test deps.sql install put + * extension_drop in a private schema and then this section intentionally + * moved search_path away from it, to prove a qualified call still worked. + * extension_drop is now installed once, ambiently (in public, see + * test/install/load.sql), by the time this file runs -- 'public' is always + * on search_path regardless of the change below, so there's no longer a + * schema this file controls to qualify against here. Proving + * schema-qualified access explicitly is test/sql/schema.sql's job now. */ --- Intentionally change our search path SET search_path = "$user", public, tap; SELECT lives_ok( - $$SELECT _test_ed.extension_drop__remove('extension_drop_test')$$ + $$SELECT extension_drop__remove('extension_drop_test')$$ , 'Drop extension command' ); SELECT lives_ok( - $$SELECT _test_ed.extension_drop__add('extension_drop_test', 'moo')$$ + $$SELECT extension_drop__add('extension_drop_test', 'moo')$$ , 'Add extension command' ); @@ -70,7 +76,7 @@ SELECT throws_ok( ); SELECT lives_ok( - $$SELECT _test_ed.extension_drop__remove('extension_drop_test')$$ + $$SELECT extension_drop__remove('extension_drop_test')$$ , 'Drop extension command' ); SELECT lives_ok( From 6eed505d6172e32d9f746e433454d81c6a2a8380 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Tue, 4 Aug 2026 15:19:52 -0500 Subject: [PATCH 3/5] Revert ci.yml pg-build-test switch: pre-existing failures on old PG predate this branch CI on this branch showed the switch to `make test && make verify-results` surfacing real pgTAP failures on PostgreSQL 9.3/9.6 (cat_tools/extension_drop never actually install there). Checked PR #10's own baseline CI (https://github.com/Postgres-Extensions/extension_tools/pull/10, run 30665031257): PG 9.3 and 9.6 already report "3 of 3 tests failed" in the raw job log there too, just silently reported as a passing check because pg-build-test's underlying `make test` hits pgxntool's `.IGNORE: installcheck` the same way. So this isn't a regression from this PR's own changes -- it's the exact masking problem RELEASE.md already documents, just now applying to a different, older part of the PG matrix than the PRs (#6/#7) it originally cites. Reverting the ci.yml step back to pg-build-test here keeps this PR scoped to test/install infrastructure; fixing cat_tools's install path on pre-PG10 belongs to whoever owns that dependency setup (PR #10 or a follow-up), not this PR. Co-Authored-By: Claude Sonnet 5 --- .github/workflows/ci.yml | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 354b400..dc82d51 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -60,15 +60,7 @@ jobs: - name: Check out the repo uses: actions/checkout@v7 - name: Test on PostgreSQL ${{ matrix.pg }} - # pgxntool's base.mk marks installcheck .IGNORE, so a plain `make - # test` (what pg-build-test itself invokes under the hood) exits 0 - # even when every pg_regress test fails -- confirmed happening for - # real on PRs #6/#7 (RELEASE.md). `make verify-results` is the - # actual pass/fail signal (it scans for raw pgTAP failures and plan - # mismatches, not just installcheck's own exit code), so run it - # explicitly after `make test` instead of relying on pg-build-test - # alone. - run: make test && make verify-results + run: pg-build-test # A single stable check name for use as a required status check in branch # protection. Matrix jobs produce names like "🐘 PostgreSQL 14" that change From 67855c7034acc8a1665a165d64857833cdb0509a Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Thu, 6 Aug 2026 14:38:38 -0500 Subject: [PATCH 4/5] Trigger a clean, final CI run From 4dd1d8ad1fb554545ea335a466020bce1cb98613 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Tue, 4 Aug 2026 15:35:36 -0500 Subject: [PATCH 5/5] ci: add pg_tle deployment smoke test for extension_drop (chained on cat_tools) Proves extension_drop can be deployed with zero filesystem footprint via pg_tle (AWS's Trusted Language Extensions), the same fresh-install proof cat_tools' own pg-tle-test job (PR #47) does for itself -- but extension_drop REQUIRES cat_tools, so this job also registers cat_tools as a pg_tle extension first (a leaf extension's pg_tle test never has to solve chained dependency resolution). Every step that could write an extension file to disk is bracketed by a new bin/assert_fs_clean check (modeled on cat_tools' script of the same name), since a stray filesystem .control file silently wins over a pg_tle registration of the same name. Verified end to end locally: built pg_tle 1.5.2 from source, registered pg_tle+cat_tools+extension_drop against template1, CASCADE-installed extension_drop in a smoke database, and called extension_drop__add/__get/ __remove against a real installed extension -- all with zero filesystem control files present throughout (confirmed via bin/assert_fs_clean). Explicitly out of scope: the update path via pg_tle (extension_drop has no prior released version to update from yet) and binary pg_upgrade of a pg_tle-deployed extension -- both noted as follow-up work in the PR. --- .github/workflows/ci.yml | 156 ++++++++++++++++++++++++++++++++++++++- bin/assert_fs_clean | 85 +++++++++++++++++++++ 2 files changed, 240 insertions(+), 1 deletion(-) create mode 100755 bin/assert_fs_clean diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index dc82d51..06a906a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -62,13 +62,167 @@ jobs: - name: Test on PostgreSQL ${{ matrix.pg }} run: pg-build-test + # Proves extension_drop can be deployed with NO filesystem footprint at + # all, via pg_tle (AWS's Trusted Language Extensions -- a database-backed + # catalog for installing an extension with no .control file on disk; + # relevant for RDS/Aurora-style managed deployments). This is a distinct + # dimension from the `test` job above: not fresh vs. updated vs. upgraded, + # but filesystem-installed vs. registered purely through pg_tle's catalog. + # + # extension_drop REQUIRES cat_tools (see the Makefile's own `cat_tools` + # target), so this job has a complication a leaf extension's pg_tle test + # doesn't: cat_tools must ALSO be registered as a pg_tle extension, before + # extension_drop, so `CREATE EXTENSION extension_drop CASCADE` resolves + # its dependency through pg_tle too instead of failing (or, worse, silently + # resolving from a stray filesystem install of cat_tools). Every step that + # could plausibly write an extension file to disk is bracketed by + # bin/assert_fs_clean checks -- see its header for why that has to be an + # active, repeated assertion, not a one-time formality. + # + # A dedicated cluster, never shared with the `test` job above: pg_tle + # requires shared_preload_libraries, and mixing pg_tle/non-pg_tle + # extension installs on one cluster can misbehave. Also deliberately its + # own job rather than folded into any existing matrix/loop -- the entire + # point of this job is proving isolation from the filesystem, and any + # sibling loop iteration doing a real `make install` would contaminate + # that proof. + # + # Explicitly OUT OF SCOPE for this job (left as follow-up work): + # - The update path via pg_tle (ALTER EXTENSION UPDATE against a pg_tle- + # registered install). extension_drop has no prior released version to + # update FROM yet (see TEST_UPDATE_FROM's comment in the Makefile), so + # there's nothing real to exercise. + # - Binary pg_upgrade of a pg_tle-deployed extension. Heavier, separate + # concern from a fresh-install smoke test. + pg-tle-test: + # Gated behind the cheap `test` job: this job compiles pg_tle from + # source and does a chained pg_tle registration, expensive enough not to + # run against a baseline that's already broken by a failing fresh-install + # test. success() is required explicitly once a job's `if:` references + # anything -- GitHub only assumes success() as a default when no `if:` is + # written at all. + needs: [test] + if: success() + strategy: + matrix: + # Intersection of two independently-moving ranges, checked directly + # rather than assumed: extension_drop's own tested range (9.3-17, + # see the `test` job's matrix above) and pg_tle 1.5.2's supported + # PostgreSQL range (12-18, dropped PG11 -- see + # pgxntool/pgtle_versions.md). 18 isn't in the `test` job's own + # matrix yet, so it's left out here too rather than testing a PG + # major extension_drop's own baseline job doesn't cover. + pg: [17, 16, 15, 14, 13, 12] + name: 🧩 pg_tle ${{ matrix.pg }} + runs-on: ubuntu-latest + container: pgxn/pgxn-tools + env: + PG_TLE_RELEASE: "1.5.2" + steps: + - name: Start PostgreSQL ${{ matrix.pg }} + run: pg-start ${{ matrix.pg }} + - name: Check out the repo + uses: actions/checkout@v5 + - name: Install pgtap (test harness dependency) + # pgTAP is a filesystem-installed dependency of the TEST HARNESS + # itself, not part of what this job proves is pg_tle-only -- it's + # never deployed via pg_tle. Installed explicitly here, before the + # baseline snapshot below, so it's part of the accepted starting + # state (like contrib) instead of tripping the contamination check + # if something installed it lazily later. + run: make pgtap + - name: Snapshot filesystem extension control files (pre-pg_tle baseline) + run: bin/assert_fs_clean snapshot ${{ matrix.pg }} /tmp/control_baseline.txt + - name: Build and install pg_tle ${{ env.PG_TLE_RELEASE }} + # flex/bison/libkrb5-dev aren't in the pgxn-tools image; pg_tle's + # build needs them (guc-file.l, and clientauth.c includes gssapi.h). + run: | + apt-get install -y flex bison libkrb5-dev + git clone --branch v${{ env.PG_TLE_RELEASE }} --depth 1 https://github.com/aws/pg_tle.git /tmp/pg_tle + make -C /tmp/pg_tle install + - name: Enable pg_tle and restart PostgreSQL ${{ matrix.pg }} + run: | + echo "shared_preload_libraries = 'pg_tle'" >> /etc/postgresql/${{ matrix.pg }}/test/postgresql.conf + pg_ctlcluster ${{ matrix.pg }} test restart + pg_isready -t 30 + - name: Register pg_tle, then cat_tools, then extension_drop -- all against template1 + # template1, not the ambient default db: pg_tle's registration + # catalog is per-database, and createdb only inherits it because it + # copies template1 by default. Every database used below is created + # AFTER this step so it inherits all three registrations. + # + # cat_tools must be registered BEFORE extension_drop: extension_drop + # requires cat_tools, so the CASCADE install below needs cat_tools + # already resolvable through pg_tle's own catalog by the time it + # runs. cat_tools is cloned fresh at the SAME git ref the Makefile's + # own filesystem `cat_tools` target pins to (CAT_TOOLS_GIT_REF), + # read via `make print-CAT_TOOLS_GIT_REF` so the two never drift + # apart. cat_tools vendors its own pgxntool copy directly (confirmed: + # its only git submodule is an unrelated linter, so a plain `git + # clone` alone gives a working `make run-pgtle`), so registering it + # needs nothing beyond the exact same target this repo uses on + # itself right after. + run: | + psql -d template1 -c "CREATE EXTENSION pg_tle" + CAT_TOOLS_GIT_REF=$(make -s print-CAT_TOOLS_GIT_REF 2>/dev/null | sed -n 's/.*set to "\(.*\)"$/\1/p') + git clone https://github.com/Postgres-Extensions/cat_tools.git /tmp/cat_tools_tle + # -C on both git and make, rather than `cd`: this is one continuous + # shell script (a multi-line `run:` block), so a bare `cd` here + # would still be in effect for the `make run-pgtle` below that's + # meant to run against OUR OWN checkout, not the cat_tools clone. + git -C /tmp/cat_tools_tle checkout "$CAT_TOOLS_GIT_REF" + PGDATABASE=template1 make -C /tmp/cat_tools_tle run-pgtle + PGDATABASE=template1 make run-pgtle + - name: Verify no stray extension control files landed on the filesystem + # CRITICAL: a filesystem control file silently wins over a pg_tle- + # registered extension of the same name, which would make this whole + # job a false pass without ever raising an error. Run again after + # every step below that could plausibly write extension files to + # disk -- never trust a single check to catch everything. + run: bin/assert_fs_clean verify ${{ matrix.pg }} /tmp/control_baseline.txt + - name: Install extension_drop purely via pg_tle (fresh install, no filesystem trace) + # Neither cat_tools nor extension_drop is ever `make install`ed in + # this job, so a successful CASCADE install here can only be + # resolving both through pg_tle's registration, not a control file + # on disk. Checked explicitly here too (not just via the + # comprehensive check above) as a guard specifically for the two + # extensions under test, in case that check's exclude logic has a + # bug. + run: | + test ! -e /usr/share/postgresql/${{ matrix.pg }}/extension/cat_tools.control + test ! -e /usr/share/postgresql/${{ matrix.pg }}/extension/extension_drop.control + createdb extension_drop_smoke + psql -d extension_drop_smoke -c "CREATE EXTENSION extension_drop CASCADE" + - name: Verify extension_drop works when deployed via pg_tle + run: | + INSTALLED=$(psql -d extension_drop_smoke -tAc "SELECT extversion FROM pg_extension WHERE extname = 'extension_drop'") + EXPECTED=$(make -s print-PGXNVERSION 2>/dev/null | sed -n 's/.*set to "\(.*\)"$/\1/p') + echo "installed=$INSTALLED expected=$EXPECTED" + if [ -z "$INSTALLED" ] || [ -z "$EXPECTED" ] || [ "$INSTALLED" != "$EXPECTED" ]; then + echo "FAIL: installed='$INSTALLED' expected='$EXPECTED'"; exit 1 + fi + # A real function call, not just a successful install: exercises + # add/get/remove against an extension (pg_tle itself) that's + # genuinely present in this database, proving the pg_tle-deployed + # functions actually execute correctly, not merely that CREATE + # EXTENSION didn't error. + psql -d extension_drop_smoke -v ON_ERROR_STOP=1 -c "SELECT extension_drop__add('pg_tle', 'SELECT 1')" + psql -d extension_drop_smoke -v ON_ERROR_STOP=1 -c "SELECT * FROM extension_drop__get('pg_tle')" > /dev/null + psql -d extension_drop_smoke -v ON_ERROR_STOP=1 -c "SELECT extension_drop__remove('pg_tle')" + - name: Verify no stray extension control files after the fresh-install smoke test + # Runs AFTER the complete flow, not before: the whole point of + # pg_tle-mode testing is proving NOTHING touched the filesystem + # THROUGHOUT the flow, not merely that the environment started + # clean. + run: bin/assert_fs_clean verify ${{ matrix.pg }} /tmp/control_baseline.txt + # A single stable check name for use as a required status check in branch # protection. Matrix jobs produce names like "🐘 PostgreSQL 14" that change # with the matrix; this aggregates them into one. It passes if every needed # job succeeded or was skipped (e.g. a docs-only push with paths-ignore) and # fails if any failed or were cancelled. all-checks-passed: - needs: [lint, test] + needs: [lint, test, pg-tle-test] if: always() runs-on: ubuntu-latest steps: diff --git a/bin/assert_fs_clean b/bin/assert_fs_clean new file mode 100755 index 0000000..93a2657 --- /dev/null +++ b/bin/assert_fs_clean @@ -0,0 +1,85 @@ +#!/usr/bin/env bash +# +# assert_fs_clean - Verify no stray PostgreSQL extension control files exist +# on disk, to prove an extension (and its dependencies) were deployed purely +# via pg_tle (AWS's Trusted Language Extensions), not a filesystem install. +# +# A pre-existing filesystem *.control file silently wins over a pg_tle- +# registered extension of the same name -- PostgreSQL never reports an +# error, it just quietly resolves CREATE EXTENSION from disk instead of +# pg_tle's catalog. That makes "prove pg_tle-only" a real, load-bearing +# assertion, not a formality: it must run AFTER whatever step it's guarding, +# not just before, since the whole point is confirming nothing wrote to disk +# THROUGHOUT the guarded flow, not merely that the environment started +# clean. +# +# Deliberately generic over extension name: it diffs the FULL set of +# *.control files against a baseline, so it catches a stray install of ANY +# extension (extension_drop, cat_tools, or something unrelated), not just a +# hardcoded name list -- a chained dependency doesn't need its own entry +# here. +# +# Modeled on cat_tools's bin/assert_fs_clean. +# +# USAGE: bin/assert_fs_clean [args] +# +# snapshot PG_MAJOR BASELINE_FILE +# Record the current *.control files in PG_MAJOR's extension directory +# to BASELINE_FILE. Run this BEFORE installing pg_tle (or anything +# else that could write to disk), so whatever ships by default (e.g. +# contrib, pgTAP installed as the test harness's own dependency) is +# excluded automatically -- no hardcoded exclude list to keep in sync. +# +# verify PG_MAJOR BASELINE_FILE +# Fail if any *.control file exists now that wasn't in BASELINE_FILE, +# other than pg_tle.control itself (the one legitimate filesystem +# install in this flow). Run this after EVERY step that could +# plausibly have written extension files to disk. +set -euo pipefail + +extdir_of() { echo "/usr/share/postgresql/$1/extension"; } + +snapshot() { + local pg_major=$1 baseline=$2 + find "$(extdir_of "$pg_major")" -maxdepth 1 -name '*.control' | sort > "$baseline" +} + +verify() { + local pg_major=$1 baseline=$2 + local after + after=$(mktemp) + # Bake the actual path into the trap string now (double-quoted expansion), + # rather than deferring expansion to whenever the trap fires -- a RETURN + # trap wouldn't fire at all under `set -e` (this script's own errexit), and + # a single-quoted EXIT trap referencing $after by name would break once + # this function returns and $after (declared local) goes out of scope. + trap "rm -f '$after'" EXIT + local new + find "$(extdir_of "$pg_major")" -maxdepth 1 -name '*.control' | sort > "$after" + new=$(comm -13 "$baseline" "$after" | grep -vx '.*/pg_tle\.control' || true) + if [ -n "$new" ]; then + echo "FAIL: unexpected extension control file(s) on disk (everything but pg_tle must be registered via pg_tle, not filesystem-installed):" >&2 + echo "$new" >&2 + exit 1 + fi + echo "OK: no stray extension control files on disk (PG $pg_major)" +} + +usage() { + echo "usage: bin/assert_fs_clean [args]" >&2 + echo " snapshot PG_MAJOR BASELINE_FILE" >&2 + echo " verify PG_MAJOR BASELINE_FILE" >&2 + exit 2 +} + +main() { + local cmd=${1:-} + shift || true + case "$cmd" in + snapshot) snapshot "$@" ;; + verify) verify "$@" ;; + *) usage ;; + esac +} + +main "$@"