Restore sql/test_factory--0.5.0.sql to its true historical content

.gitignore

@@ -18,6 +18,13 @@ control.mk
 
 # built targets
 # Note: Version-specific files (sql/*--*.sql) are now tracked in git and should be committed
+# Exception: the `stable` pseudo-version (default_version between releases,
+# see ../ai/RELEASE.md and ../ai/CLAUDE.md's "Version-specific SQL files")
+# is permanently current, not a frozen release -- it would be regenerated
+# and re-diffed on every source edit for zero test-coverage value if
+# tracked. The update script *to* stable (sql/*--<last-release>--stable.sql)
+# is not affected by this and must stay committed.
+sql/*--stable.sql
 
 # Generated by asciidoctor from doc/*.asc; only the .asc source is tracked
 doc/*.html

bin/test_existing

@@ -26,12 +26,12 @@
 #
 #   run-suite DB
 #       Assert the installed version matches the current build (never
-#       hardcoded -- derived from `make -s print-PGXNVERSION`, so a broken
-#       extraction can't silently compare "" to ""), then run the suite in
-#       existing mode via --use-existing (pg_regress must not drop/recreate
-#       DB) and gate on verify-results, not a bare `make test` (pgxntool
-#       marks installcheck .IGNORE, so a plain test run exits 0 even when
-#       regression.diffs is nonempty).
+#       hardcoded -- derived from `make -s print-EXTENSION_test_factory_VERSION`,
+#       so a broken extraction can't silently compare "" to ""), then run
+#       the suite in existing mode via --use-existing (pg_regress must not
+#       drop/recreate DB) and gate on verify-results, not a bare `make test`
+#       (pgxntool marks installcheck .IGNORE, so a plain test run exits 0
+#       even when regression.diffs is nonempty).
 #
 # No separate guard-planting subcommand: unlike cat_tools, test_factory's
 # dependency guard (a view in schema test_factory_drop_guard depending on
@@ -57,8 +57,15 @@ psql_do() {
   psql -d "$db" -v ON_ERROR_STOP=1 "$@"
 }
 
+# EXTENSION_test_factory_VERSION, not PGXNVERSION: the latter is the
+# *distribution* version from META.json, which no longer tracks what
+# CREATE EXTENSION actually installs now that test_factory.control's
+# default_version sits at the literal 'stable' pseudo-version between
+# releases (see ../ai/RELEASE.md) -- a bare CREATE EXTENSION always
+# resolves to whatever default_version says, not the distribution's
+# semver.
 current_version() {
-  make -s print-PGXNVERSION 2>/dev/null | sed -n 's/.*set to "\(.*\)"$/\1/p'
+  make -s print-EXTENSION_test_factory_VERSION 2>/dev/null | sed -n 's/.*set to "\(.*\)"$/\1/p'
 }
 
 installed_version() {

sql/test_factory--0.5.0--stable.sql

@@ -0,0 +1,50 @@
+/*
+ * https://github.com/Postgres-Extensions/test_factory/issues/14: whoever
+ * originally installed 0.5.0 never automatically got SET-enabled (PG16+)
+ * or even plain (pre-16) membership in test_factory__owner -- that's the
+ * bug, and it applies just as much to an already-existing 0.5.0 install
+ * as to a fresh one (see sql/test_factory.sql's own comment for the fresh
+ * case). This grants it retroactively, so whoever runs this update -- and
+ * anyone else who later needs to SET ROLE test_factory__owner -- has it.
+ *
+ * No object in this extension changed between 0.5.0 and here, so unlike
+ * the fresh-install script, there's nothing to create or alter AS
+ * test_factory__owner, and so no role to switch to or restore.
+ */
+DO $body$
+BEGIN
+	IF current_setting('server_version_num')::int >= 160000 THEN
+		IF NOT pg_has_role(current_user, 'test_factory__owner', 'SET') THEN
+			BEGIN
+				EXECUTE format('GRANT test_factory__owner TO %I WITH SET TRUE', current_user);
+			EXCEPTION
+				WHEN insufficient_privilege THEN
+					RAISE EXCEPTION
+						'role "%" lacks SET-enabled membership in "test_factory__owner", and lacks ADMIN OPTION to grant it to itself'
+						, current_user
+						USING ERRCODE = 'insufficient_privilege'
+						, HINT = format(
+							'Ask a superuser, or a role with ADMIN OPTION on "test_factory__owner", to run: %s'
+							, format('GRANT test_factory__owner TO %I WITH SET TRUE;', current_user)
+						);
+			END;
+		END IF;
+	ELSIF NOT pg_has_role(current_user, 'test_factory__owner', 'MEMBER') THEN
+		BEGIN
+			EXECUTE format('GRANT test_factory__owner TO %I', current_user);
+		EXCEPTION
+			WHEN insufficient_privilege THEN
+				RAISE EXCEPTION
+					'role "%" is not a member of "test_factory__owner", and lacks ADMIN OPTION to grant it to itself'
+					, current_user
+					USING ERRCODE = 'insufficient_privilege'
+					, HINT = format(
+						'Ask a superuser, or a role with ADMIN OPTION on "test_factory__owner", to run: %s'
+						, format('GRANT test_factory__owner TO %I;', current_user)
+					);
+		END;
+	END IF;
+END
+$body$;
+
+-- vi: expandtab ts=2 sw=2

sql/test_factory--0.5.0.sql

@@ -6,7 +6,6 @@
  * it's much lighter weight than creating a table.
  */
 SELECT pg_catalog.set_config('test_factory.original_role', current_user, true);
-
 DO $body$
 BEGIN
 	CREATE ROLE test_factory__owner;
@@ -16,86 +15,6 @@ EXCEPTION
 END
 $body$;
 
-/*
- * CREATE ROLE alone never grants the creator any relationship to the role
- * it just created -- on ANY version, confirmed directly against a genuine
- * non-superuser CREATEROLE installer with zero prior grants: SET ROLE
- * test_factory__owner below fails outright ("permission denied to set
- * role") without an explicit GRANT first, even immediately after this
- * same session created it. A real superuser bypasses the SET ROLE check
- * entirely regardless of any of this, which is why this was easy to miss
- * without testing against a genuine non-superuser role.
- *
- * As of PG16, plain membership isn't enough either -- SET ROLE requires
- * the SET option specifically, which needs GRANT ... WITH SET TRUE.
- * Before PG16, plain membership (no WITH SET syntax, which doesn't exist
- * yet) already confers the ability to SET ROLE. pg_has_role's 'SET'
- * privilege type is PG16+ only, but as a plain function argument (not a
- * catalog column reference) it's safe to call inside this same
- * version-gated branch, unlike a static reference to
- * pg_auth_members.set_option, which would fail to parse on older servers
- * even inside the gate.
- *
- * Either way, skip the GRANT when it's not actually needed: an installer
- * might already have the membership it needs some other way -- e.g. a DBA
- * pre-provisioned the role and granted it directly, deliberately
- * withholding ADMIN OPTION as a least-privilege measure -- and GRANT ROLE
- * requires ADMIN OPTION on the target role (or superuser) to run AT ALL,
- * regardless of whether it would end up a no-op; forcing it unconditionally
- * broke that already-working case (confirmed against a live non-superuser
- * role with SET but not ADMIN OPTION: "ERROR: permission denied to grant
- * role ... Only roles with the ADMIN option ... may grant this role").
- *
- * If the installer has neither the membership it needs nor ADMIN OPTION to
- * grant it themselves, installation genuinely cannot proceed -- nobody
- * else can do it on their behalf from inside this script. Catch that
- * specific case and say so plainly instead of surfacing Postgres's generic
- * permission error.
- */
-DO $body$
-BEGIN
-	IF current_setting('server_version_num')::int >= 160000 THEN
-		IF NOT pg_has_role(current_user, 'test_factory__owner', 'SET') THEN
-			BEGIN
-				EXECUTE format('GRANT test_factory__owner TO %I WITH SET TRUE', current_user);
-			EXCEPTION
-				WHEN insufficient_privilege THEN
-					/*
-					 * RAISE's own %-substitution is plain string
-					 * interpolation, not format()'s %I/%L -- build the
-					 * copy-pastable suggested command with format() first
-					 * (so the role name is properly identifier-quoted),
-					 * then substitute the whole result in with a single,
-					 * ordinary %.
-					 */
-					RAISE EXCEPTION
-						'role "%" lacks SET-enabled membership in "test_factory__owner", and lacks ADMIN OPTION to grant it to itself'
-						, current_user
-						USING ERRCODE = 'insufficient_privilege'
-						, HINT = format(
-							'Ask a superuser, or a role with ADMIN OPTION on "test_factory__owner", to run: %s'
-							, format('GRANT test_factory__owner TO %I WITH SET TRUE;', current_user)
-						);
-			END;
-		END IF;
-	ELSIF NOT pg_has_role(current_user, 'test_factory__owner', 'MEMBER') THEN
-		BEGIN
-			EXECUTE format('GRANT test_factory__owner TO %I', current_user);
-		EXCEPTION
-			WHEN insufficient_privilege THEN
-				RAISE EXCEPTION
-					'role "%" is not a member of "test_factory__owner", and lacks ADMIN OPTION to grant it to itself'
-					, current_user
-					USING ERRCODE = 'insufficient_privilege'
-					, HINT = format(
-						'Ask a superuser, or a role with ADMIN OPTION on "test_factory__owner", to run: %s'
-						, format('GRANT test_factory__owner TO %I;', current_user)
-					);
-		END;
-	END IF;
-END
-$body$;
-
 CREATE SCHEMA tf AUTHORIZATION test_factory__owner;
 COMMENT ON SCHEMA tf IS $$Test factory. Tools for maintaining test data.$$;
 GRANT USAGE ON SCHEMA tf TO public;

test/install/load.sql

@@ -246,39 +246,30 @@ SELECT :'load_mode' = 'existing' AS is_existing
   DROP EXTENSION IF EXISTS test_factory_pgtap CASCADE;
   DROP EXTENSION IF EXISTS test_factory CASCADE;
 
-  -- Everything from here on -- the actual install this mode exists to
-  -- test -- runs as the installer instead.
-  SET SESSION AUTHORIZATION :installer_role;
-
-  /*
-   * pgtap must land in a dedicated "tap" schema BEFORE test_factory_pgtap
-   * installs, not after. test_factory_pgtap.control declares pgtap as a
-   * requirement too, so the CREATE EXTENSION ... CASCADE (or plain CREATE
-   * EXTENSION, in update mode) below would otherwise cascade-install pgtap
-   * itself into whatever schema this session's ambient search_path
-   * resolves to (public, by default) -- and since that satisfies "pgtap is
-   * already installed", the main suite's own tap_setup.sql (which does
-   * CREATE EXTENSION IF NOT EXISTS pgtap SCHEMA tap) then skips creating it
-   * in "tap" at all, leaving every pgTAP-based test file failing with
-   * "function no_plan() does not exist" (hit this for real writing this
-   * file). IF NOT EXISTS on both statements: harmless no-op on a rerun
-   * where a previous pass already did this and the drop-first reset above
-   * didn't touch it (cascading test_factory_pgtap's drop doesn't remove
-   * pgtap -- pgtap is its dependency, not the other way around).
-   */
-  CREATE SCHEMA IF NOT EXISTS tap;
-  CREATE EXTENSION IF NOT EXISTS pgtap SCHEMA tap;
-
-  -- Captured before either branch below runs CREATE EXTENSION, so the
-  -- role-restore proof after \endif covers whichever one actually ran.
-  SELECT current_user AS role_before_install
-  \gset
-
   SELECT :'load_mode' = 'update' AS is_update
   \gset
   -- update-vs-fresh install
   \if :is_update
 
+    /*
+     * update mode runs entirely as the ambient role, not
+     * test_factory_installer: extension ownership can't be transferred
+     * (`ALTER EXTENSION ... OWNER TO` isn't valid syntax -- confirmed),
+     * so only the role that ran the ORIGINAL CREATE EXTENSION can ever
+     * run ALTER EXTENSION UPDATE on it. There's no reachable "a
+     * different non-superuser applies this later" scenario to test here
+     * -- whether the original install itself could have been done
+     * non-superuser is exactly what the fresh branch below already
+     * proves.
+     */
+    CREATE SCHEMA IF NOT EXISTS tap;
+    CREATE EXTENSION IF NOT EXISTS pgtap SCHEMA tap;
+
+    -- Captured before CREATE EXTENSION, so the role-restore proof below
+    -- covers it too.
+    SELECT current_user AS role_before_install
+    \gset
+
     CREATE EXTENSION test_factory VERSION :'update_from';
     SET client_min_messages = ERROR; -- suppress update-script deprecation NOTICEs
     \if :has_update_to
@@ -297,19 +288,86 @@ SELECT :'load_mode' = 'existing' AS is_existing
   -- update-vs-fresh install
   \else
 
+    -- Everything from here on -- the actual install this mode exists to
+    -- test -- runs as the installer instead.
+    SET SESSION AUTHORIZATION :installer_role;
+
+    /*
+     * pgtap must land in a dedicated "tap" schema BEFORE test_factory_pgtap
+     * installs, not after. test_factory_pgtap.control declares pgtap as a
+     * requirement too, so the CREATE EXTENSION ... CASCADE below would
+     * otherwise cascade-install pgtap itself into whatever schema this
+     * session's ambient search_path resolves to (public, by default) --
+     * and since that satisfies "pgtap is already installed", the main
+     * suite's own tap_setup.sql (which does CREATE EXTENSION IF NOT
+     * EXISTS pgtap SCHEMA tap) then skips creating it in "tap" at all,
+     * leaving every pgTAP-based test file failing with "function
+     * no_plan() does not exist" (hit this for real writing this file).
+     * IF NOT EXISTS on both statements: harmless no-op on a rerun where a
+     * previous pass already did this and the drop-first reset above
+     * didn't touch it (cascading test_factory_pgtap's drop doesn't
+     * remove pgtap -- pgtap is its dependency, not the other way
+     * around).
+     */
+    CREATE SCHEMA IF NOT EXISTS tap;
+    CREATE EXTENSION IF NOT EXISTS pgtap SCHEMA tap;
+
+    -- Captured before CREATE EXTENSION, so the role-restore proof below
+    -- covers it too.
+    SELECT current_user AS role_before_install
+    \gset
+
     /*
-     * fresh: install for real, right here, uniformly with update/existing --
-     * so test/sql/base.sql and test/sql/pgtap.sql can assume both
-     * extensions are already present in every mode, instead of each mode
-     * needing its own install-or-skip dance (what test/helpers/
-     * create_extension.sql used to do; deleted along with this change).
      * CASCADE proves test_factory_pgtap.control's "requires = 'pgtap,
      * test_factory'" line actually pulls test_factory in --
      * test/sql/pgtap.sql separately proves the dependency is *enforced*
      * (via pg_depend), not just that cascade happens to work.
      */
     CREATE EXTENSION test_factory_pgtap CASCADE;
 
+    /*
+     * Regression test for https://github.com/Postgres-Extensions/test_factory/issues/14,
+     * checked from THIS session (still test_factory_installer, per the
+     * SET SESSION AUTHORIZATION above) rather than from
+     * test/sql/base.sql's own separate connection, which would only ever
+     * see whatever role CI or a developer happens to connect as --
+     * pg_has_role(current_user, ...) is unconditionally true for a
+     * superuser regardless of whether the fix's GRANT logic in
+     * sql/test_factory.sql ever ran. test_factory_installer is freshly
+     * created every run with zero prior relationships, so it can only
+     * have this membership if that GRANT actually fired -- there's no
+     * other way it could already be there. Only checked here, not in
+     * update mode above: update mode never switches to
+     * test_factory_installer (see its own comment), so current_user
+     * there is whatever ambient role connected -- typically a superuser,
+     * for whom this would be checking nothing.
+     *
+     * pg_has_role's 'SET' privilege type is PG16+ only (same reasoning as
+     * sql/test_factory.sql's own comment); pre-16, plain membership is
+     * the closest equivalent ('SET' isn't a meaningful distinction yet --
+     * membership itself already confers the ability to SET ROLE).
+     */
+    SELECT (current_setting('server_version_num')::int >= 160000) AS pg16plus
+    \gset
+    -- pg16+ SET-enabled membership check
+    \if :pg16plus
+    DO $$
+    BEGIN
+      IF NOT pg_has_role(current_user, 'test_factory__owner', 'SET') THEN
+        RAISE EXCEPTION 'issue #14 regression: % lacks SET-enabled membership in test_factory__owner after install', current_user;
+      END IF;
+    END $$;
+    -- pg16+ SET-enabled membership check
+    \else
+    DO $$
+    BEGIN
+      IF NOT pg_has_role(current_user, 'test_factory__owner', 'MEMBER') THEN
+        RAISE EXCEPTION 'issue #14 regression: % lacks membership in test_factory__owner after install', current_user;
+      END IF;
+    END $$;
+    -- pg16+ SET-enabled membership check
+    \endif
+
   -- update-vs-fresh install
   \endif
 
@@ -333,44 +391,6 @@ SELECT :'load_mode' = 'existing' AS is_existing
   DO $$ BEGIN RAISE EXCEPTION 'CREATE EXTENSION did not restore the calling role'; END $$;
   \endif
 
-  /*
-   * Regression test for https://github.com/Postgres-Extensions/test_factory/issues/14,
-   * checked from THIS session (still test_factory_installer, per the SET
-   * SESSION AUTHORIZATION above) rather than from test/sql/base.sql's own
-   * separate connection, which would only ever see whatever role CI or a
-   * developer happens to connect as -- pg_has_role(current_user, ...) is
-   * unconditionally true for a superuser regardless of whether the fix's
-   * GRANT logic in sql/test_factory.sql ever ran. test_factory_installer is
-   * freshly created every run with zero prior relationships, so it can only
-   * have this membership if that GRANT actually fired -- there's no other
-   * way it could already be there.
-   *
-   * pg_has_role's 'SET' privilege type is PG16+ only (same reasoning as
-   * sql/test_factory.sql's own comment); pre-16, plain membership is the
-   * closest equivalent ('SET' isn't a meaningful distinction yet --
-   * membership itself already confers the ability to SET ROLE).
-   */
-  SELECT (current_setting('server_version_num')::int >= 160000) AS pg16plus
-  \gset
-  -- pg16+ SET-enabled membership check
-  \if :pg16plus
-  DO $$
-  BEGIN
-    IF NOT pg_has_role(current_user, 'test_factory__owner', 'SET') THEN
-      RAISE EXCEPTION 'issue #14 regression: % lacks SET-enabled membership in test_factory__owner after install', current_user;
-    END IF;
-  END $$;
-  -- pg16+ SET-enabled membership check
-  \else
-  DO $$
-  BEGIN
-    IF NOT pg_has_role(current_user, 'test_factory__owner', 'MEMBER') THEN
-      RAISE EXCEPTION 'issue #14 regression: % lacks membership in test_factory__owner after install', current_user;
-    END IF;
-  END $$;
-  -- pg16+ SET-enabled membership check
-  \endif
-
 -- existing-vs-fresh/update
 \endif
 

test_factory.control

@@ -1,5 +1,5 @@
 comment = 'A framework for managing test data'
-default_version = '0.5.0'
+default_version = 'stable'
 relocatable = false
 # Not a security boundary weakening: test_factory__owner is a locked-down,
 # dedicated owner role, and every privileged function is SECURITY DEFINER