diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d5aa980..cc40a4f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -8,22 +8,8 @@ name: CI # extension_drop version on an OLD cluster, plant a dependency guard, binary # pg_upgrade straight to a NEWER major, then run the suite in existing mode # against the real migrated objects (see bin/test_existing). No update step -# and no bridge leg: extension_drop has only ever shipped one real version -# (1.0.0 -- see HISTORY.asc/RELEASE.md; the only PGXN listing, 0.1.x from -# 2017, predates the current SQL entirely), so extversion never changes -# across a leg and there is no known pg_upgrade-unsafe old version to bridge -# from (see ~/advanced-extension-testing.md ยง6c's own guidance not to build -# that preemptively). Legs' old_pg floor is 12, NOT extension_drop's own -# claimed 9.3 floor above: this workflow's CAT_TOOLS_GIT_REF (see below) -# makes `make install` build cat_tools from the `0.3.0` git tag instead of -# PGXN (see the Makefile's `cat_tools` target comment -- PGXN's published -# cat_tools is a stale 2017 release extension_drop can't use), and that -# current cat_tools requires PostgreSQL >= 12 for a fresh install (its own META.json, -# build.requires.PostgreSQL). Below PG12, `make install` cannot complete at -# all today, independent of anything this job does -- consistent with the -# already-known pre-existing old-PG failures on the plain `test` job (see -# this branch's own history: "Revert ci.yml pg-build-test switch: -# pre-existing failures on old PG predate this branch"). +# and no bridge leg: extversion never changes across a leg, and there is no +# known pg_upgrade-unsafe old version to bridge from. # # Scope: push only runs on master (post-merge); PR commits are covered by # pull_request -- avoids double-running CI for the same commit. @@ -165,6 +151,36 @@ jobs: uses: actions/checkout@v7 - name: Test on PostgreSQL ${{ matrix.pg }} run: pg-build-test + # Update-path leg (0.1.1 -> stable): runs here as extra steps in the + # same already-running container/cluster rather than a separate job, + # since a whole extra job means a whole extra container boot + + # PostgreSQL startup for no isolation benefit -- nothing about this + # leg needs its own environment, unlike e.g. pg_tle's job, which + # genuinely does. + # + # PG11+ only: cat_tools.routine__parse_arg_types_text() (which + # extension_drop's install script calls unconditionally, in both 0.1.1 + # and current) transitively hits cat_tools's _cat_tools.function__drop_temp, + # which runs a hardcoded `DROP ROUTINE` -- syntax that doesn't exist + # before PG11 -- see + # https://github.com/Postgres-Extensions/extension_tools/issues/44. + # + # rsync: pgxntool's test-build feature needs it (see + # pgxntool/run-test-build.sh), but pg-build-test above never actually + # exercises test-build -- it goes through a lower-level PGXS path that + # bypasses this repo's own `test:` Makefile target entirely. A plain + # `make test`/`make install` (this step, and pg-upgrade-test below) + # does, so it needs installing explicitly, same as pg-upgrade-test + # already does. + - name: Install rsync + if: matrix.pg >= 11 + run: apt-get install -y rsync + - name: "Prove the update path: install 0.1.1, plant guard, ALTER EXTENSION UPDATE, re-prove guard, assert version" + if: matrix.pg >= 11 + run: bin/test_update_path.sh + - name: Run the pgTAP suite in update mode (0.1.1 -> stable) against the result + if: matrix.pg >= 11 + run: make test-update # Proves extension_drop survives a BINARY pg_upgrade (in-place catalog # migration to a newer PostgreSQL major), not just a fresh install. Each diff --git a/Makefile b/Makefile index 960c018..8fb1748 100644 --- a/Makefile +++ b/Makefile @@ -11,12 +11,11 @@ PGXNTOOL_ENABLE_TEST_INSTALL = yes # - 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. +# update behaves identically to a fresh install. TEST_UPDATE_FROM +# defaults to 0.1.1 (see sql/extension_drop--0.1.1.sql and +# sql/extension_drop--0.1.1--stable.sql). Empty TEST_UPDATE_TO (the +# default) means "update to the current default_version", which is now +# the `stable` pseudo-version. # - 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 @@ -37,14 +36,15 @@ $(error TEST_LOAD_SOURCE must be 'fresh', 'update' or 'existing', got '$(TEST_LO 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 ?= +# Empty TEST_UPDATE_TO means "update to the current default_version" (now +# `stable`). TEST_UPDATE_FROM defaults to 0.1.1, overridable if needed. The +# guard below just protects against someone explicitly blanking it out +# (TEST_UPDATE_FROM= on the command line). +TEST_UPDATE_FROM ?= 0.1.1 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) +$(error TEST_UPDATE_FROM must not be blank when TEST_LOAD_SOURCE=update) endif endif @@ -107,10 +107,13 @@ $(DESTDIR)$(datadir)/extension/cat_tools.control: # Style linter (see https://github.com/Postgres-Extensions/linter, vendored # at .vendor/linter -- lint.mk is the thin local hand-off, see its comment). # Scoped to the actively-maintained source rather than the default -# `sql/ test/`: sql/extension_drop--1.0.0.sql is a frozen, already-released -# version file (RELEASE.md's "Ongoing development" section -- once a version -# is released, its sql/--.sql is never hand-edited again), so -# linting it would produce permanent, unfixable findings and make `make -# lint` unusable as a CI gate. Lint the hand-maintained source instead. -LINT_TARGETS = sql/extension_drop.sql test/ +# `sql/ test/`: frozen, already-released version files (RELEASE.md's +# "Ongoing development" section -- once a version is released, its +# sql/--.sql is never hand-edited again) would produce +# permanent, unfixable findings and make `make lint` unusable as a CI +# gate. $(EXTENSION_SQL_FILES) (pgxntool's control.mk.sh, from each +# .control file at the repo root) is exactly the hand-maintained +# sql/.sql for every extension this distribution provides, so this +# keeps working if a second extension is ever added. +LINT_TARGETS = $(EXTENSION_SQL_FILES) test/ include lint.mk diff --git a/bin/test_update_path.sh b/bin/test_update_path.sh new file mode 100755 index 0000000..dbc3738 --- /dev/null +++ b/bin/test_update_path.sh @@ -0,0 +1,86 @@ +#!/bin/sh +# Exercises the extension_drop update path: 0.1.1 -> stable. +# +# Assumes extension_drop and cat_tools are already built and installed into +# the active PostgreSQL cluster (`make install`, which pulls in the cat_tools +# deps target first) and that psql's ambient connection defaults reach it. +# +# The dependency-guard checks attempt the blocked DROP EXTENSION and inspect +# the actual error, rather than trusting the guard exists; the version check +# reads extension_drop.control's default_version dynamically rather than +# hardcoding an expected value that would silently drift out of sync with a +# future release. +# 1. CREATE EXTENSION extension_drop VERSION '0.1.1' -- installs the +# actual historical release. CASCADE (to auto-install cat_tools) only +# exists from PG10 -- pre-PG10 needs cat_tools created explicitly +# first, same as test/install/load.sql. +# 2. Plant a dependency-guard view and prove a non-CASCADE DROP EXTENSION +# is blocked -- BEFORE the update, proving the guard actually attaches +# to the 0.1.1-era extension_drop__commands table. +# 3. ALTER EXTENSION extension_drop UPDATE -- runs +# sql/extension_drop--0.1.1--stable.sql. +# 4. Re-prove the SAME guard still blocks a non-CASCADE drop -- proves the +# update script didn't touch extension_drop__commands's identity. +# 5. Assert the installed version now matches extension_drop.control's +# default_version, read dynamically from the control file rather than +# hardcoded, with empty-value guards. +set -eu + +cd "$(dirname "$0")/.." + +DB=${1:-extension_drop_update_test} + +dropdb --if-exists "$DB" +createdb "$DB" + +PG10_PLUS=$(psql -tAc "SELECT current_setting('server_version_num')::int >= 100000" -d "$DB") + +if [ "$PG10_PLUS" = "t" ]; then + CREATE_EXTENSION_DROP="CREATE EXTENSION extension_drop VERSION '0.1.1' CASCADE;" +else + CREATE_EXTENSION_DROP="CREATE EXTENSION IF NOT EXISTS cat_tools; +CREATE EXTENSION extension_drop VERSION '0.1.1';" +fi + +psql -v ON_ERROR_STOP=1 -d "$DB" -c " +$CREATE_EXTENSION_DROP + +CREATE SCHEMA extension_drop_drop_guard; +CREATE VIEW extension_drop_drop_guard.guard AS + SELECT NULL::extension_drop__commands AS guarded_member; +" + +assert_guard_blocks_drop() { + label=$1 + if psql -v ON_ERROR_STOP=1 -d "$DB" -c 'DROP EXTENSION extension_drop' >/tmp/guard_drop.out 2>/tmp/guard_drop.err; then + echo "FAIL ($label): DROP EXTENSION extension_drop succeeded -- the dependency guard did not block it" >&2 + exit 1 + fi + if ! grep -q 'cannot drop extension extension_drop because other objects depend on it' /tmp/guard_drop.err; then + echo "FAIL ($label): DROP EXTENSION failed, but not with the expected dependency-guard error:" >&2 + cat /tmp/guard_drop.err >&2 + exit 1 + fi + echo "OK ($label): non-CASCADE DROP EXTENSION extension_drop is blocked by the dependency guard" +} + +assert_guard_blocks_drop "pre-update, at 0.1.1" + +psql -v ON_ERROR_STOP=1 -d "$DB" -c "SET client_min_messages = ERROR; ALTER EXTENSION extension_drop UPDATE" + +assert_guard_blocks_drop "post-update" + +INSTALLED=$(psql -tAc "SELECT extversion FROM pg_extension WHERE extname = 'extension_drop'" -d "$DB" | tr -d '[:space:]') +EXPECTED=$(sed -n "s/^default_version[[:space:]]*=[[:space:]]*'\([^']*\)'.*/\1/p" extension_drop.control | tr -d '[:space:]') + +if [ -z "$INSTALLED" ] || [ -z "$EXPECTED" ] || [ "$INSTALLED" != "$EXPECTED" ]; then + echo "FAIL: installed='$INSTALLED' expected='$EXPECTED' (derived from extension_drop.control)" >&2 + exit 1 +fi +echo "OK: extension_drop landed at '$INSTALLED' after update, matching extension_drop.control's default_version" + +dropdb "$DB" + +echo "PASS: 0.1.1 -> $INSTALLED update path verified (install, guard survival, version assertion)." + +# vi: expandtab ts=2 sw=2 diff --git a/sql/extension_drop--0.1.1--stable.sql b/sql/extension_drop--0.1.1--stable.sql new file mode 100644 index 0000000..b011c62 --- /dev/null +++ b/sql/extension_drop--0.1.1--stable.sql @@ -0,0 +1,28 @@ +CREATE OR REPLACE FUNCTION extension_drop__event_trigger( +) RETURNS event_trigger LANGUAGE plpgsql SET search_path FROM CURRENT AS +$body$ +DECLARE + r extension_drop__commands; +BEGIN + RAISE DEBUG 'extension_drop event trigger entry: tg_event %, tg_tag %', tg_event, tg_tag; + FOR r IN + SELECT c.* + FROM extension_drop__commands c + JOIN pg_event_trigger_dropped_objects() d + ON c.extension_name = d.object_name + AND d.object_type = 'extension' + LOOP + RAISE DEBUG E'extension "%" is being dropped; executing SQL:\n%', r.extension_name, r.sql; + EXECUTE r.sql; + DELETE FROM extension_drop__commands WHERE extension_name = r.extension_name; + END LOOP; + + /* + * Need to do this after the fact since the extensions being dropped have + * already been removed from the catalog by the time this function is called. + */ + PERFORM extension_drop__sanity_assert(); +END +$body$; + +-- vim: sw=2 ts=2 expandtab diff --git a/sql/extension_drop--0.1.1.sql b/sql/extension_drop--0.1.1.sql new file mode 100644 index 0000000..846dcf9 --- /dev/null +++ b/sql/extension_drop--0.1.1.sql @@ -0,0 +1,324 @@ +/* + * NOTE: All pg_temp objects must be dropped at the end of the script! + * Otherwise the eventual DROP CASCADE of pg_temp when the session ends will + * also drop the extension! Instead of risking problems, create our own + * "temporary" schema instead. + */ +CREATE SCHEMA __extension_drop; + +CREATE TABLE __extension_drop.messages AS SELECT pg_catalog.current_setting('client_min_messages'); +SET client_min_messages = WARNING; + +CREATE FUNCTION __extension_drop.exec( + sql text +) RETURNS void LANGUAGE plpgsql AS $body$ +BEGIN + RAISE DEBUG 'sql = %', sql; + EXECUTE sql; +END +$body$; + +CREATE FUNCTION __extension_drop.safe_dump( + relation regclass + , filter text DEFAULT '' +) RETURNS void LANGUAGE plpgsql AS $body$ +BEGIN + PERFORM pg_catalog.pg_extension_config_dump(relation, filter); +EXCEPTION WHEN feature_not_supported THEN + NULL; +END +$body$; + +CREATE FUNCTION __extension_drop.create_function( + function_name text + , args text + , options text + , body text + , comment text + , grants text DEFAULT NULL +) RETURNS void LANGUAGE plpgsql AS $body$ +DECLARE + c_clean_args text := cat_tools.function__arg_types_text(args); + + create_template CONSTANT text := $template$ +CREATE OR REPLACE FUNCTION %s( +%s +) RETURNS %s SET search_path FROM CURRENT AS +%L +$template$ + ; + + revoke_template CONSTANT text := $template$ +REVOKE ALL ON FUNCTION %s( +%s +) FROM public; +$template$ + ; + + grant_template CONSTANT text := $template$ +GRANT EXECUTE ON FUNCTION %s( +%s +) TO %s; +$template$ + ; + + comment_template CONSTANT text := $template$ +COMMENT ON FUNCTION %s( +%s +) IS %L; +$template$ + ; + +BEGIN + PERFORM __extension_drop.exec( format( + create_template + , function_name + , args + , options -- TODO: Force search_path if options ~* 'definer' + , body + ) ) + ; + + IF grants IS NOT NULL THEN + PERFORM __extension_drop.exec( format( + revoke_template + , function_name + , c_clean_args + ) ) + ; + IF grants <> '' THEN + PERFORM __extension_drop.exec( format( + grant_template + , function_name + , c_clean_args + , grants + ) ) + ; + END IF; + END IF; + + IF comment IS NOT NULL THEN + PERFORM __extension_drop.exec( format( + comment_template + , function_name + , c_clean_args + , comment + ) ) + ; + END IF; +END +$body$; + +CREATE TABLE extension_drop__commands( + extension_name name PRIMARY KEY + , sql text NOT NULL +); +SELECT __extension_drop.safe_dump('extension_drop__commands', ''); + +SELECT __extension_drop.create_function( + 'extension_drop__sanity_check' + , 'ignore name DEFAULT NULL' + , 'name[] LANGUAGE sql STABLE' + , $body$ +SELECT array( + SELECT extension_name + FROM extension_drop__commands c + WHERE NOT EXISTS(SELECT 1 FROM pg_catalog.pg_extension e WHERE e.extname = c.extension_name) + AND extension_name IS DISTINCT FROM ignore + ) +$body$ + , $$Returns an array of extensions that have drop commands but do not exist. This array should always be empty!$$ +); + +SELECT __extension_drop.create_function( + 'extension_drop__sanity_assert' + , 'ignore name DEFAULT NULL' + , 'void LANGUAGE plpgsql STABLE' + , $body$ +DECLARE + bad name[] := extension_drop__sanity_check(ignore); +BEGIN + IF bad != '{}'::name[] THEN + RAISE 'unexpected drop commands' + USING ERRCODE = 'XD001' + , HINT = $$This should not happen unless someone manually inserted into "extension_drop__commands" or messed with the "extension_drop" event trigger. + Use SELECT extension_drop__repair() to fix this.$$ + , DETAIL = format( + '%s not exist: %s' + , CASE WHEN array_length(bad, 1) = 1 THEN 'This extension does' ELSE 'These extensions do' END + , array_to_string(bad, ', ') + ) + ; + END IF; +END +$body$ + , $$Throws an error if the "extension_drop__commands" table is not in a sane state.$$ +); + +/* + * REPAIR + */ +SELECT __extension_drop.create_function( + 'extension_drop__repair' + , '' + , 'void LANGUAGE sql' + , $body$ +DELETE FROM extension_drop__commands WHERE extension_name = ANY( extension_drop__sanity_check() ) +$body$ + , 'Repairs the "extension_drop__commands" table. THIS FUNCTION SHOULD NEVER BE NEEDED.' + , '' -- Just revoke all access +); + +/* + * GET + */ +SELECT __extension_drop.create_function( + 'extension_drop__get' + , $$ + extension_name extension_drop__commands.extension_name%TYPE +$$ + , 'extension_drop__commands STABLE LANGUAGE plpgsql' + , $body$ +DECLARE + ret extension_drop__commands; +BEGIN + PERFORM extension_drop__sanity_assert(); + SELECT INTO STRICT ret + * + FROM extension_drop__commands d + WHERE d.extension_name = extension_drop__get.extension_name + ; + + RETURN ret; + +EXCEPTION WHEN no_data_found THEN + RAISE 'no drop commands for extension "%"', extension_name + USING errcode = 'no_data_found' + ; +END +$body$ + , $$Get info about a set of commands to be run when an extension is dropped.$$ +); + +/* + * ADD + */ +SELECT __extension_drop.create_function( + 'extension_drop__add' + , $$ + extension_name extension_drop__commands.extension_name%TYPE + , sql extension_drop__commands.sql%TYPE +$$ + , 'void LANGUAGE plpgsql' + , $body$ +BEGIN + INSERT INTO extension_drop__commands VALUES(extension_name, sql); + PERFORM extension_drop__sanity_assert(); +END +$body$ + , $$Adds a set of commands to be run when an extension is dropped.$$ + , '' -- Just revoke all access +); + +/* + * REMOVE + */ +SELECT __extension_drop.create_function( + 'extension_drop__remove' + , $$ + extension_name extension_drop__commands.extension_name%TYPE +$$ + , 'void LANGUAGE sql' + , $body$ +DELETE FROM extension_drop__commands d + -- extension_drop__get() runs sanity checks for us + WHERE d.extension_name = (extension_drop__get(extension_name)).extension_name +$body$ + , $$Remove a set of commands to be run when an extension is dropped.$$ + , '' -- Just revoke all access +); + +/* + * UPDATE + */ +SELECT __extension_drop.create_function( + 'extension_drop__update' + , $$ + extension_name extension_drop__commands.extension_name%TYPE + , sql extension_drop__commands.sql%TYPE +$$ + , 'void LANGUAGE sql' + , $body$ +UPDATE extension_drop__commands d + SET sql = extension_drop__update.sql + -- extension_drop__get() runs sanity checks for us + WHERE d.extension_name = (extension_drop__get(extension_name)).extension_name +$body$ + , $$Update the set of commands to be run when an extension is dropped.$$ + , '' -- Just revoke all access +); + +/* + * TRIGGER FUNCTION + */ +SELECT __extension_drop.create_function( + 'extension_drop__event_trigger' + , '' + , 'event_trigger LANGUAGE plpgsql' + , $body$ +DECLARE + r extension_drop__commands; +BEGIN + FOR r IN + SELECT c.* + FROM extension_drop__commands c + JOIN pg_event_trigger_dropped_objects() d + ON c.extension_name = d.object_name + AND d.object_type = 'extension' + LOOP + RAISE DEBUG E'extension "%" is being dropped; executing SQL:\n%', r.extension_name, r.sql; + EXECUTE r.sql; + DELETE FROM extension_drop__commands WHERE extension_name = r.extension_name; + END LOOP; + + /* + * Need to do this after the fact since the extensions being dropped have + * already been removed from the catalog by the time this function is called. + */ + PERFORM extension_drop__sanity_assert(); +END +$body$ + , 'Event trigger function that does the actual work for extension_drop.' +); + +CREATE EVENT TRIGGER extension_drop + ON sql_drop + WHEN tag IN( 'DROP EXTENSION' ) -- NOTE! This MUST be IN + EXECUTE PROCEDURE extension_drop__event_trigger() +; + +/* + * Drop "temporary" objects + */ +SELECT __extension_drop.exec('SET client_min_messages = ' || current_setting) + FROM __extension_drop.messages +; +DROP TABLE __extension_drop.messages; +DROP FUNCTION __extension_drop.create_function( + function_name text + , args text + , options text + , body text + , comment text + , grants text +); +DROP FUNCTION __extension_drop.safe_dump( + relation regclass + , text +); +DROP FUNCTION __extension_drop.exec( + sql text +); +DROP SCHEMA __extension_drop; + +-- vim: sw=2 ts=2 expandtab diff --git a/test/install/load.sql b/test/install/load.sql index d90544e..e3904d7 100644 --- a/test/install/load.sql +++ b/test/install/load.sql @@ -19,15 +19,9 @@ * - 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. + * the current default_version. extension_drop.test_update_from defaults + * to 0.1.1 (see sql/extension_drop--0.1.1.sql). Exercised in CI as + * extra steps in the `test` job's own matrix (see ci.yml). * - 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