From 0e09c76007115ec2fe0c725cd6cd69859ac50c20 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Tue, 14 Jul 2026 17:24:35 -0500 Subject: [PATCH 1/3] Fix issues #12/#13: drop the original_role temp table from install Both install scripts saved the caller's role in a CREATE TEMP TABLE ... ON COMMIT DROP so they could SET LOCAL ROLE to own objects and restore afterward. That temp table becomes extension-owned, so ON COMMIT DROP conflicts with extension ownership (uninstallable / COMMIT failures, #12/#13), and it collides if the caller already has an original_role temp table. Save the role in a transaction-local GUC via set_config() instead -- no extension-owned object. Co-Authored-By: Claude Opus 4.8 (1M context) (cherry picked from commit 4c688a472328ad6f2f6eaf1b963e023cf5553d22) --- sql/test_factory--0.5.0.sql | 15 ++++++--------- sql/test_factory.sql | 15 ++++++--------- sql/test_factory_pgtap--0.1.0.sql | 15 ++++++--------- sql/test_factory_pgtap.sql | 15 ++++++--------- 4 files changed, 24 insertions(+), 36 deletions(-) diff --git a/sql/test_factory--0.5.0.sql b/sql/test_factory--0.5.0.sql index 636bcb5..3105145 100644 --- a/sql/test_factory--0.5.0.sql +++ b/sql/test_factory--0.5.0.sql @@ -1,7 +1,8 @@ /* DO NOT EDIT - AUTO-GENERATED FILE */ -CREATE TEMP TABLE original_role ON COMMIT DROP AS SELECT current_user AS original_role; - -GRANT SELECT ON pg_temp.original_role TO public; +-- Save the caller's role so we can restore it at the end (we SET LOCAL ROLE +-- below to own our objects). Stashed in a transaction-local GUC rather than a +-- temp table, which would be owned by the extension and break CREATE EXTENSION. +SELECT pg_catalog.set_config('test_factory.original_role', current_user, true); DO $body$ BEGIN CREATE ROLE test_factory__owner; @@ -250,15 +251,11 @@ END $body$; --select (tf.get('moo','moo')::moo).*; +-- Restore the caller's role saved at the top of this script. DO $body$ -DECLARE - c_sql CONSTANT text := 'SET ROLE ' || (SELECT original_role FROM pg_temp.original_role); BEGIN - --RAISE WARNING 'c_sql = %', c_sql; - EXECUTE c_sql; + EXECUTE 'SET ROLE ' || pg_catalog.quote_ident(pg_catalog.current_setting('test_factory.original_role')); END $body$; -DROP TABLE pg_temp.original_role; - -- vi: expandtab ts=2 sw=2 diff --git a/sql/test_factory.sql b/sql/test_factory.sql index 3faf586..7a1efab 100644 --- a/sql/test_factory.sql +++ b/sql/test_factory.sql @@ -1,6 +1,7 @@ -CREATE TEMP TABLE original_role ON COMMIT DROP AS SELECT current_user AS original_role; - -GRANT SELECT ON pg_temp.original_role TO public; +-- Save the caller's role so we can restore it at the end (we SET LOCAL ROLE +-- below to own our objects). Stashed in a transaction-local GUC rather than a +-- temp table, which would be owned by the extension and break CREATE EXTENSION. +SELECT pg_catalog.set_config('test_factory.original_role', current_user, true); DO $body$ BEGIN CREATE ROLE test_factory__owner; @@ -249,15 +250,11 @@ END $body$; --select (tf.get('moo','moo')::moo).*; +-- Restore the caller's role saved at the top of this script. DO $body$ -DECLARE - c_sql CONSTANT text := 'SET ROLE ' || (SELECT original_role FROM pg_temp.original_role); BEGIN - --RAISE WARNING 'c_sql = %', c_sql; - EXECUTE c_sql; + EXECUTE 'SET ROLE ' || pg_catalog.quote_ident(pg_catalog.current_setting('test_factory.original_role')); END $body$; -DROP TABLE pg_temp.original_role; - -- vi: expandtab ts=2 sw=2 diff --git a/sql/test_factory_pgtap--0.1.0.sql b/sql/test_factory_pgtap--0.1.0.sql index 67e2a94..e4b8ea5 100644 --- a/sql/test_factory_pgtap--0.1.0.sql +++ b/sql/test_factory_pgtap--0.1.0.sql @@ -1,6 +1,8 @@ /* DO NOT EDIT - AUTO-GENERATED FILE */ -CREATE TEMP TABLE original_role ON COMMIT DROP AS SELECT current_user AS original_role; -GRANT SELECT ON pg_temp.original_role TO public; +-- Save the caller's role so we can restore it at the end (we SET LOCAL ROLE +-- below to own our objects). Stashed in a transaction-local GUC rather than a +-- temp table, which would be owned by the extension and break CREATE EXTENSION. +SELECT pg_catalog.set_config('test_factory_pgtap.original_role', current_user, true); SET LOCAL ROLE test_factory__owner; @@ -26,16 +28,11 @@ BEGIN END $body$; --- Set role back to original value +-- Set role back to original value (saved at the top of this script). DO $body$ -DECLARE - c_sql CONSTANT text := 'SET ROLE ' || (SELECT original_role FROM pg_temp.original_role); BEGIN - --RAISE WARNING 'c_sql = %', c_sql; - EXECUTE c_sql; + EXECUTE 'SET ROLE ' || pg_catalog.quote_ident(pg_catalog.current_setting('test_factory_pgtap.original_role')); END $body$; -DROP TABLE pg_temp.original_role; - -- vi: expandtab ts=2 sw=2 diff --git a/sql/test_factory_pgtap.sql b/sql/test_factory_pgtap.sql index 5a8cae4..1967570 100644 --- a/sql/test_factory_pgtap.sql +++ b/sql/test_factory_pgtap.sql @@ -1,5 +1,7 @@ -CREATE TEMP TABLE original_role ON COMMIT DROP AS SELECT current_user AS original_role; -GRANT SELECT ON pg_temp.original_role TO public; +-- Save the caller's role so we can restore it at the end (we SET LOCAL ROLE +-- below to own our objects). Stashed in a transaction-local GUC rather than a +-- temp table, which would be owned by the extension and break CREATE EXTENSION. +SELECT pg_catalog.set_config('test_factory_pgtap.original_role', current_user, true); SET LOCAL ROLE test_factory__owner; @@ -25,16 +27,11 @@ BEGIN END $body$; --- Set role back to original value +-- Set role back to original value (saved at the top of this script). DO $body$ -DECLARE - c_sql CONSTANT text := 'SET ROLE ' || (SELECT original_role FROM pg_temp.original_role); BEGIN - --RAISE WARNING 'c_sql = %', c_sql; - EXECUTE c_sql; + EXECUTE 'SET ROLE ' || pg_catalog.quote_ident(pg_catalog.current_setting('test_factory_pgtap.original_role')); END $body$; -DROP TABLE pg_temp.original_role; - -- vi: expandtab ts=2 sw=2 From f5b720a3c048904182d965cd3779bd07675ac82d Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Fri, 17 Jul 2026 18:00:13 -0500 Subject: [PATCH 2/3] Fix comment formatting on original_role save/restore comment Convert the new multi-line explanation to the block-comment (/* ... */) style used for other multi-line comments in these files, instead of a paragraph spread across bare `--` lines. --- sql/test_factory--0.5.0.sql | 8 +++++--- sql/test_factory.sql | 8 +++++--- sql/test_factory_pgtap--0.1.0.sql | 8 +++++--- sql/test_factory_pgtap.sql | 8 +++++--- 4 files changed, 20 insertions(+), 12 deletions(-) diff --git a/sql/test_factory--0.5.0.sql b/sql/test_factory--0.5.0.sql index 3105145..f54cad0 100644 --- a/sql/test_factory--0.5.0.sql +++ b/sql/test_factory--0.5.0.sql @@ -1,7 +1,9 @@ /* DO NOT EDIT - AUTO-GENERATED FILE */ --- Save the caller's role so we can restore it at the end (we SET LOCAL ROLE --- below to own our objects). Stashed in a transaction-local GUC rather than a --- temp table, which would be owned by the extension and break CREATE EXTENSION. +/* + * Save the caller's role so we can restore it at the end (we SET LOCAL ROLE + * below to own our objects). Stashed in a transaction-local GUC rather than a + * temp table, which would be owned by the extension and break CREATE EXTENSION. + */ SELECT pg_catalog.set_config('test_factory.original_role', current_user, true); DO $body$ BEGIN diff --git a/sql/test_factory.sql b/sql/test_factory.sql index 7a1efab..72df5f1 100644 --- a/sql/test_factory.sql +++ b/sql/test_factory.sql @@ -1,6 +1,8 @@ --- Save the caller's role so we can restore it at the end (we SET LOCAL ROLE --- below to own our objects). Stashed in a transaction-local GUC rather than a --- temp table, which would be owned by the extension and break CREATE EXTENSION. +/* + * Save the caller's role so we can restore it at the end (we SET LOCAL ROLE + * below to own our objects). Stashed in a transaction-local GUC rather than a + * temp table, which would be owned by the extension and break CREATE EXTENSION. + */ SELECT pg_catalog.set_config('test_factory.original_role', current_user, true); DO $body$ BEGIN diff --git a/sql/test_factory_pgtap--0.1.0.sql b/sql/test_factory_pgtap--0.1.0.sql index e4b8ea5..c5e3382 100644 --- a/sql/test_factory_pgtap--0.1.0.sql +++ b/sql/test_factory_pgtap--0.1.0.sql @@ -1,7 +1,9 @@ /* DO NOT EDIT - AUTO-GENERATED FILE */ --- Save the caller's role so we can restore it at the end (we SET LOCAL ROLE --- below to own our objects). Stashed in a transaction-local GUC rather than a --- temp table, which would be owned by the extension and break CREATE EXTENSION. +/* + * Save the caller's role so we can restore it at the end (we SET LOCAL ROLE + * below to own our objects). Stashed in a transaction-local GUC rather than a + * temp table, which would be owned by the extension and break CREATE EXTENSION. + */ SELECT pg_catalog.set_config('test_factory_pgtap.original_role', current_user, true); SET LOCAL ROLE test_factory__owner; diff --git a/sql/test_factory_pgtap.sql b/sql/test_factory_pgtap.sql index 1967570..96855e2 100644 --- a/sql/test_factory_pgtap.sql +++ b/sql/test_factory_pgtap.sql @@ -1,6 +1,8 @@ --- Save the caller's role so we can restore it at the end (we SET LOCAL ROLE --- below to own our objects). Stashed in a transaction-local GUC rather than a --- temp table, which would be owned by the extension and break CREATE EXTENSION. +/* + * Save the caller's role so we can restore it at the end (we SET LOCAL ROLE + * below to own our objects). Stashed in a transaction-local GUC rather than a + * temp table, which would be owned by the extension and break CREATE EXTENSION. + */ SELECT pg_catalog.set_config('test_factory_pgtap.original_role', current_user, true); SET LOCAL ROLE test_factory__owner; From 368e907b8c071c3cd17099f303f6efbe4686d4e5 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Fri, 17 Jul 2026 18:21:51 -0500 Subject: [PATCH 3/3] Correct rationale in original_role save/restore comment The GUC isn't used to dodge a hypothetical CREATE EXTENSION break from an extension-owned temp table -- that's trivial to avoid either way. The actual reason is that a GUC is much lighter weight than creating a table. --- sql/test_factory--0.5.0.sql | 5 +++-- sql/test_factory.sql | 5 +++-- sql/test_factory_pgtap--0.1.0.sql | 5 +++-- sql/test_factory_pgtap.sql | 5 +++-- 4 files changed, 12 insertions(+), 8 deletions(-) diff --git a/sql/test_factory--0.5.0.sql b/sql/test_factory--0.5.0.sql index f54cad0..340e33f 100644 --- a/sql/test_factory--0.5.0.sql +++ b/sql/test_factory--0.5.0.sql @@ -1,8 +1,9 @@ /* DO NOT EDIT - AUTO-GENERATED FILE */ /* * Save the caller's role so we can restore it at the end (we SET LOCAL ROLE - * below to own our objects). Stashed in a transaction-local GUC rather than a - * temp table, which would be owned by the extension and break CREATE EXTENSION. + * below to own our objects). A GUC is used instead of a temp table not to + * avoid CREATE EXTENSION breakage (trivial to avoid either way) but because + * it's much lighter weight than creating a table. */ SELECT pg_catalog.set_config('test_factory.original_role', current_user, true); DO $body$ diff --git a/sql/test_factory.sql b/sql/test_factory.sql index 72df5f1..c44b076 100644 --- a/sql/test_factory.sql +++ b/sql/test_factory.sql @@ -1,7 +1,8 @@ /* * Save the caller's role so we can restore it at the end (we SET LOCAL ROLE - * below to own our objects). Stashed in a transaction-local GUC rather than a - * temp table, which would be owned by the extension and break CREATE EXTENSION. + * below to own our objects). A GUC is used instead of a temp table not to + * avoid CREATE EXTENSION breakage (trivial to avoid either way) but because + * it's much lighter weight than creating a table. */ SELECT pg_catalog.set_config('test_factory.original_role', current_user, true); DO $body$ diff --git a/sql/test_factory_pgtap--0.1.0.sql b/sql/test_factory_pgtap--0.1.0.sql index c5e3382..9eea098 100644 --- a/sql/test_factory_pgtap--0.1.0.sql +++ b/sql/test_factory_pgtap--0.1.0.sql @@ -1,8 +1,9 @@ /* DO NOT EDIT - AUTO-GENERATED FILE */ /* * Save the caller's role so we can restore it at the end (we SET LOCAL ROLE - * below to own our objects). Stashed in a transaction-local GUC rather than a - * temp table, which would be owned by the extension and break CREATE EXTENSION. + * below to own our objects). A GUC is used instead of a temp table not to + * avoid CREATE EXTENSION breakage (trivial to avoid either way) but because + * it's much lighter weight than creating a table. */ SELECT pg_catalog.set_config('test_factory_pgtap.original_role', current_user, true); diff --git a/sql/test_factory_pgtap.sql b/sql/test_factory_pgtap.sql index 96855e2..4056d14 100644 --- a/sql/test_factory_pgtap.sql +++ b/sql/test_factory_pgtap.sql @@ -1,7 +1,8 @@ /* * Save the caller's role so we can restore it at the end (we SET LOCAL ROLE - * below to own our objects). Stashed in a transaction-local GUC rather than a - * temp table, which would be owned by the extension and break CREATE EXTENSION. + * below to own our objects). A GUC is used instead of a temp table not to + * avoid CREATE EXTENSION breakage (trivial to avoid either way) but because + * it's much lighter weight than creating a table. */ SELECT pg_catalog.set_config('test_factory_pgtap.original_role', current_user, true);