Install extension_drop into a fresh, randomly named schema on every test run

CLAUDE.md

@@ -20,10 +20,19 @@ same schema-independence property. It's genuinely schema-flexible. See
 methodology (why `schema=` and not `relocatable=` is what matters, and how
 to actually prove search_path-independence).
 
-This is best implemented as a `TEST_SCHEMA` switch on the *whole* test
-suite (mirroring `TEST_LOAD_SOURCE`'s existing GUC-propagation mechanism
-through `test/install/load.sql` / `test/deps.sql`), not confined to a single
-dedicated test file — a dedicated file only proves the property for whatever
-it happens to exercise, not for the rest of the suite.
-`Postgres-Extensions/pg_count_nulls` PR #28 ("TEST_SCHEMA switching in
-test/install") is a working reference implementation of this exact pattern.
+`test/install/load.sql` implements this by installing into a freshly
+created, randomly named schema (a constant, quote-forcing prefix plus a
+random suffix) on every single fresh/update-mode run — not a configurable
+switch confined to one leg or one dedicated test file, since a single
+dedicated file only proves the property for whatever it happens to
+exercise, not for the rest of the suite. `test/deps.sql` rediscovers that
+schema per test-file session (it never adds it to `search_path`, and
+never assumes a naming convention beyond what it queries from
+`pg_extension`/`pg_namespace` directly); `test/finish.sql` asserts at the
+end of every test file that the schema was never reachable via
+`search_path` regardless.
+
+`https://github.com/Postgres-Extensions/pg_count_nulls/pull/55` is the
+reference this was adapted from (not ported wholesale — see that PR, and
+`test/install/load.sql`'s own header comment, for the structural
+differences that don't transfer between the two extensions).

bin/test_existing

@@ -53,27 +53,8 @@
 # non-CASCADE DROP EXTENSION fails, and actively PROVE that here (see
 # plant_guard): if the drop unexpectedly succeeds, this script fails CI. See
 # also test/sql/dependency_guard.sql, which proves the same mechanism as a
-# pgTAP test (a separate, rolled-back proof -- see EXISTING_MODE_EXCLUDE below
-# for why this script's guard uses a DIFFERENT schema name than that test's).
-#
-# EXISTING_MODE_EXCLUDE (below): test/sql/schema.sql's whole job is proving
-# the schema-targeting/quoting install pipeline -- it does this by freely
-# DROPping and recreating extension_drop (non-CASCADE) in several schemas,
-# which is fundamentally incompatible with a real, persistent dependency
-# guard: the very first statement in that file (a plain
-# `DROP EXTENSION extension_drop;`) would fail against a guarded database,
-# not because of a bug, but because the guard is doing exactly its job.
-# Confirmed hitting this running the flow locally (PG12->PG17): schema.sql's
-# first statement failed with the guard's own 2BP01 error. Not a regression --
-# schema.sql already runs, and is already proven, by the regular fresh `test`
-# job on every PostgreSQL major. Existing mode's job is different: prove the
-# SURVIVING install still works and wasn't silently reinstalled, which
-# dependency_guard.sql + simple.sql already cover. So it's excluded from the
-# existing-mode REGRESS list here, not papered over. (test/sql/zzz_build.sql
-# used to be excluded for a similar reason -- it's since been migrated to
-# pgxntool's native test/build/ feature, which runs as its own separate
-# installcheck invocation entirely outside the main REGRESS list, so there's
-# nothing left here to exclude it from.)
+# pgTAP test (a separate, rolled-back proof, using a different guard schema
+# than this script's own -- see GUARD_SCHEMA below).
 set -euo pipefail
 
 # Run from the repository root (where `make` works and test paths resolve),
@@ -89,19 +70,11 @@ cd "$(dirname "$(readlink -f "$0")")/.."
 # event trigger, all key off it) -- referencing its row type means the guard
 # needs no updating even if a future release adds a column to it. Uses a
 # DIFFERENT schema than test/sql/dependency_guard.sql's own
-# extension_drop_drop_guard (see the EXISTING_MODE_EXCLUDE comment above) so
-# the two never collide when both exist in the same database.
+# extension_drop_drop_guard so the two never collide when both exist in the
+# same database.
 GUARD_SCHEMA=extension_drop_ci_guard
 GUARD_VIEW=guard
 
-# See the top-of-file comment: schema.sql is excluded from the existing-mode
-# suite run, not because it's broken, but because its own job (freely
-# DROPping and recreating extension_drop in several schemas) is inherently
-# incompatible with a real, persistent dependency guard. It already runs,
-# and is already proven, by the regular fresh `test` job on every
-# PostgreSQL major.
-EXISTING_MODE_EXCLUDE="schema"
-
 # ---------------------------------------------------------------------------
 # psql helpers
 # ---------------------------------------------------------------------------
@@ -203,19 +176,13 @@ prepare() {
   plant_guard "$db"
 }
 
-# Compute the existing-mode REGRESS list: every test/sql/*.sql basename
-# except EXISTING_MODE_EXCLUDE (see the top-of-file comment for why). Derived
-# from the actual directory contents (not a hardcoded full list) so a future
-# new test/sql file is automatically included in existing-mode runs unless
-# someone deliberately adds it to EXISTING_MODE_EXCLUDE.
+# Compute the existing-mode REGRESS list: every test/sql/*.sql basename,
+# derived from the actual directory contents (not a hardcoded list) so a
+# future new test/sql file is automatically included in existing-mode runs.
 existing_mode_regress() {
-  local f base
+  local f
   for f in test/sql/*.sql; do
-    base=$(basename "$f" .sql)
-    case " $EXISTING_MODE_EXCLUDE " in
-      *" $base "*) continue ;;
-    esac
-    echo "$base"
+    basename "$f" .sql
   done
 }
 
@@ -241,8 +208,8 @@ run_suite() {
   #      filesystem-touching step existing mode exists to avoid.
   #   2. verify-results depends on `test`, so it re-runs the suite; it must
   #      carry the SAME existing-mode overrides (including REGRESS) or it
-  #      would re-run the FULL suite (hitting the schema.sql conflict above)
-  #      instead of verifying THIS existing database.
+  #      would re-run the FULL suite instead of verifying THIS existing
+  #      database.
   # An ARRAY, not a flat string: REGRESS's value is itself multiple
   # space-separated test names, so passing it through a single unquoted
   # variable (like cat_tools's own $existing_args, which never needed a

test/deps.sql

@@ -1,6 +1,39 @@
 -- Add any test dependency statements here
 -- Note: pgTap is loaded by setup.sql
 
+/*
+ * test/install/load.sql (fresh/update mode) installs extension_drop into a
+ * freshly created, randomly named schema every run -- see its header
+ * comment. This file runs in its own SEPARATE psql session per test file
+ * (pg_regress spawns a new connection per test file under test/sql/), so it
+ * cannot reuse a psql variable set in load.sql's session; it has to
+ * rediscover the schema name from the catalog instead.
+ *
+ * pg_extension.extnamespace is an oid, not a name -- join pg_namespace
+ * directly rather than casting through extnamespace::regnamespace: that
+ * cast produces regnamespace's quoted TEXT display form (e.g. "My Schema"
+ * complete with literal quote characters for a name needing them), and
+ * reinterpreting those literal quotes as part of the name is subtly wrong.
+ * The direct join always returns the real, unquoted name.
+ *
+ * This also handles existing mode "for free": whatever schema a real
+ * pg_upgrade actually used is found the same way, no special-casing needed.
+ *
+ * Deliberately NOT added to search_path: doing so would make any
+ * accidentally-unqualified self-reference inside extension_drop's own
+ * functions resolve correctly too, masking exactly the class of bug this
+ * whole random-schema scheme exists to catch. Every test file that calls
+ * into extension_drop's own functions qualifies those calls explicitly
+ * using :'extension_drop_schema' instead (test/sql/simple.sql,
+ * test/sql/dependency_guard.sql) -- test/finish.sql asserts at the end of
+ * every file that the schema never ended up on search_path regardless.
+ */
+SELECT n.nspname AS extension_drop_schema
+FROM pg_namespace n
+JOIN pg_extension x ON n.oid = x.extnamespace
+WHERE x.extname = 'extension_drop'
+\gset
+
 \set TT extension_drop_test_table
 CREATE TEMP TABLE :TT (i int);
 

test/expected/dependency_guard.out

@@ -1,6 +1,7 @@
 \set ECHO none
-1..3
+1..4
 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
+ok 4 - extension_drop's schema should not be reachable via search_path
 # TRANSACTION INTENTIONALLY LEFT OPEN!

test/expected/dependency_guard_1.out

@@ -0,0 +1,7 @@
+\set ECHO none
+1..4
+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
+ok 4 # SKIP existing mode: extension_drop's placement is outside this suite's control
+# TRANSACTION INTENTIONALLY LEFT OPEN!

test/expected/simple.out

@@ -1,5 +1,5 @@
 \set ECHO none
-1..11
+1..12
 ok 1 - Insert into extension_drop_test_table
 ok 2 - Create test extension
 ok 3 - Verify extension_drop__get()
@@ -11,4 +11,5 @@ ok 8 - Add extension command
 ok 9 - Dropping extension with bad command should fail
 ok 10 - Drop extension command
 ok 11 - Drop test extension
+ok 12 - extension_drop's schema should not be reachable via search_path
 # TRANSACTION INTENTIONALLY LEFT OPEN!

test/expected/simple_1.out

@@ -0,0 +1,15 @@
+\set ECHO none
+1..12
+ok 1 - Insert into extension_drop_test_table
+ok 2 - Create test extension
+ok 3 - Verify extension_drop__get()
+ok 4 - Drop test extension
+ok 5 - extension_drop_test_table is empty
+ok 6 - Create test extension again
+ok 7 - Drop extension command
+ok 8 - Add extension command
+ok 9 - Dropping extension with bad command should fail
+ok 10 - Drop extension command
+ok 11 - Drop test extension
+ok 12 # SKIP existing mode: extension_drop's placement is outside this suite's control
+# TRANSACTION INTENTIONALLY LEFT OPEN!

test/finish.sql

@@ -0,0 +1,37 @@
+/*
+ * Sanity check: prove nothing in this test file left extension_drop's
+ * schema reachable via search_path by the time it finishes, regardless of
+ * what the file itself was testing. This repo's test files are individual
+ * plain pgTAP scripts (no shared runtests()/teardown__ function library to
+ * hook a check into, unlike e.g. Postgres-Extensions/pg_count_nulls) -- so
+ * every file under test/sql that needs this calls this file instead of
+ * calling test/pgxntool/finish.sql (the vendored default) directly. This
+ * file does the check, then delegates to the real one.
+ *
+ * Reuses :'extension_drop_schema' (set once by test/deps.sql) rather than
+ * rediscovering it -- no test file moves extension_drop to a different
+ * schema mid-run, so the value set at the top of the session is still
+ * accurate here.
+ *
+ * existing mode is the one exception: there, extension_drop was installed
+ * by a real pg_upgrade external to this suite (see test/install/load.sql),
+ * landing wherever THAT process's own ambient defaults put it -- typically
+ * public, which stays on search_path by default. We have no control over
+ * or guarantee about that placement, so there's nothing to assert here in
+ * that mode; skip() still counts as one test, so no plan() changes needed
+ * either way.
+ */
+SELECT current_setting('extension_drop.test_load_mode') = 'existing' AS extension_drop_existing_mode
+\gset
+
+\if :extension_drop_existing_mode
+SELECT skip('existing mode: extension_drop''s placement is outside this suite''s control');
+\else
+SELECT is(
+  current_schemas(true) @> array[:'extension_drop_schema'::name]
+  , false
+  , 'extension_drop''s schema should not be reachable via search_path'
+);
+\endif
+
+\i test/pgxntool/finish.sql

test/install/load.sql

@@ -10,11 +10,6 @@
  * into every (rolled-back) test/sql/ file instead of each one re-installing
  * it from scratch. test/deps.sql (run per test) only sets the psql
  * variables the suite references; it does not install anything itself.
- * 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
@@ -41,12 +36,23 @@
  *
  * 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.
+ * control file has no schema= line, so it is genuinely install-schema-
+ * flexible -- nothing in its own SQL may assume where it landed. fresh/update
+ * mode exploits that on every single run below: each install targets a
+ * freshly created schema with a randomly generated name (never public,
+ * never the same name twice in a row), so a hardcoded-schema bug in
+ * extension_drop's own SQL fails immediately and unconditionally, instead of
+ * only on the rare install that happens to land outside the default
+ * search_path. The random schema is targeted directly via CREATE EXTENSION
+ * ... SCHEMA, never by first mutating search_path -- that would let an
+ * install succeed via a coincidentally arranged search_path and mask
+ * exactly the kind of qualification bug this exists to catch. existing mode
+ * (below) does not create anything -- it only asserts against whatever a
+ * real pg_upgrade already produced, wherever that happened to land.
+ * The random schema is deliberately never added to search_path either --
+ * see test/deps.sql, which rediscovers it fresh instead of trusting a
+ * search_path shortcut, and test/finish.sql, which asserts at the end of
+ * every test file that it never ended up on search_path regardless.
  */
 SET client_min_messages = WARNING;
 
@@ -120,10 +126,70 @@ DROP EXTENSION IF EXISTS extension_drop CASCADE;
 SELECT current_setting('server_version_num')::int >= 100000 AS extension_drop_pg10_plus
 \gset
 
--- update mode: install at the OLD version, then ALTER EXTENSION UPDATE below.
-\if :extension_drop_mode_update
+/*
+ * Fresh/update installs always target a fresh, randomly named schema --
+ * never public, never reused -- so nothing below can coast on landing in a
+ * predictable place. The constant prefix (including its trailing space)
+ * already forces identifier quoting on its own, before the random suffix
+ * is even appended -- unlike a mixed-case-only name, which would only force
+ * quoting by coincidence of which characters the randomness happened to
+ * produce.
+ *
+ * Cleanup-before-create: a prior run that crashed before reaching its own
+ * teardown would otherwise leave its randomly-named schema behind forever,
+ * since nothing else knows that name to find and drop it later. Matching on
+ * the constant prefix finds and drops any such leftovers before generating
+ * this run's own name.
+ */
+DO $DO$
+DECLARE
+  r record;
+BEGIN
+  FOR r IN SELECT nspname FROM pg_namespace WHERE nspname ~ '^extension_drop test schema ' LOOP
+    EXECUTE format('DROP SCHEMA %I CASCADE', r.nspname);
+  END LOOP;
+END
+$DO$;
+
+SELECT 'extension_drop test schema ' || substr(md5(random()::text), 1, 12) AS extension_drop_test_schema
+\gset
+
+CREATE SCHEMA :"extension_drop_test_schema";
+
+/*
+ * CASCADE auto-installs cat_tools on PG10+; pre-PG10 needs it created
+ * explicitly first instead (CREATE EXTENSION ... CASCADE was only added in
+ * PG10, though event triggers themselves exist from 9.3).
+ */
+\if :extension_drop_pg10_plus
+\else
+CREATE EXTENSION IF NOT EXISTS cat_tools;
+\endif
+
+-- Read unconditionally; empty and unused outside update mode.
 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
+
+/*
+ * One combined suffix covers every fresh/update x pre/post-PG10
+ * combination -- SCHEMA is always present, VERSION only in update mode,
+ * CASCADE only on PG10+ -- so a SINGLE CREATE EXTENSION statement below
+ * handles all four cases instead of duplicating it once per combination.
+ */
+SELECT
+    format(' SCHEMA %I', :'extension_drop_test_schema')
+    || CASE WHEN :'extension_drop_mode_update'::boolean
+              THEN format(' VERSION %L', :'extension_drop_test_update_from')
+              ELSE ''
+       END
+    || CASE WHEN :'extension_drop_pg10_plus'::boolean THEN ' CASCADE' ELSE '' END
+  AS extension_drop_create_suffix
+\gset
+
+CREATE EXTENSION extension_drop:extension_drop_create_suffix;
+
+-- update mode only: bring the just-installed old version up to date.
+\if :extension_drop_mode_update
 /*
  * Build the optional target clause once so a SINGLE ALTER EXTENSION covers
  * both cases: an empty test_update_to yields '' (update to the current
@@ -134,28 +200,13 @@ 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;
--- fresh mode: plain CREATE EXTENSION at the current version.
-\else
-\if :extension_drop_pg10_plus
-CREATE EXTENSION extension_drop CASCADE;
-\else
-CREATE EXTENSION IF NOT EXISTS cat_tools;
-CREATE EXTENSION extension_drop;
-\endif
--- end \if :extension_drop_mode_update (fresh vs. update install branch)
+-- end \if :extension_drop_mode_update (update-mode-only ALTER EXTENSION UPDATE)
 \endif
 -- end \if :extension_drop_mode_existing (existing mode skips the whole (re)install block)
 \endif