On Tue, Jun 28, 2022 at 10:12:48AM +0900, Michael Paquier wrote:
> As far as I can see, test_oat_hook has no need to keep around the
> extra role it creates as part of the regression tests, because at the
> end of the test there are no objects that depend on it.  Wouldn't it
> be better to make the test self-isolated?  NO_INSTALLCHECK is set in
> the module because of the issue with caching and the namespace search
> hooks, but it seems to me that we'd better make the test self-isolated
> in the long term, like in the attached.

And actually, I have found a second issue here.  The tests issue a
GRANT on work_mem, like that:
GRANT SET ON PARAMETER work_mem TO PUBLIC;

This has as effect to leave around an entry in pg_parameter_acl, which
is designed this way in aclchk.c.  However, this interacts with
guc_privs.sql in unsafe_tests, because those tests include similar
queries GRANT queries, also on work_mem.  So, if one issues an
installcheck on test_oat_modules followed by an installcheck in
unsafe_tests, the latter fails.  I think that we'd better add an extra
REVOKE to clear the contents of pg_parameter_acl.
--
Michael
diff --git a/src/test/modules/test_oat_hooks/expected/test_oat_hooks.out b/src/test/modules/test_oat_hooks/expected/test_oat_hooks.out
index 39b274b8fa..a86455395d 100644
--- a/src/test/modules/test_oat_hooks/expected/test_oat_hooks.out
+++ b/src/test/modules/test_oat_hooks/expected/test_oat_hooks.out
@@ -44,6 +44,9 @@ privileges for parameter another.bogus
 GRANT SET ON PARAMETER work_mem TO PUBLIC;
 NOTICE:  in process utility: superuser attempting GrantStmt
 NOTICE:  in process utility: superuser finished GrantStmt
+REVOKE SET ON PARAMETER work_mem FROM PUBLIC;
+NOTICE:  in process utility: superuser attempting GrantStmt
+NOTICE:  in process utility: superuser finished GrantStmt
 REVOKE ALTER SYSTEM ON PARAMETER work_mem FROM PUBLIC;
 NOTICE:  in process utility: superuser attempting GrantStmt
 NOTICE:  in process utility: superuser finished GrantStmt
@@ -299,3 +302,4 @@ REVOKE ALL PRIVILEGES ON PARAMETER
 	test_oat_hooks.user_var2, test_oat_hooks.super_var2
 	FROM regress_role_joe;
 DROP ROLE regress_role_joe;
+DROP ROLE regress_test_user;
diff --git a/src/test/modules/test_oat_hooks/sql/test_oat_hooks.sql b/src/test/modules/test_oat_hooks/sql/test_oat_hooks.sql
index 8b6d5373aa..c27d0068ba 100644
--- a/src/test/modules/test_oat_hooks/sql/test_oat_hooks.sql
+++ b/src/test/modules/test_oat_hooks/sql/test_oat_hooks.sql
@@ -26,6 +26,7 @@ DROP ROLE regress_role_joe;
 
 -- Check the behavior of the hooks relative to do-nothing grants and revokes
 GRANT SET ON PARAMETER work_mem TO PUBLIC;
+REVOKE SET ON PARAMETER work_mem FROM PUBLIC;
 REVOKE ALTER SYSTEM ON PARAMETER work_mem FROM PUBLIC;
 
 -- Check the behavior of the hooks relative to unrecognized parameters
@@ -98,3 +99,4 @@ REVOKE ALL PRIVILEGES ON PARAMETER
 	test_oat_hooks.user_var2, test_oat_hooks.super_var2
 	FROM regress_role_joe;
 DROP ROLE regress_role_joe;
+DROP ROLE regress_test_user;

Attachment: signature.asc
Description: PGP signature

Reply via email to