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;
signature.asc
Description: PGP signature