Always install count_nulls into a fresh, randomly named schema for tests

.github/workflows/ci.yml

@@ -35,15 +35,9 @@
 #                              through AWS pg_tle's database-backed catalog
 #                              instead of a filesystem .control file.
 #
-# Every TEST_SCHEMA value (empty - no schema targeting at all - and
-# 'Quoted', a name requiring SQL identifier quoting) is exercised too, via
-# `make test-schema-all`'s in-Makefile loop rather than a CI matrix
-# dimension - a schema name is just an input the same assertions run
-# against, not a real environment difference, so crossing it into the
-# matrix would only multiply job count for no added confidence (see the
-# Makefile's TEST_SCHEMA_VALUES comment). Every leg passes against the SAME
-# test/expected/extension_tests.out (see test/README.md for how the suite
-# keeps its output schema-invariant).
+# During testing we always install into a freshly, randomly generated
+# schema (see test/install/load.sql; test/README.md covers how
+# test/expected/extension_tests.out stays schema-invariant).
 #
 # `changes` is a cheap gate that lets the heavy jobs above skip themselves on
 # doc-only pushes, and also derives the shared PostgreSQL-major list those
@@ -251,16 +245,10 @@ jobs:
   # UPDATEs to current (no pg_upgrade, same PostgreSQL) and reruns the
   # suite - a single job rather than a dedicated one, since a load mode is
   # just an input the same assertions run against, not a real environment
-  # difference (same reasoning as TEST_SCHEMA below), and the per-version
-  # container/checkout setup would otherwise be duplicated across two jobs
-  # with the same PG matrix. Every TEST_SCHEMA value (empty - no schema
-  # targeting at all - and 'Quoted', a name requiring SQL identifier
-  # quoting) is exercised too, via `make test-schema-all`'s in-Makefile
-  # loop rather than a CI matrix dimension - a schema name is just an
-  # input the same assertions run against, not a real environment
-  # difference, so crossing it into the matrix would only multiply job
-  # count for no added confidence (see the Makefile's TEST_SCHEMA_VALUES
-  # comment). Every leg passes against the SAME
+  # difference, and the per-version container/checkout setup would
+  # otherwise be duplicated across two jobs with the same PG matrix. Each
+  # install (both legs) always targets its own freshly, randomly generated
+  # schema (see test/install/load.sql), so both legs pass against the SAME
   # test/expected/extension_tests.out (see test/README.md for how the
   # suite keeps its output schema-invariant).
   test:
@@ -284,8 +272,8 @@ jobs:
         run: pg-start ${{ matrix.pg }}
       - name: Check out the repo
         uses: actions/checkout@v4
-      - name: Test on PostgreSQL ${{ matrix.pg }}, across every TEST_SCHEMA value
-        run: make test-schema-all
+      - name: Test on PostgreSQL ${{ matrix.pg }}
+        run: make test
       - name: Install count_nulls
         run: make install
       - name: Update 0.9.6 -> current and run the suite
@@ -300,10 +288,8 @@ jobs:
   # A single old_pg/new_pg leg (old_pg from the changes job's floor_pg
   # output, new_pg from its newest_pg output - see that job's comment; NOT
   # a second/third hardcoded copy of either number), which via TWIN
-  # databases in the SAME cluster, migrated by a SINGLE pg_upgrade call
-  # (the same trick this file uses to cross TEST_SCHEMA without doubling
-  # matrix legs elsewhere), compares BOTH update-vs-pg_upgrade orderings a
-  # real user could hit:
+  # databases in the SAME cluster, migrated by a SINGLE pg_upgrade call,
+  # compares BOTH update-vs-pg_upgrade orderings a real user could hit:
   #   upgrade_oldest_first  -- stays at 0.9.6 through the
   #     binary pg_upgrade, updated to current AFTER it (on the NEW
   #     cluster). Proves pg_upgrade itself correctly preserves objects
@@ -341,9 +327,6 @@ jobs:
   # functions over anyarray/json/jsonb, nothing version-sensitive to
   # break at a specific boundary. Revisit if count_nulls ever grows
   # something catalog-touching.
-  #
-  # Not yet crossed with TEST_SCHEMA (a later phase adds that, once it can
-  # do so for both this job and the test job's update leg together).
   pg-upgrade-test:
     needs: [changes]
     # Skipped outright (not just matrix-reduced like `test`) on a draft PR:
@@ -391,8 +374,8 @@ jobs:
         # anywhere in this job cannot silently make the eventual
         # existing-mode run test a fresh install instead.
         run: |
-          bin/test_existing prepare-old upgrade_oldest_first "" 0.9.6
-          bin/test_existing prepare-old upgrade_current_first "" 0.9.6
+          bin/test_existing prepare-old upgrade_oldest_first 0.9.6
+          bin/test_existing prepare-old upgrade_current_first 0.9.6
       - name: Update to current before pg_upgrade (proves pg_upgrade preserves CURRENT objects)
         # Updates ONLY upgrade_current_first
         # (upgrade_oldest_first must NOT be touched here - it
@@ -455,8 +438,8 @@ jobs:
         # test a fresh install instead of the migrated objects. Once per
         # database, since each holds an independent ordering's result.
         run: |
-          bin/test_existing run-suite upgrade_oldest_first ""
-          bin/test_existing run-suite upgrade_current_first ""
+          bin/test_existing run-suite upgrade_oldest_first
+          bin/test_existing run-suite upgrade_current_first
 
   pg-tle-test:
     needs: [changes]
@@ -585,11 +568,11 @@ jobs:
 
   # A single stable check name for use as a required status check in branch
   # protection rules. Matrix jobs produce check names like
-  # "🐘 PostgreSQL 14 (schema none)" which would all need to be listed
-  # individually and updated whenever the matrix changes. This job passes if
-  # all others passed or were skipped (e.g. the heavy jobs gated off by the
-  # `changes` job on a docs-only push), and fails if any failed or were
-  # cancelled.
+  # "🐘 PostgreSQL 14" (one per supported major) which would all need to be
+  # listed individually and updated whenever the matrix changes. This job
+  # passes if all others passed or were skipped (e.g. the heavy jobs gated
+  # off by the `changes` job on a docs-only push), and fails if any failed
+  # or were cancelled.
   all-checks-passed:
     needs: [changes, lint, test, pg-upgrade-test, pg-tle-test]
     if: always()

Makefile

@@ -12,64 +12,6 @@ testdeps: $(wildcard test/*/*.sql) $(wildcard test/*.sql) # Be careful not to in
 LINT_TARGETS = sql/count_nulls.sql test/
 include lint.mk
 
-# TEST_SCHEMA selects which schema test/install/load.sql installs count_nulls
-# into, for the WHOLE test run (every test file sees the SAME schema in a
-# given run).
-#
-# Empty (the default): don't target any schema at all - count_nulls installs
-# wherever the session's own default search_path already resolves. Non-empty:
-# explicitly CREATE SCHEMA, then CREATE EXTENSION ... WITH SCHEMA that name -
-# including a name that requires SQL identifier quoting (mixed case -
-# unquoted would fold to lowercase), to exercise the suite's %I
-# schema-qualification rather than just its literal test data. Locally:
-# `make test TEST_SCHEMA=Quoted`.
-#
-# Installing into two schemas only proves that %I-qualification works if the
-# test session's search_path never includes count_nulls' own schema in at
-# least one of those legs - otherwise an extension full of unqualified,
-# resolve-by-accident references would pass every leg too (see
-# test/core/functions.sql's header and test__check_ncs in
-# test/sql/extension_tests.sql, which is what actually checks this). This
-# suite excludes it in BOTH legs, which is stronger than the minimum needed -
-# not a requirement in itself.
-#
-# Propagated as a GUC (count_nulls.test_schema), exported unconditionally via
-# PGOPTIONS - pg_regress doesn't forward make variables, but the psql
-# processes it spawns inherit the environment. Empty is a valid, deliberate
-# value (not an error) - read without missing_ok, so a truly unpropagated GUC
-# still fails loudly instead of looking identical to a deliberately empty one.
-TEST_SCHEMA ?=
-export PGOPTIONS := $(PGOPTIONS) -c count_nulls.test_schema=$(TEST_SCHEMA)
-
-# Every TEST_SCHEMA value the suite is tested against. A single source so
-# test-schema-all/test-update-schema-all and CI can't silently drift onto
-# different sets. See the TEST_SCHEMA comment above for why exercising more
-# than one value here is meaningful (search_path exclusion), not just
-# "install into schema A vs schema B".
-TEST_SCHEMA_VALUES = "" Quoted
-
-# TEST_SCHEMA is deliberately NOT a CI matrix dimension: unlike PostgreSQL
-# major (a real environment difference - different binaries, different
-# container) or pg_tle deployment (a real isolation boundary - must never
-# share a runner with a filesystem install), a schema name is just an input
-# value the SAME assertions run against in the SAME environment. Crossing it
-# into the matrix would only multiply job count (container boot + checkout
-# per leg) for zero additional confidence per dollar. Loop it inside make
-# instead - the same pattern test-update already uses for the load-mode
-# axis, generalized to a list via a shell loop. Sequential recursive $(MAKE)
-# calls, deliberately NOT bare prerequisites (which Make can run
-# concurrently under -j and would collide on the same throwaway test
-# database). `exit 1` on the first failure so a later iteration can't hide
-# an earlier one; each iteration is echoed so a failure's TEST_SCHEMA value
-# is still directly attributable in the log even without a separate CI
-# check name per value.
-.PHONY: test-schema-all
-test-schema-all:
-	@for schema in $(TEST_SCHEMA_VALUES); do \
-		echo "=== TEST_SCHEMA=$$schema ==="; \
-		$(MAKE) test TEST_SCHEMA="$$schema" || exit 1; \
-	done
-
 # TEST_LOAD_SOURCE selects how test/install/load.sql installs count_nulls
 # for the WHOLE test run:
 #   - fresh (default): CREATE EXTENSION count_nulls (current version).
@@ -87,8 +29,10 @@ test-schema-all:
 # "update" (this) is extension-level (ALTER EXTENSION UPDATE); "upgrade" is
 # cluster-level (pg_upgrade) - 'existing' is how that axis is exercised.
 #
-# Propagated the same way as TEST_SCHEMA: via the count_nulls.test_load_mode
-# GUC, exported unconditionally through PGOPTIONS, read without missing_ok.
+# Propagated as a GUC (count_nulls.test_load_mode), exported unconditionally
+# through PGOPTIONS - pg_regress doesn't forward make variables, but the
+# psql processes it spawns inherit the environment. Read without missing_ok:
+# a genuinely unpropagated GUC must fail loudly, not look like a valid value.
 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)')

bin/test_existing

@@ -32,14 +32,16 @@
 #
 # USAGE: bin/test_existing <subcommand> [args]
 #
-#   prepare-old DB SCHEMA INSTALL_VERSION
+#   prepare-old DB INSTALL_VERSION
 #       Old-cluster prep for pg-upgrade-test: create DB + extension at
-#       INSTALL_VERSION in SCHEMA, then plant + prove the dependency guard.
+#       INSTALL_VERSION (in a freshly, randomly generated schema - see
+#       test/helpers/create_test_schema.sql), then plant + prove the
+#       dependency guard.
 #
 #   update DB [TO_VERSION]
 #       ALTER EXTENSION count_nulls UPDATE [TO 'TO_VERSION'] (empty => current).
 #
-#   run-suite DB SCHEMA
+#   run-suite DB
 #       Assert the current version, re-prove the guard, drop it, then run the
 #       suite in existing mode (extension must be at the current version).
 #
@@ -101,8 +103,8 @@ installed_version() {
 # Plant the guard and PROVE it blocks a non-CASCADE drop. Call right after
 # CREATE EXTENSION (and before any update/upgrade) so it persists through them.
 plant_guard() {
-  local db=$1 schema=$2
-  psql -d "$db" -v ON_ERROR_STOP=1 -v schema="$schema" -f bin/test_existing.sql/plant_guard.sql
+  local db=$1
+  psql -d "$db" -v ON_ERROR_STOP=1 -f bin/test_existing.sql/plant_guard.sql
   assert_drop_blocked "$db"
 }
 
@@ -140,35 +142,29 @@ update_ext() {
   psql_do "$db" -c "ALTER EXTENSION count_nulls UPDATE $to"
 }
 
-# CREATE EXTENSION count_nulls at VERSION, targeting SCHEMA - unless SCHEMA
-# is empty, in which case it's created untouched, wherever the session's
-# own default search_path resolves (ordinarily 'public'). A quoted empty
-# identifier ("") is a real Postgres syntax error, so this can't just always
-# emit `CREATE SCHEMA IF NOT EXISTS "$schema"` - the empty case has to skip
-# that entirely, mirroring test/install/load.sql's own :count_nulls_has_schema
-# branch.
+# CREATE EXTENSION count_nulls at VERSION, into a freshly, randomly
+# generated schema - shared with test/install/load.sql's own fresh/update
+# installs via test/helpers/create_test_schema.sql (see that file for the
+# full rationale).
 create_extension_in_schema() {
-  local db=$1 schema=$2 version=$3 sql=""
-  if [ -n "$schema" ]; then
-    sql="CREATE SCHEMA IF NOT EXISTS \"$schema\"; SET search_path = \"$schema\"; "
-  fi
-  psql_do "$db" -c "${sql}CREATE EXTENSION count_nulls VERSION '$version'"
+  local db=$1 version=$2
+  psql -d "$db" -v ON_ERROR_STOP=1 -v version="$version" -f test/helpers/create_test_schema.sql
 }
 
 # ---------------------------------------------------------------------------
 # Subcommand implementations
 # ---------------------------------------------------------------------------
 
-# prepare-old DB SCHEMA INSTALL_VERSION
+# prepare-old DB INSTALL_VERSION
 #   Old-cluster preparation for pg-upgrade-test: create the database and the
-#   extension at INSTALL_VERSION in SCHEMA, then plant + prove the guard. No
+#   extension at INSTALL_VERSION, then plant + prove the guard. No
 #   bridge-update step first: count_nulls ships no SELECT-*-over-catalog
 #   views, so it has no known pg_upgrade-unsafe old version to bridge past.
 prepare_old() {
-  local db=$1 schema=$2 install=$3
+  local db=$1 install=$2
   createdb "$db"
-  create_extension_in_schema "$db" "$schema" "$install"
-  plant_guard "$db" "$schema"
+  create_extension_in_schema "$db" "$install"
+  plant_guard "$db"
 }
 
 # Run the pgTAP suite against an already-populated database in existing mode.
@@ -179,7 +175,7 @@ prepare_old() {
 # then runs the suite via --use-existing so pg_regress does NOT drop/recreate
 # the database.
 run_suite() {
-  local db=$1 schema=$2
+  local db=$1
   assert_version "$db" current
   assert_drop_blocked "$db"
   drop_guard "$db"
@@ -188,14 +184,14 @@ run_suite() {
   # verify-results`) is a real gate as of pgxntool 2.3.0 - it now exits
   # non-zero on regression failures instead of always exiting 0 regardless
   # of pg_regress's result (see this repo's pgxntool 2.3.0 bump).
-  make test TEST_LOAD_SOURCE=existing TEST_SCHEMA="$schema" CONTRIB_TESTDB="$db" EXTRA_REGRESS_OPTS=--use-existing
+  make test TEST_LOAD_SOURCE=existing CONTRIB_TESTDB="$db" EXTRA_REGRESS_OPTS=--use-existing
 }
 
 usage() {
   echo "usage: bin/test_existing <subcommand> [args]" >&2
-  echo "  prepare-old DB SCHEMA INSTALL_VERSION" >&2
+  echo "  prepare-old DB INSTALL_VERSION" >&2
   echo "  update DB [TO_VERSION]" >&2
-  echo "  run-suite DB SCHEMA" >&2
+  echo "  run-suite DB" >&2
   exit 2
 }
 

bin/test_existing.sql/plant_guard.sql

@@ -8,23 +8,17 @@
  * otherwise fall through to a silent fresh reinstall and the job would
  * still report green).
  *
- * Usage: psql -v ON_ERROR_STOP=1 -v schema=<schema-or-empty> -f plant_guard.sql
- * (empty schema means "wherever null_count already resolves unqualified" -
- * i.e. count_nulls was installed without targeting a schema).
+ * count_nulls always installs into its own randomly generated schema (see
+ * test/helpers/create_test_schema.sql) - this session didn't create it, so
+ * it has no other way to know its name; test/helpers/find_test_schema.sql
+ * discovers it live via pg_namespace.
+ *
+ * Usage: psql -v ON_ERROR_STOP=1 -f plant_guard.sql
  */
 \set ON_ERROR_STOP on
 
-/*
- * schema_prefix: either empty, or the quoted schema name followed by a
- * literal '.' - so the view definition below is a single statement with a
- * plain (unquoted) substitution, rather than branching the whole CREATE
- * VIEW on whether a schema was given. quote_ident(), not :"schema" -
- * :schema_prefix is pasted as-is (unquoted substitution), so it must
- * already be valid, properly-quoted SQL text by the time it lands there.
- */
-SELECT CASE WHEN :'schema' <> '' THEN quote_ident(:'schema') || '.' ELSE '' END AS schema_prefix
-\gset
+\i test/helpers/find_test_schema.sql
 
 CREATE SCHEMA IF NOT EXISTS count_nulls_drop_guard;
 CREATE OR REPLACE VIEW count_nulls_drop_guard.guard AS
-  SELECT :schema_prefix null_count(NULL::int, NULL::int) AS guarded_member;
+  SELECT :"test_schema".null_count(NULL::int, NULL::int) AS guarded_member;