Date: Tue, 17 Mar 2026 19:50:33 -0500
From: Nathan Bossart <nathandbossart@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Robert Haas <robertmhaas@gmail.com>,
	Peter Eisentraut <peter@eisentraut.org>,
	"David G. Johnston" <david.g.johnston@gmail.com>,
	"Ing. Marijo Kristo" <marijo.kristo@icloud.com>,
	PostgreSQL Bug List <pgsql-bugs@lists.postgresql.org>
Subject: Re: Revoke Connect Privilege from Database not working
Message-ID: <abn22fc7-oHGObbZ@nathan>
References: <CAKFQuwa7m2smqqpgPetw=i8Aj-xqg9Zjc5Z2aX3AUwNh96WnXw@mail.gmail.com>
 <d9bf666c-4d11-4196-99a8-b71d01d9ad40@me.com>
 <CAKFQuwbB-ZKtN_p_y5sWa2MrTuy5=pRNPWSj1Ud4HHvTuhb54w@mail.gmail.com>
 <3467676.1744041977@sss.pgh.pa.us>
 <CAKFQuwbpC5w6sUq8gZQATrviZUT4bYpxW+=2uH6sWWMg7fWjzg@mail.gmail.com>
 <aRYLkTpazxKhnS_w@nathan>
 <1933586.1768950341@sss.pgh.pa.us>
 <aXDwtbXCu42Fdmrn@nathan>
 <2222571.1769014621@sss.pgh.pa.us>
 <aatOzgie9RlzbGoo@nathan>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="m6BrJmZ21ThuEPo6"
Content-Disposition: inline
In-Reply-To: <aatOzgie9RlzbGoo@nathan>
Archived-At: <https://www.postgresql.org/message-id/abn22fc7-oHGObbZ%40nathan>
Precedence: bulk


--m6BrJmZ21ThuEPo6
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

I went ahead and tried adding docs, tests, and a commit message.  The
documentation for these commands might need a revamp.  They seem to meander
a bit, probably due to decades of organic development.  But that's probably
not this patch's problem.

-- 
nathan

--m6BrJmZ21ThuEPo6
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment;
	filename=v3-0001-Allow-choosing-specific-grantors-via-GRANT-REVOKE.patch

From b6c7ba8ff4934fcd25e3a9adb66694b604dfe799 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@postgresql.org>
Date: Tue, 17 Mar 2026 19:40:31 -0500
Subject: [PATCH v3 1/1] Allow choosing specific grantors via GRANT/REVOKE ...
 GRANTED BY.

Except for GRANT and REVOKE on roles, the GRANTED BY clause
currently only accepts the current role to match the SQL standard.
And even if an acceptable grantor (i.e., the current role) is
specified, Postgres ignores it and chooses the "best" grantor for
the command.  Allowing the user to select a specific grantor would
allow better control over the precise behavior of GRANT/REVOKE
statements.  This commit adds that ability.  For consistency with
select_best_grantor(), we only permit choosing grantor roles for
which the current role inherits privileges.

Author: Nathan Bossart <nathandbossart@gmail.com>
Co-authored-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/aRYLkTpazxKhnS_w%40nathan
---
 doc/src/sgml/ref/grant.sgml              |  6 +--
 doc/src/sgml/ref/revoke.sgml             |  8 +++-
 src/backend/catalog/aclchk.c             | 31 ++++--------
 src/backend/utils/adt/acl.c              | 33 +++++++++++--
 src/include/nodes/parsenodes.h           |  2 +-
 src/include/utils/acl.h                  |  2 +-
 src/include/utils/aclchk_internal.h      |  1 +
 src/test/regress/expected/privileges.out | 60 +++++++++++++++++++++++-
 src/test/regress/sql/privileges.sql      | 34 ++++++++++++++
 9 files changed, 144 insertions(+), 33 deletions(-)

diff --git a/doc/src/sgml/ref/grant.sgml b/doc/src/sgml/ref/grant.sgml
index 0e57348d893..1eb5a96cc57 100644
--- a/doc/src/sgml/ref/grant.sgml
+++ b/doc/src/sgml/ref/grant.sgml
@@ -158,9 +158,9 @@ GRANT <replaceable class="parameter">role_name</replaceable> [, ...] TO <replace
   </para>
 
   <para>
-   If <literal>GRANTED BY</literal> is specified, the specified grantor must
-   be the current user.  This clause is currently present in this form only
-   for SQL compatibility.
+   If <literal>GRANTED BY</literal> is specified, the grant is recorded as
+   having been done by the specified role.  A role can only attribute a grant
+   to another role if they possess the privileges of that role.
   </para>
 
   <para>
diff --git a/doc/src/sgml/ref/revoke.sgml b/doc/src/sgml/ref/revoke.sgml
index 948ac534446..244e8becfa2 100644
--- a/doc/src/sgml/ref/revoke.sgml
+++ b/doc/src/sgml/ref/revoke.sgml
@@ -181,6 +181,12 @@ REVOKE [ { ADMIN | INHERIT | SET } OPTION FOR ]
    Otherwise, both the privilege and the grant option are revoked.
   </para>
 
+  <para>
+   If <literal>GRANTED BY</literal> is specified, only privileges granted by
+   the specified role are revoked.  A role can only revoke grants by another
+   role if they possess the privileges of that role.
+  </para>
+
   <para>
    If a user holds a privilege with grant option and has granted it to
    other users then the privileges held by those other users are
@@ -282,7 +288,7 @@ REVOKE [ { ADMIN | INHERIT | SET } OPTION FOR ]
     If the role executing <command>REVOKE</command> holds privileges
     indirectly via more than one role membership path, it is unspecified
     which containing role will be used to perform the command.  In such cases
-    it is best practice to use <command>SET ROLE</command> to become the specific
+    it is best practice to use <literal>GRANTED BY</literal> to specify which
     role you want to do the <command>REVOKE</command> as.  Failure to do so might
     lead to revoking privileges other than the ones you intended, or not
     revoking anything at all.
diff --git a/src/backend/catalog/aclchk.c b/src/backend/catalog/aclchk.c
index 52f38480c52..67424fe3b0c 100644
--- a/src/backend/catalog/aclchk.c
+++ b/src/backend/catalog/aclchk.c
@@ -98,6 +98,7 @@ typedef struct
 	AclMode		privileges;
 	List	   *grantees;
 	bool		grant_option;
+	RoleSpec   *grantor;
 	DropBehavior behavior;
 } InternalDefaultACL;
 
@@ -398,22 +399,6 @@ ExecuteGrantStmt(GrantStmt *stmt)
 	const char *errormsg;
 	AclMode		all_privileges;
 
-	if (stmt->grantor)
-	{
-		Oid			grantor;
-
-		grantor = get_rolespec_oid(stmt->grantor, false);
-
-		/*
-		 * Currently, this clause is only for SQL compatibility, not very
-		 * interesting otherwise.
-		 */
-		if (grantor != GetUserId())
-			ereport(ERROR,
-					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-					 errmsg("grantor must be current user")));
-	}
-
 	/*
 	 * Turn the regular GrantStmt into the InternalGrant form.
 	 */
@@ -441,6 +426,7 @@ ExecuteGrantStmt(GrantStmt *stmt)
 	istmt.col_privs = NIL;		/* may get filled below */
 	istmt.grantees = NIL;		/* filled below */
 	istmt.grant_option = stmt->grant_option;
+	istmt.grantor = stmt->grantor;
 	istmt.behavior = stmt->behavior;
 
 	/*
@@ -973,6 +959,7 @@ ExecAlterDefaultPrivilegesStmt(ParseState *pstate, AlterDefaultPrivilegesStmt *s
 	/* privileges to be filled below */
 	iacls.grantees = NIL;		/* filled below */
 	iacls.grant_option = action->grant_option;
+	iacls.grantor = action->grantor;
 	iacls.behavior = action->behavior;
 
 	/*
@@ -1503,6 +1490,7 @@ RemoveRoleFromObjectACL(Oid roleid, Oid classid, Oid objid)
 		iacls.privileges = ACL_NO_RIGHTS;
 		iacls.grantees = list_make1_oid(roleid);
 		iacls.grant_option = false;
+		iacls.grantor = NULL;
 		iacls.behavior = DROP_CASCADE;
 
 		/* Do it */
@@ -1559,6 +1547,7 @@ RemoveRoleFromObjectACL(Oid roleid, Oid classid, Oid objid)
 		istmt.col_privs = NIL;
 		istmt.grantees = list_make1_oid(roleid);
 		istmt.grant_option = false;
+		istmt.grantor = NULL;
 		istmt.behavior = DROP_CASCADE;
 
 		ExecGrantStmt_oids(&istmt);
@@ -1713,7 +1702,7 @@ ExecGrant_Attribute(InternalGrant *istmt, Oid relOid, const char *relname,
 	merged_acl = aclconcat(old_rel_acl, old_acl);
 
 	/* Determine ID to do the grant as, and available grant options */
-	select_best_grantor(GetUserId(), col_privileges,
+	select_best_grantor(istmt->grantor, col_privileges,
 						merged_acl, ownerId,
 						&grantorId, &avail_goptions);
 
@@ -1998,7 +1987,7 @@ ExecGrant_Relation(InternalGrant *istmt)
 			ObjectType	objtype;
 
 			/* Determine ID to do the grant as, and available grant options */
-			select_best_grantor(GetUserId(), this_privileges,
+			select_best_grantor(istmt->grantor, this_privileges,
 								old_acl, ownerId,
 								&grantorId, &avail_goptions);
 
@@ -2213,7 +2202,7 @@ ExecGrant_common(InternalGrant *istmt, Oid classid, AclMode default_privs,
 		}
 
 		/* Determine ID to do the grant as, and available grant options */
-		select_best_grantor(GetUserId(), istmt->privileges,
+		select_best_grantor(istmt->grantor, istmt->privileges,
 							old_acl, ownerId,
 							&grantorId, &avail_goptions);
 
@@ -2368,7 +2357,7 @@ ExecGrant_Largeobject(InternalGrant *istmt)
 		}
 
 		/* Determine ID to do the grant as, and available grant options */
-		select_best_grantor(GetUserId(), istmt->privileges,
+		select_best_grantor(istmt->grantor, istmt->privileges,
 							old_acl, ownerId,
 							&grantorId, &avail_goptions);
 
@@ -2514,7 +2503,7 @@ ExecGrant_Parameter(InternalGrant *istmt)
 		}
 
 		/* Determine ID to do the grant as, and available grant options */
-		select_best_grantor(GetUserId(), istmt->privileges,
+		select_best_grantor(istmt->grantor, istmt->privileges,
 							old_acl, ownerId,
 							&grantorId, &avail_goptions);
 
diff --git a/src/backend/utils/adt/acl.c b/src/backend/utils/adt/acl.c
index b9190e700dc..01caa12eca7 100644
--- a/src/backend/utils/adt/acl.c
+++ b/src/backend/utils/adt/acl.c
@@ -5481,6 +5481,10 @@ select_best_admin(Oid member, Oid role)
 /*
  * Select the effective grantor ID for a GRANT or REVOKE operation.
  *
+ * If the GRANT/REVOKE has an explicit GRANTED BY clause, we always use
+ * exactly that role (which may result in granting/revoking no privileges).
+ * Otherwise, we seek a "best" grantor, starting with the current user.
+ *
  * The grantor must always be either the object owner or some role that has
  * been explicitly granted grant options.  This ensures that all granted
  * privileges appear to flow from the object owner, and there are never
@@ -5493,25 +5497,44 @@ select_best_admin(Oid member, Oid role)
  * role has 'em all.  In this case we pick a role with the largest number
  * of desired options.  Ties are broken in favor of closer ancestors.
  *
- * roleId: the role attempting to do the GRANT/REVOKE
+ * grantedBy: the GRANTED BY clause of GRANT/REVOKE, or NULL if none
  * privileges: the privileges to be granted/revoked
  * acl: the ACL of the object in question
  * ownerId: the role owning the object in question
  * *grantorId: receives the OID of the role to do the grant as
- * *grantOptions: receives the grant options actually held by grantorId
- *
- * If no grant options exist, we set grantorId to roleId, grantOptions to 0.
+ * *grantOptions: receives grant options actually held by grantorId (maybe 0)
  */
 void
-select_best_grantor(Oid roleId, AclMode privileges,
+select_best_grantor(const RoleSpec *grantedBy, AclMode privileges,
 					const Acl *acl, Oid ownerId,
 					Oid *grantorId, AclMode *grantOptions)
 {
+	Oid			roleId = GetUserId();
 	AclMode		needed_goptions = ACL_GRANT_OPTION_FOR(privileges);
 	List	   *roles_list;
 	int			nrights;
 	ListCell   *l;
 
+	/*
+	 * If we have GRANTED BY, resolve it and verify current user is allowed to
+	 * specify that role.
+	 */
+	if (grantedBy)
+	{
+		Oid			grantor = get_rolespec_oid(grantedBy, false);
+
+		if (!has_privs_of_role(roleId, grantor))
+			ereport(ERROR,
+					(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+					 errmsg("must inherit privileges of role \"%s\"",
+							GetUserNameFromId(grantor, false))));
+		/* Use exactly that grantor, whether it has privileges or not */
+		*grantorId = grantor;
+		*grantOptions = aclmask_direct(acl, grantor, ownerId,
+									   needed_goptions, ACLMASK_ALL);
+		return;
+	}
+
 	/*
 	 * The object owner is always treated as having all grant options, so if
 	 * roleId is the owner it's easy.  Also, if roleId is a superuser it's
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index ffadd667167..4e1ea9d1e8e 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -2672,7 +2672,7 @@ typedef struct GrantStmt
 	/* privileges == NIL denotes ALL PRIVILEGES */
 	List	   *grantees;		/* list of RoleSpec nodes */
 	bool		grant_option;	/* grant or revoke grant option */
-	RoleSpec   *grantor;
+	RoleSpec   *grantor;		/* GRANTED BY clause, or NULL if none */
 	DropBehavior behavior;		/* drop behavior (for REVOKE) */
 } GrantStmt;
 
diff --git a/src/include/utils/acl.h b/src/include/utils/acl.h
index 0bd1a5ce506..0b9b04e78ee 100644
--- a/src/include/utils/acl.h
+++ b/src/include/utils/acl.h
@@ -224,7 +224,7 @@ extern void check_rolespec_name(const RoleSpec *role, const char *detail_msg);
 extern HeapTuple get_rolespec_tuple(const RoleSpec *role);
 extern char *get_rolespec_name(const RoleSpec *role);
 
-extern void select_best_grantor(Oid roleId, AclMode privileges,
+extern void select_best_grantor(const RoleSpec *grantedBy, AclMode privileges,
 								const Acl *acl, Oid ownerId,
 								Oid *grantorId, AclMode *grantOptions);
 
diff --git a/src/include/utils/aclchk_internal.h b/src/include/utils/aclchk_internal.h
index 38317e2ed37..fa0b65fbba7 100644
--- a/src/include/utils/aclchk_internal.h
+++ b/src/include/utils/aclchk_internal.h
@@ -38,6 +38,7 @@ typedef struct
 	List	   *col_privs;
 	List	   *grantees;
 	bool		grant_option;
+	RoleSpec   *grantor;
 	DropBehavior behavior;
 } InternalGrant;
 
diff --git a/src/test/regress/expected/privileges.out b/src/test/regress/expected/privileges.out
index 9c9cdd12af5..7069e9febb8 100644
--- a/src/test/regress/expected/privileges.out
+++ b/src/test/regress/expected/privileges.out
@@ -321,7 +321,7 @@ SELECT pg_get_acl(0, 0, 0); -- null
 (1 row)
 
 GRANT TRUNCATE ON atest2 TO regress_priv_user4 GRANTED BY regress_priv_user5;  -- error
-ERROR:  grantor must be current user
+ERROR:  must inherit privileges of role "regress_priv_user5"
 SET SESSION AUTHORIZATION regress_priv_user2;
 SELECT session_user, current_user;
     session_user    |    current_user    
@@ -3621,3 +3621,61 @@ SELECT * FROM information_schema.table_privileges t
 
 DROP TABLE grantor_test1, grantor_test2, grantor_test3;
 DROP ROLE regress_grantor1, regress_grantor2, regress_grantor3;
+-- GRANTED BY
+CREATE ROLE regress_grantor1;
+CREATE ROLE regress_grantor2 ROLE regress_grantor1;
+CREATE ROLE regress_grantor3 ROLE regress_grantor1;
+CREATE ROLE regress_grantor4 ROLE regress_grantor1;
+CREATE ROLE regress_grantor5;
+CREATE TABLE grantor_test ();
+GRANT SELECT ON grantor_test TO regress_grantor2 WITH GRANT OPTION;
+GRANT UPDATE ON grantor_test TO regress_grantor3 WITH GRANT OPTION;
+GRANT SELECT, UPDATE ON grantor_test TO regress_grantor4 WITH GRANT OPTION;
+SET ROLE regress_grantor1;
+GRANT SELECT, UPDATE ON grantor_test TO regress_grantor5;
+SELECT * FROM information_schema.table_privileges t
+    WHERE grantor LIKE 'regress_grantor%' ORDER BY ROW(t.*);
+     grantor      |     grantee      | table_catalog | table_schema |  table_name  | privilege_type | is_grantable | with_hierarchy 
+------------------+------------------+---------------+--------------+--------------+----------------+--------------+----------------
+ regress_grantor4 | regress_grantor5 | regression    | public       | grantor_test | SELECT         | NO           | YES
+ regress_grantor4 | regress_grantor5 | regression    | public       | grantor_test | UPDATE         | NO           | NO
+(2 rows)
+
+REVOKE SELECT, UPDATE ON grantor_test FROM regress_grantor5;
+GRANT SELECT, UPDATE ON grantor_test TO regress_grantor5 GRANTED BY regress_grantor2;
+WARNING:  not all privileges were granted for "grantor_test"
+GRANT SELECT, UPDATE ON grantor_test TO regress_grantor5 GRANTED BY regress_grantor3;
+WARNING:  not all privileges were granted for "grantor_test"
+SELECT * FROM information_schema.table_privileges t
+    WHERE grantor LIKE 'regress_grantor%' ORDER BY ROW(t.*);
+     grantor      |     grantee      | table_catalog | table_schema |  table_name  | privilege_type | is_grantable | with_hierarchy 
+------------------+------------------+---------------+--------------+--------------+----------------+--------------+----------------
+ regress_grantor2 | regress_grantor5 | regression    | public       | grantor_test | SELECT         | NO           | YES
+ regress_grantor3 | regress_grantor5 | regression    | public       | grantor_test | UPDATE         | NO           | NO
+(2 rows)
+
+REVOKE SELECT, UPDATE ON grantor_test FROM regress_grantor5 GRANTED BY regress_grantor2;
+WARNING:  not all privileges could be revoked for "grantor_test"
+WARNING:  not all privileges could be revoked for column "tableoid" of relation "grantor_test"
+WARNING:  not all privileges could be revoked for column "cmax" of relation "grantor_test"
+WARNING:  not all privileges could be revoked for column "xmax" of relation "grantor_test"
+WARNING:  not all privileges could be revoked for column "cmin" of relation "grantor_test"
+WARNING:  not all privileges could be revoked for column "xmin" of relation "grantor_test"
+WARNING:  not all privileges could be revoked for column "ctid" of relation "grantor_test"
+REVOKE SELECT, UPDATE ON grantor_test FROM regress_grantor5 GRANTED BY regress_grantor3;
+WARNING:  not all privileges could be revoked for "grantor_test"
+WARNING:  not all privileges could be revoked for column "tableoid" of relation "grantor_test"
+WARNING:  not all privileges could be revoked for column "cmax" of relation "grantor_test"
+WARNING:  not all privileges could be revoked for column "xmax" of relation "grantor_test"
+WARNING:  not all privileges could be revoked for column "cmin" of relation "grantor_test"
+WARNING:  not all privileges could be revoked for column "xmin" of relation "grantor_test"
+WARNING:  not all privileges could be revoked for column "ctid" of relation "grantor_test"
+SELECT * FROM information_schema.table_privileges t
+    WHERE grantor LIKE 'regress_grantor%' ORDER BY ROW(t.*);
+ grantor | grantee | table_catalog | table_schema | table_name | privilege_type | is_grantable | with_hierarchy 
+---------+---------+---------------+--------------+------------+----------------+--------------+----------------
+(0 rows)
+
+RESET ROLE;
+DROP TABLE grantor_test;
+DROP ROLE regress_grantor1, regress_grantor2, regress_grantor3, regress_grantor4, regress_grantor5;
diff --git a/src/test/regress/sql/privileges.sql b/src/test/regress/sql/privileges.sql
index e34c65fc1b2..9f21c2945bd 100644
--- a/src/test/regress/sql/privileges.sql
+++ b/src/test/regress/sql/privileges.sql
@@ -2211,3 +2211,37 @@ SELECT * FROM information_schema.table_privileges t
 
 DROP TABLE grantor_test1, grantor_test2, grantor_test3;
 DROP ROLE regress_grantor1, regress_grantor2, regress_grantor3;
+
+-- GRANTED BY
+CREATE ROLE regress_grantor1;
+CREATE ROLE regress_grantor2 ROLE regress_grantor1;
+CREATE ROLE regress_grantor3 ROLE regress_grantor1;
+CREATE ROLE regress_grantor4 ROLE regress_grantor1;
+CREATE ROLE regress_grantor5;
+CREATE TABLE grantor_test ();
+GRANT SELECT ON grantor_test TO regress_grantor2 WITH GRANT OPTION;
+GRANT UPDATE ON grantor_test TO regress_grantor3 WITH GRANT OPTION;
+GRANT SELECT, UPDATE ON grantor_test TO regress_grantor4 WITH GRANT OPTION;
+SET ROLE regress_grantor1;
+
+GRANT SELECT, UPDATE ON grantor_test TO regress_grantor5;
+
+SELECT * FROM information_schema.table_privileges t
+    WHERE grantor LIKE 'regress_grantor%' ORDER BY ROW(t.*);
+
+REVOKE SELECT, UPDATE ON grantor_test FROM regress_grantor5;
+GRANT SELECT, UPDATE ON grantor_test TO regress_grantor5 GRANTED BY regress_grantor2;
+GRANT SELECT, UPDATE ON grantor_test TO regress_grantor5 GRANTED BY regress_grantor3;
+
+SELECT * FROM information_schema.table_privileges t
+    WHERE grantor LIKE 'regress_grantor%' ORDER BY ROW(t.*);
+
+REVOKE SELECT, UPDATE ON grantor_test FROM regress_grantor5 GRANTED BY regress_grantor2;
+REVOKE SELECT, UPDATE ON grantor_test FROM regress_grantor5 GRANTED BY regress_grantor3;
+
+SELECT * FROM information_schema.table_privileges t
+    WHERE grantor LIKE 'regress_grantor%' ORDER BY ROW(t.*);
+
+RESET ROLE;
+DROP TABLE grantor_test;
+DROP ROLE regress_grantor1, regress_grantor2, regress_grantor3, regress_grantor4, regress_grantor5;
-- 
2.50.1 (Apple Git-155)


--m6BrJmZ21ThuEPo6--