Backends reflect "GRANT role_name" changes rather quickly, due to a syscache
invalidation callback. Let's register an additional callback to reflect
"ALTER ROLE ... [NO]INHERIT" with equal speed. I propose to back-patch this.
While pg_authid changes may be more frequent than pg_auth_members changes, I
expect neither is frequent enough to worry about the resulting acl.c cache
miss rate.
pg_authid changes don't affect cached_membership_roles, so I could have
invalidated cached_privs_roles only. That felt like needless complexity. I
expect cached_privs_role gets the bulk of traffic, since SELECT, INSERT,
UPDATE and DELETE use it. cached_membership_roles pertains to DDL and such.
Author: Noah Misch <n...@leadboat.com>
Commit: Noah Misch <n...@leadboat.com>
Invalidate acl.c caches when pg_authid changes.
This makes existing sessions reflect "ALTER ROLE ... [NO]INHERIT" as
quickly as they have been reflecting "GRANT role_name". Back-patch to
9.5 (all supported versions).
Reviewed by FIXME.
Discussion: https://postgr.es/m/FIXME
diff --git a/src/backend/utils/adt/acl.c b/src/backend/utils/adt/acl.c
index f97489f..fe6c444 100644
--- a/src/backend/utils/adt/acl.c
+++ b/src/backend/utils/adt/acl.c
@@ -52,7 +52,6 @@ typedef struct
* role. In most of these tests the "given role" is the same, namely the
* active current user. So we can optimize it by keeping a cached list of
* all the roles the "given role" is a member of, directly or indirectly.
- * The cache is flushed whenever we detect a change in pg_auth_members.
*
* There are actually two caches, one computed under "has_privs" rules
* (do not recurse where rolinherit isn't true) and one computed under
@@ -4675,12 +4674,16 @@ initialize_acl(void)
if (!IsBootstrapProcessingMode())
{
/*
- * In normal mode, set a callback on any syscache invalidation
of
- * pg_auth_members rows
+ * In normal mode, set a callback on any syscache invalidation
of rows
+ * of pg_auth_members (for each AUTHMEM search in this file) or
+ * pg_authid (for has_rolinherit())
*/
CacheRegisterSyscacheCallback(AUTHMEMROLEMEM,
RoleMembershipCacheCallback,
(Datum) 0);
+ CacheRegisterSyscacheCallback(AUTHOID,
+
RoleMembershipCacheCallback,
+
(Datum) 0);
}
}
diff --git a/src/test/regress/expected/privileges.out
b/src/test/regress/expected/privileges.out
index 0a2dd37..7754c20 100644
--- a/src/test/regress/expected/privileges.out
+++ b/src/test/regress/expected/privileges.out
@@ -350,6 +350,13 @@ SET SESSION AUTHORIZATION regress_priv_user1;
SELECT * FROM atest3; -- fail
ERROR: permission denied for table atest3
DELETE FROM atest3; -- ok
+BEGIN;
+RESET SESSION AUTHORIZATION;
+ALTER ROLE regress_priv_user1 NOINHERIT;
+SET SESSION AUTHORIZATION regress_priv_user1;
+DELETE FROM atest3;
+ERROR: permission denied for table atest3
+ROLLBACK;
-- views
SET SESSION AUTHORIZATION regress_priv_user3;
CREATE VIEW atestv1 AS SELECT * FROM atest1; -- ok
diff --git a/src/test/regress/sql/privileges.sql
b/src/test/regress/sql/privileges.sql
index e0c1a29..4911ad4 100644
--- a/src/test/regress/sql/privileges.sql
+++ b/src/test/regress/sql/privileges.sql
@@ -220,6 +220,12 @@ SET SESSION AUTHORIZATION regress_priv_user1;
SELECT * FROM atest3; -- fail
DELETE FROM atest3; -- ok
+BEGIN;
+RESET SESSION AUTHORIZATION;
+ALTER ROLE regress_priv_user1 NOINHERIT;
+SET SESSION AUTHORIZATION regress_priv_user1;
+DELETE FROM atest3;
+ROLLBACK;
-- views
