Subject: Re: allow granting CLUSTER, REFRESH MATERIALIZED VIEW, and REINDEX

From: Jeff Davis <[email-masked]>
Date: Fri, 13 Jan 2023 13:30:28 -0800

On Fri, 2023-01-13 at 12:33 -0800, Nathan Bossart wrote:
> That would fix the problem in the original complaint, but it wouldn't
> allow
> for vacuuming toast tables directly if you only have MAINTAIN
> privileges on
> the main relation.  If you can vacuum the toast table indirectly via
> the
> main relation, shouldn't it be possible to vacuum it directly?

Perhaps, but that's barely supported today: you have to awkwardly find
the internal toast table name yourself, and you need the admin to grant
you USAGE on the pg_toast schema. I don't think we're obligated to also
support this hackery for non-owners with a new MAINTAIN privilege.

If we care about that use case, let's do it right and have forms of
VACUUM/CLUSTER/REINDEX that check permissions on the main table, skip
the work on the main table, and descend directly to the toast tables.
That doesn't seem hard, but it's a separate patch.

Right now, we should simply fix the problem.

-- 
Jeff Davis
PostgreSQL Contributor Team - AWS

---

From: Nathan Bossart <[email-masked]>
Date: Fri, 13 Jan 2023 14:56:26 -0800

> That doesn't seem hard, but it's a separate patch.

You may be interested in https://commitfest.postgresql.org/41/4088/.

> Right now, we should simply fix the problem.

Okay.  Here is a new patch set.  I've split the partition work out to a
separate patch, and I've removed the main relation lookups for TOAST tables
in favor of adding a skip_privs flag to vacuum_rel().  The latter patch
probably needs some additional commentary and tests, which I'll go ahead
and add if we want to proceed with this approach.  I'm assuming the session
lock should be sufficient for avoiding the case where the TOAST table's OID
is reused by the time we get to it, but I'm not sure if it's sufficient to
prevent vacuuming if the privileges on the main relation have changed
across transactions.  Even if it's not, I'm not sure that case is worth
worrying about too much.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
>From 989674a789187a8e620bb70c2155e4e3ccacdcdf Mon Sep 17 00:00:00 2001
From: Nathan Bossart <[email-masked]>
Date: Mon, 9 Jan 2023 14:29:24 -0800
Subject: [PATCH v5 1/2] fix MAINTAIN privs for partitions

---
 doc/src/sgml/ref/analyze.sgml                 |  5 ++-
 doc/src/sgml/ref/cluster.sgml                 | 17 ++++++---
 doc/src/sgml/ref/lock.sgml                    |  5 ++-
 doc/src/sgml/ref/reindex.sgml                 |  6 +++-
 doc/src/sgml/ref/vacuum.sgml                  |  5 ++-
 src/backend/commands/cluster.c                | 35 +++++++++++++------
 src/backend/commands/indexcmds.c              | 22 ++++++------
 src/backend/commands/lockcmds.c               |  8 +++++
 src/backend/commands/tablecmds.c              | 30 ++++++++++++++--
 src/backend/commands/vacuum.c                 |  9 +++--
 src/include/commands/tablecmds.h              |  1 +
 .../expected/cluster-conflict-partition.out   |  6 ++--
 .../specs/cluster-conflict-partition.spec     |  5 +--
 src/test/regress/expected/cluster.out         |  4 ++-
 src/test/regress/expected/vacuum.out          | 18 ----------
 src/test/regress/sql/cluster.sql              |  2 ++
 16 files changed, 119 insertions(+), 59 deletions(-)

diff --git a/doc/src/sgml/ref/analyze.sgml b/doc/src/sgml/ref/analyze.sgml
index a26834da4f..2f94e89cb0 100644
--- a/doc/src/sgml/ref/analyze.sgml
+++ b/doc/src/sgml/ref/analyze.sgml
@@ -156,7 +156,10 @@ ANALYZE [ VERBOSE ] [ <replaceable class="parameter">table_and_columns</replacea
    analyze all tables in their databases, except shared catalogs.
    (The restriction for shared catalogs means that a true database-wide
    <command>ANALYZE</command> can only be performed by superusers and roles
-   with privileges of <literal>pg_maintain</literal>.)
+   with privileges of <literal>pg_maintain</literal>.)  If a role has
+   permission to <command>ANALYZE</command> a partitioned table, it is also
+   permitted to <command>ANALYZE</command> each of its partitions, regardless
+   of whether the role has the aforementioned privileges on the partition.
    <command>ANALYZE</command> will skip over any tables that the calling user
    does not have permission to analyze.
   </para>
diff --git a/doc/src/sgml/ref/cluster.sgml b/doc/src/sgml/ref/cluster.sgml
index 145101e6a5..b9f2acb1de 100644
--- a/doc/src/sgml/ref/cluster.sgml
+++ b/doc/src/sgml/ref/cluster.sgml
@@ -69,10 +69,7 @@ CLUSTER [VERBOSE]
   <para>
    <command>CLUSTER</command> without any parameter reclusters all the
    previously-clustered tables in the current database that the calling user
-   owns or has the <literal>MAINTAIN</literal> privilege for, or all such tables
-   if called by a superuser or a role with privileges of the
-   <link linkend="predefined-roles-table"><literal>pg_maintain</literal></link>
-   role.  This form of <command>CLUSTER</command> cannot be
+   has privileges for.  This form of <command>CLUSTER</command> cannot be
    executed inside a transaction block.
   </para>
 
@@ -134,6 +131,18 @@ CLUSTER [VERBOSE]
  <refsect1>
   <title>Notes</title>
 
+   <para>
+    To cluster a table, one must have the <literal>MAINTAIN</literal> privilege
+    on the table or be the table's owner, a superuser, or a role with
+    privileges of the
+    <link linkend="predefined-roles-table"><literal>pg_maintain</literal></link>
+    role.  If a role has permission to <command>CLUSTER</command> a partitioned
+    table, it is also permitted to <command>CLUSTER</command> each of its
+    partitions, regardless of whether the role has the aforementioned
+    privileges on the partition.  <command>CLUSTER</command> will skip over any
+    tables that the calling user does not have permission to cluster.
+   </para>
+
    <para>
     In cases where you are accessing single rows randomly
     within a table, the actual order of the data in the
diff --git a/doc/src/sgml/ref/lock.sgml b/doc/src/sgml/ref/lock.sgml
index d9c5bf9a1d..21a9f88c70 100644
--- a/doc/src/sgml/ref/lock.sgml
+++ b/doc/src/sgml/ref/lock.sgml
@@ -176,7 +176,10 @@ LOCK [ TABLE ] [ ONLY ] <replaceable class="parameter">name</replaceable> [ * ]
     or <literal>TRUNCATE</literal> privileges on the target table. All other
     forms of <command>LOCK</command> are allowed with
     table-level <literal>UPDATE</literal>, <literal>DELETE</literal>,
-    or <literal>TRUNCATE</literal> privileges.
+    or <literal>TRUNCATE</literal> privileges.  If a role has permission to
+    lock a partitioned table, it is also permitted to lock each of its
+    partitions, regardless of whether the role has the aforementioned
+    privileges on the partition.
    </para>
 
    <para>
diff --git a/doc/src/sgml/ref/reindex.sgml b/doc/src/sgml/ref/reindex.sgml
index 192513f34e..c6ad2546f9 100644
--- a/doc/src/sgml/ref/reindex.sgml
+++ b/doc/src/sgml/ref/reindex.sgml
@@ -306,7 +306,11 @@ REINDEX [ ( <replaceable class="parameter">option</replaceable> [, ...] ) ] { DA
    indexes on shared catalogs will be skipped unless the user owns the
    catalog (which typically won't be the case), has privileges of the
    <literal>pg_maintain</literal> role, or has the <literal>MAINTAIN</literal>
-   privilege on the catalog.  Of course, superusers can always reindex anything.
+   privilege on the catalog.  If a role has permission to
+   <command>REINDEX</command> a partitioned table, it is also permitted to
+   <command>REINDEX</command> each of its partitions, regardless of whether the
+   role has the aforementioned privileges on the partition.  Of course,
+   superusers can always reindex anything.
   </para>
 
   <para>
diff --git a/doc/src/sgml/ref/vacuum.sgml b/doc/src/sgml/ref/vacuum.sgml
index 8fa8421847..545b23b54f 100644
--- a/doc/src/sgml/ref/vacuum.sgml
+++ b/doc/src/sgml/ref/vacuum.sgml
@@ -401,7 +401,10 @@ VACUUM [ FULL ] [ FREEZE ] [ VERBOSE ] [ ANALYZE ] [ <replaceable class="paramet
     vacuum all tables in their databases, except shared catalogs.
     (The restriction for shared catalogs means that a true database-wide
     <command>VACUUM</command> can only be performed by superusers and roles
-    with privileges of <literal>pg_maintain</literal>.)
+    with privileges of <literal>pg_maintain</literal>.)  If a role has
+    permission to <command>VACUUM</command> a partitioned table, it is also
+    permitted to <command>VACUUM</command> each of its partitions, regardless
+    of whether the role has the aforementioned privileges on the partition.
     <command>VACUUM</command> will skip over any tables that the calling user
     does not have permission to vacuum.
    </para>
diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c
index f11691aff7..369fea7c04 100644
--- a/src/backend/commands/cluster.c
+++ b/src/backend/commands/cluster.c
@@ -80,6 +80,7 @@ static void copy_table_data(Oid OIDNewHeap, Oid OIDOldHeap, Oid OIDOldIndex,
 static List *get_tables_to_cluster(MemoryContext cluster_context);
 static List *get_tables_to_cluster_partitioned(MemoryContext cluster_context,
 											   Oid indexOid);
+static bool cluster_is_permitted_for_relation(Oid relid, Oid userid);

 /*---------------------------------------------------------------------------
@@ -366,8 +367,7 @@ cluster_rel(Oid tableOid, Oid indexOid, ClusterParams *params)
 	if (recheck)
 	{
 		/* Check that the user still has privileges for the relation */
-		if (!object_ownercheck(RelationRelationId, tableOid, save_userid) &&
-			pg_class_aclcheck(tableOid, save_userid, ACL_MAINTAIN) != ACLCHECK_OK)
+		if (!cluster_is_permitted_for_relation(tableOid, save_userid))
 		{
 			relation_close(OldHeap, AccessExclusiveLock);
 			goto out;
@@ -1645,8 +1645,7 @@ get_tables_to_cluster(MemoryContext cluster_context)
 
 		index = (Form_pg_index) GETSTRUCT(indexTuple);
 
-		if (!object_ownercheck(RelationRelationId, index->indrelid, GetUserId()) &&
-			pg_class_aclcheck(index->indrelid, GetUserId(), ACL_MAINTAIN) != ACLCHECK_OK)
+		if (!cluster_is_permitted_for_relation(index->indrelid, GetUserId()))
 			continue;
 
 		/* Use a permanent memory context for the result list */
@@ -1694,12 +1693,11 @@ get_tables_to_cluster_partitioned(MemoryContext cluster_context, Oid indexOid)
 		if (get_rel_relkind(indexrelid) != RELKIND_INDEX)
 			continue;
 
-		/* Silently skip partitions which the user has no access to. */
-		if (!object_ownercheck(RelationRelationId, relid, GetUserId()) &&
-			pg_class_aclcheck(relid, GetUserId(), ACL_MAINTAIN) != ACLCHECK_OK &&
-			(!object_ownercheck(DatabaseRelationId, MyDatabaseId, GetUserId()) ||
-			 IsSharedRelation(relid)))
-			continue;
+		/*
+		 * We already checked that the user has privileges to CLUSTER the
+		 * partitioned table when we locked it earlier, so there's no need to
+		 * check the privileges again here.
+		 */
 
 		/* Use a permanent memory context for the result list */
 		old_context = MemoryContextSwitchTo(cluster_context);
@@ -1714,3 +1712,20 @@ get_tables_to_cluster_partitioned(MemoryContext cluster_context, Oid indexOid)
 
 	return rtcs;
 }
+
+/*
+ * Return whether userid has privileges to CLUSTER relid.  If not, this
+ * function emits a WARNING.
+ */
+static bool
+cluster_is_permitted_for_relation(Oid relid, Oid userid)
+{
+	if (pg_class_aclcheck(relid, userid, ACL_MAINTAIN) == ACLCHECK_OK ||
+		has_partition_ancestor_privs(relid, userid, ACL_MAINTAIN))
+		return true;
+
+	ereport(WARNING,
+			(errmsg("permission denied to cluster \"%s\", skipping it",
+					get_rel_name(relid))));
+	return false;
+}
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 8afc006f89..16ec0b114e 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -2796,9 +2796,9 @@ RangeVarCallbackForReindexIndex(const RangeVar *relation,
 
 	/* Check permissions */
 	table_oid = IndexGetRelation(relId, true);
-	if (!object_ownercheck(RelationRelationId, relId, GetUserId()) &&
-		OidIsValid(table_oid) &&
-		pg_class_aclcheck(table_oid, GetUserId(), ACL_MAINTAIN) != ACLCHECK_OK)
+	if (OidIsValid(table_oid) &&
+		pg_class_aclcheck(table_oid, GetUserId(), ACL_MAINTAIN) != ACLCHECK_OK &&
+		!has_partition_ancestor_privs(table_oid, GetUserId(), ACL_MAINTAIN))
 		aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_INDEX,
 					   relation->relname);
 
@@ -3008,16 +3008,16 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
 
 		/*
 		 * The table can be reindexed if the user has been granted MAINTAIN on
-		 * the table or the user is a superuser, the table owner, or the
-		 * database/schema owner (but in the latter case, only if it's not a
-		 * shared relation).  object_ownercheck includes the superuser case,
-		 * and depending on objectKind we already know that the user has
-		 * permission to run REINDEX on this database or schema per the
-		 * permission checks at the beginning of this routine.
+		 * the table or one of its partition ancestors or the user is a
+		 * superuser, the table owner, or the database/schema owner (but in the
+		 * latter case, only if it's not a shared relation).  pg_class_aclcheck
+		 * includes the superuser case, and depending on objectKind we already
+		 * know that the user has permission to run REINDEX on this database or
+		 * schema per the permission checks at the beginning of this routine.
 		 */
 		if (classtuple->relisshared &&
-			!object_ownercheck(RelationRelationId, relid, GetUserId()) &&
-			pg_class_aclcheck(relid, GetUserId(), ACL_MAINTAIN) != ACLCHECK_OK)
+			pg_class_aclcheck(relid, GetUserId(), ACL_MAINTAIN) != ACLCHECK_OK &&
+			!has_partition_ancestor_privs(relid, GetUserId(), ACL_MAINTAIN))
 			continue;
 
 		/*
diff --git a/src/backend/commands/lockcmds.c b/src/backend/commands/lockcmds.c
index 99e68bff85..de2a975882 100644
--- a/src/backend/commands/lockcmds.c
+++ b/src/backend/commands/lockcmds.c
@@ -19,6 +19,7 @@
 #include "catalog/namespace.h"
 #include "catalog/pg_inherits.h"
 #include "commands/lockcmds.h"
+#include "commands/tablecmds.h"
 #include "miscadmin.h"
 #include "nodes/nodeFuncs.h"
 #include "parser/parse_clause.h"
@@ -305,5 +306,12 @@ LockTableAclCheck(Oid reloid, LOCKMODE lockmode, Oid userid)
 
 	aclresult = pg_class_aclcheck(reloid, userid, aclmask);
 
+	/*
+	 * If this is a partition, check permissions of its ancestors if needed.
+	 */
+	if (aclresult != ACLCHECK_OK &&
+		has_partition_ancestor_privs(reloid, userid, ACL_MAINTAIN))
+		aclresult = ACLCHECK_OK;
+
 	return aclresult;
 }
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 1fbdad4b64..7c697a285b 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -16886,12 +16886,38 @@ RangeVarCallbackMaintainsTable(const RangeVar *relation,
 				 errmsg("\"%s\" is not a table or materialized view", relation->relname)));
 
 	/* Check permissions */
-	if (!object_ownercheck(RelationRelationId, relId, GetUserId()) &&
-		pg_class_aclcheck(relId, GetUserId(), ACL_MAINTAIN) != ACLCHECK_OK)
+	if (pg_class_aclcheck(relId, GetUserId(), ACL_MAINTAIN) != ACLCHECK_OK &&
+		!has_partition_ancestor_privs(relId, GetUserId(), ACL_MAINTAIN))
 		aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_TABLE,
 					   relation->relname);
 }
 
+/*
+ * If relid is a partition, returns whether userid has any of the privileges
+ * specified in acl on any of its ancestors.  Otherwise, returns false.
+ */
+bool
+has_partition_ancestor_privs(Oid relid, Oid userid, AclMode acl)
+{
+	List	   *ancestors;
+	ListCell   *lc;
+
+	if (!get_rel_relispartition(relid))
+		return false;
+
+	ancestors = get_partition_ancestors(relid);
+	foreach(lc, ancestors)
+	{
+		Oid			ancestor = lfirst_oid(lc);
+
+		if (OidIsValid(ancestor) &&
+			pg_class_aclcheck(ancestor, userid, acl) == ACLCHECK_OK)
+			return true;
+	}
+
+	return false;
+}
+
 /*
  * Callback to RangeVarGetRelidExtended() for TRUNCATE processing.
  */
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index c4ed7efce3..5908733347 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -41,6 +41,7 @@
 #include "catalog/pg_namespace.h"
 #include "commands/cluster.h"
 #include "commands/defrem.h"
+#include "commands/tablecmds.h"
 #include "commands/vacuum.h"
 #include "miscadmin.h"
 #include "nodes/makefuncs.h"
@@ -598,11 +599,13 @@ vacuum_is_permitted_for_relation(Oid relid, Form_pg_class reltuple,
 	 *   - the role owns the relation
 	 *   - the role owns the current database and the relation is not shared
 	 *   - the role has been granted the MAINTAIN privilege on the relation
+	 *   - the role has privileges to vacuum/analyze any of the relation's
+	 *     partition ancestors
 	 *----------
 	 */
-	if (object_ownercheck(RelationRelationId, relid, GetUserId()) ||
-		(object_ownercheck(DatabaseRelationId, MyDatabaseId, GetUserId()) && !reltuple->relisshared) ||
-		pg_class_aclcheck(relid, GetUserId(), ACL_MAINTAIN) == ACLCHECK_OK)
+	if ((object_ownercheck(DatabaseRelationId, MyDatabaseId, GetUserId()) && !reltuple->relisshared) ||
+		pg_class_aclcheck(relid, GetUserId(), ACL_MAINTAIN) == ACLCHECK_OK ||
+		has_partition_ancestor_privs(relid, GetUserId(), ACL_MAINTAIN))
 		return true;
 
 	relname = NameStr(reltuple->relname);
diff --git a/src/include/commands/tablecmds.h b/src/include/commands/tablecmds.h
index 2e717fa815..e7c2b91a58 100644
--- a/src/include/commands/tablecmds.h
+++ b/src/include/commands/tablecmds.h
@@ -98,6 +98,7 @@ extern void AtEOSubXact_on_commit_actions(bool isCommit,
 extern void RangeVarCallbackMaintainsTable(const RangeVar *relation,
 										   Oid relId, Oid oldRelId,
 										   void *arg);
+extern bool has_partition_ancestor_privs(Oid relid, Oid userid, AclMode acl);
 
 extern void RangeVarCallbackOwnsRelation(const RangeVar *relation,
 										 Oid relId, Oid oldRelId, void *arg);
diff --git a/src/test/isolation/expected/cluster-conflict-partition.out b/src/test/isolation/expected/cluster-conflict-partition.out
index 7acb675c97..8d21276996 100644
--- a/src/test/isolation/expected/cluster-conflict-partition.out
+++ b/src/test/isolation/expected/cluster-conflict-partition.out
@@ -22,14 +22,16 @@ starting permutation: s1_begin s1_lock_child s2_auth s2_cluster s1_commit s2_res
 step s1_begin: BEGIN;
 step s1_lock_child: LOCK cluster_part_tab1 IN SHARE UPDATE EXCLUSIVE MODE;
 step s2_auth: SET ROLE regress_cluster_part;
-step s2_cluster: CLUSTER cluster_part_tab USING cluster_part_ind;
+step s2_cluster: CLUSTER cluster_part_tab USING cluster_part_ind; <waiting ...>
 step s1_commit: COMMIT;
+step s2_cluster: <... completed>
 step s2_reset: RESET ROLE;
 
 starting permutation: s1_begin s2_auth s1_lock_child s2_cluster s1_commit s2_reset
 step s1_begin: BEGIN;
 step s2_auth: SET ROLE regress_cluster_part;
 step s1_lock_child: LOCK cluster_part_tab1 IN SHARE UPDATE EXCLUSIVE MODE;
-step s2_cluster: CLUSTER cluster_part_tab USING cluster_part_ind;
+step s2_cluster: CLUSTER cluster_part_tab USING cluster_part_ind; <waiting ...>
 step s1_commit: COMMIT;
+step s2_cluster: <... completed>
 step s2_reset: RESET ROLE;
diff --git a/src/test/isolation/specs/cluster-conflict-partition.spec b/src/test/isolation/specs/cluster-conflict-partition.spec
index 5091f684a9..ae38cb4ee3 100644
--- a/src/test/isolation/specs/cluster-conflict-partition.spec
+++ b/src/test/isolation/specs/cluster-conflict-partition.spec
@@ -27,11 +27,8 @@ step s2_auth           { SET ROLE regress_cluster_part; }
 step s2_cluster        { CLUSTER cluster_part_tab USING cluster_part_ind; }
 step s2_reset          { RESET ROLE; }
 
-# CLUSTER on the parent waits if locked, passes for all cases.
+# CLUSTER waits if locked, passes for all cases.
 permutation s1_begin s1_lock_parent s2_auth s2_cluster s1_commit s2_reset
 permutation s1_begin s2_auth s1_lock_parent s2_cluster s1_commit s2_reset
-
-# When taking a lock on a partition leaf, CLUSTER on the parent skips
-# the leaf, passes for all cases.
 permutation s1_begin s1_lock_child s2_auth s2_cluster s1_commit s2_reset
 permutation s1_begin s2_auth s1_lock_child s2_cluster s1_commit s2_reset
diff --git a/src/test/regress/expected/cluster.out b/src/test/regress/expected/cluster.out
index 542c2e098c..2eec483eaa 100644
--- a/src/test/regress/expected/cluster.out
+++ b/src/test/regress/expected/cluster.out
@@ -352,7 +352,9 @@ INSERT INTO clstr_3 VALUES (1);
 -- this user can only cluster clstr_1 and clstr_3, but the latter
 -- has not been clustered
 SET SESSION AUTHORIZATION regress_clstr_user;
+SET client_min_messages = ERROR;  -- order of "skipping" warnings may vary
 CLUSTER;
+RESET client_min_messages;
 SELECT * FROM clstr_1 UNION ALL
   SELECT * FROM clstr_2 UNION ALL
   SELECT * FROM clstr_3;
@@ -513,7 +515,7 @@ SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a
 -----------+----------
  ptnowner  | t
  ptnowner1 | f
- ptnowner2 | t
+ ptnowner2 | f
 (3 rows)
 
 DROP TABLE ptnowner;
diff --git a/src/test/regress/expected/vacuum.out b/src/test/regress/expected/vacuum.out
index d860be0e20..458adee7f8 100644
--- a/src/test/regress/expected/vacuum.out
+++ b/src/test/regress/expected/vacuum.out
@@ -353,20 +353,14 @@ ALTER TABLE vacowned_parted OWNER TO regress_vacuum;
 ALTER TABLE vacowned_part1 OWNER TO regress_vacuum;
 SET ROLE regress_vacuum;
 VACUUM vacowned_parted;
-WARNING:  permission denied to vacuum "vacowned_part2", skipping it
 VACUUM vacowned_part1;
 VACUUM vacowned_part2;
-WARNING:  permission denied to vacuum "vacowned_part2", skipping it
 ANALYZE vacowned_parted;
-WARNING:  permission denied to analyze "vacowned_part2", skipping it
 ANALYZE vacowned_part1;
 ANALYZE vacowned_part2;
-WARNING:  permission denied to analyze "vacowned_part2", skipping it
 VACUUM (ANALYZE) vacowned_parted;
-WARNING:  permission denied to vacuum "vacowned_part2", skipping it
 VACUUM (ANALYZE) vacowned_part1;
 VACUUM (ANALYZE) vacowned_part2;
-WARNING:  permission denied to vacuum "vacowned_part2", skipping it
 RESET ROLE;
 -- Only one partition owned by other user.
 ALTER TABLE vacowned_parted OWNER TO CURRENT_USER;
@@ -395,26 +389,14 @@ ALTER TABLE vacowned_parted OWNER TO regress_vacuum;
 ALTER TABLE vacowned_part1 OWNER TO CURRENT_USER;
 SET ROLE regress_vacuum;
 VACUUM vacowned_parted;
-WARNING:  permission denied to vacuum "vacowned_part1", skipping it
-WARNING:  permission denied to vacuum "vacowned_part2", skipping it
 VACUUM vacowned_part1;
-WARNING:  permission denied to vacuum "vacowned_part1", skipping it
 VACUUM vacowned_part2;
-WARNING:  permission denied to vacuum "vacowned_part2", skipping it
 ANALYZE vacowned_parted;
-WARNING:  permission denied to analyze "vacowned_part1", skipping it
-WARNING:  permission denied to analyze "vacowned_part2", skipping it
 ANALYZE vacowned_part1;
-WARNING:  permission denied to analyze "vacowned_part1", skipping it
 ANALYZE vacowned_part2;
-WARNING:  permission denied to analyze "vacowned_part2", skipping it
 VACUUM (ANALYZE) vacowned_parted;
-WARNING:  permission denied to vacuum "vacowned_part1", skipping it
-WARNING:  permission denied to vacuum "vacowned_part2", skipping it
 VACUUM (ANALYZE) vacowned_part1;
-WARNING:  permission denied to vacuum "vacowned_part1", skipping it
 VACUUM (ANALYZE) vacowned_part2;
-WARNING:  permission denied to vacuum "vacowned_part2", skipping it
 RESET ROLE;
 DROP TABLE vacowned;
 DROP TABLE vacowned_parted;
diff --git a/src/test/regress/sql/cluster.sql b/src/test/regress/sql/cluster.sql
index 6cb9c926c0..a4cfaae807 100644
--- a/src/test/regress/sql/cluster.sql
+++ b/src/test/regress/sql/cluster.sql
@@ -145,7 +145,9 @@ INSERT INTO clstr_3 VALUES (1);
 -- this user can only cluster clstr_1 and clstr_3, but the latter
 -- has not been clustered
 SET SESSION AUTHORIZATION regress_clstr_user;
+SET client_min_messages = ERROR;  -- order of "skipping" warnings may vary
 CLUSTER;
+RESET client_min_messages;
 SELECT * FROM clstr_1 UNION ALL
   SELECT * FROM clstr_2 UNION ALL
   SELECT * FROM clstr_3;
-- 
2.25.1

>From 8782e45511e3a2bc7052173b811315f7aca0ef00 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <[email-masked]>
Date: Fri, 13 Jan 2023 14:47:48 -0800
Subject: [PATCH v5 2/2] fix MAINTAIN privs for TOAST tables

---
 src/backend/commands/vacuum.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index 5908733347..070fbbb856 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -92,7 +92,8 @@ static void vac_truncate_clog(TransactionId frozenXID,
 							  MultiXactId minMulti,
 							  TransactionId lastSaneFrozenXid,
 							  MultiXactId lastSaneMinMulti);
-static bool vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params);
+static bool vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params,
+					   bool skip_privs);
 static double compute_parallel_delay(void);
 static VacOptValue get_vacoptval_from_boolean(DefElem *def);
 static bool vac_tid_reaped(ItemPointer itemptr, void *state);
@@ -502,7 +503,7 @@ vacuum(List *relations, VacuumParams *params,
 
 			if (params->options & VACOPT_VACUUM)
 			{
-				if (!vacuum_rel(vrel->oid, vrel->relation, params))
+				if (!vacuum_rel(vrel->oid, vrel->relation, params, false))
 					continue;
 			}
 
@@ -1831,7 +1832,7 @@ vac_truncate_clog(TransactionId frozenXID,
  *		At entry and exit, we are not inside a transaction.
  */
 static bool
-vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params)
+vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params, bool skip_privs)
 {
 	LOCKMODE	lmode;
 	Relation	rel;
@@ -1918,7 +1919,8 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params)
 	 * happen across multiple transactions where privileges could have changed
 	 * in-between.  Make sure to only generate logs for VACUUM in this case.
 	 */
-	if (!vacuum_is_permitted_for_relation(RelationGetRelid(rel),
+	if (!skip_privs &&
+		!vacuum_is_permitted_for_relation(RelationGetRelid(rel),
 										  rel->rd_rel,
 										  params->options & VACOPT_VACUUM))
 	{
@@ -2092,7 +2094,7 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params)
 	 * totally unimportant for toast relations.
 	 */
 	if (toast_relid != InvalidOid)
-		vacuum_rel(toast_relid, NULL, params);
+		vacuum_rel(toast_relid, NULL, params, true);
 
 	/*
 	 * Now release the session-level lock on the main table.
-- 
2.25.1

---

From: Nathan Bossart <[email-masked]>
Date: Fri, 13 Jan 2023 15:13:39 -0800

On Fri, Jan 13, 2023 at 02:56:26PM -0800, Nathan Bossart wrote:
> Okay.  Here is a new patch set.

And here is a rebased patch set (c44f633 changed the same LOCK TABLE docs).

>From 48be435025ba10235c821cbd298c43763cbc5a56 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <[email-masked]>
Date: Mon, 9 Jan 2023 14:29:24 -0800
Subject: [PATCH v6 1/2] fix MAINTAIN privs for partitions

---

 
+   <para>
+    To cluster a table, one must have the <literal>MAINTAIN</literal> privilege
+    on the table or be the table's owner, a superuser, or a role with
+    privileges of the
+    <link linkend="predefined-roles-table"><literal>pg_maintain</literal></link>
+    role.  If a role has permission to <command>CLUSTER</command> a partitioned
+    table, it is also permitted to <command>CLUSTER</command> each of its
+    partitions, regardless of whether the role has the aforementioned
+    privileges on the partition.  <command>CLUSTER</command> will skip over any
+    tables that the calling user does not have permission to cluster.
+   </para>
+
    <para>
     In cases where you are accessing single rows randomly
     within a table, the actual order of the data in the
diff --git a/doc/src/sgml/ref/lock.sgml b/doc/src/sgml/ref/lock.sgml
index 8524182211..5b3b2b793a 100644
--- a/doc/src/sgml/ref/lock.sgml
+++ b/doc/src/sgml/ref/lock.sgml
@@ -177,7 +177,10 @@ LOCK [ TABLE ] [ ONLY ] <replaceable class="parameter">name</replaceable> [ * ]
     MODE</literal> (or a less-conflicting mode as described in <xref
     linkend="explicit-locking"/>) is permitted. If a user has
     <literal>SELECT</literal> privileges on the table, <literal>ACCESS SHARE
-    MODE</literal> is permitted.
+    MODE</literal> is permitted.  If a role has permission to lock a
+    partitioned table, it is also permitted to lock each of its partitions,
+    regardless of whether the role has the aforementioned privileges on the
+    partition.
    </para>
 
    <para>
 
   <para>

 

 
 
 	return rtcs;
 }
+
+/*
 
 	/* Check permissions */

 		/*
 
 		/*
index 410d78b040..6bf1b815f0 100644

+	/*
 
 	/* Check permissions */
 
+/*
index ea1428dc8c..076b1e54eb 100644

 

 DROP TABLE ptnowner;

>From 06e39cac619b6c006e2263f98034dec7361ca842 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <[email-masked]>
Date: Fri, 13 Jan 2023 14:47:48 -0800
Subject: [PATCH v6 2/2] fix MAINTAIN privs for TOAST tables

---

index 076b1e54eb..7b1a4b127e 100644

 
 	/*

---

From: Nathan Bossart <[email-masked]>
Date: Tue, 13 Jun 2023 14:12:46 -0700

I've been reviewing ff9618e lately, and I'm wondering whether it has the
same problem that 19de0ab solved.  Specifically, ff9618e introduces
has_partition_ancestor_privs(), which is used to check whether a user has
MAINTAIN on any partition ancestors.  This involves syscache lookups, and
presently this function does not take any relation locks.  I did spend some
time trying to induce cache lookup errors, but I didn't have any luck.
However, unless this can be made safe without too much trouble, I think I'm
inclined to partially revert ff9618e, leaving the TOAST-related parts
intact.

By reverting the partition-related parts of ff9618e, users would need to
have MAINTAIN on the partition itself to perform the maintenance command.
MAINTAIN on the partitioned table would no longer be sufficient.  This is
more like how things work on supported versions today.  Privileges are
checked for each partition, so a command that flows down to all partitions
might refuse to process a partition (e.g., if the current user doesn't own
the partition).

In the future, perhaps we could reevaluate adding these partition ancestor
privilege checks, but I'd rather leave it out for now instead of
introducing behavior in v16 that is potentially buggy and difficult to
remove post-GA.

---

From: Michael Paquier <[email-masked]>
Date: Wed, 14 Jun 2023 08:16:15 +0900

> inclined to partially revert ff9618e, leaving the TOAST-related parts

Hmm.  get_rel_relispartition() and pg_class_aclcheck() are rather
reliable when it comes to that as far as it goes.  Still
get_partition_ancestors() is your problem, isn't it?  Indeed, it seems
like a bad idea to do partition tree lookups without at least an
AccessShareLock as you may finish with a list that makes
pg_class_aclcheck() complain on a missing relation.  The race is
pretty narrow, but a stop point in get_partition_ancestors() with some
partition tree manipulation should be enough to make operations like a
schema-wide REINDEX less transparent with missing relations at least.

has_partition_ancestor_privs() is used in
RangeVarCallbackMaintainsTable(), on top of that.  As written, it
encourages incorrect use patterns.

> introducing behavior in v16 that is potentially buggy and difficult to

While on it, this buzzes me:
 static bool
-vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params)
+vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params, bool skip_privs)

VacuumParams has been originally introduced to avoid extending
vacuum_rel() with a bunch of arguments, no?

So, yes, agreed about the removal of has_partition_ancestor_privs().
I am adding an open item assigned to you and Jeff.
--
Michael

---

From: Nathan Bossart <[email-masked]>
Date: Tue, 13 Jun 2023 16:54:42 -0700

> vacuum_rel() with a bunch of arguments, no?

Yeah, that could probably be moved into VacuumParams.

> I am adding an open item assigned to you and Jeff.

Thanks.  I suspect there's more discussion incoming, but I'm hoping to
close this item one way or another by 16beta2.

---

From: Nathan Bossart <[email-masked]>
Date: Wed, 14 Jun 2023 11:17:11 -0700

> close this item one way or another by 16beta2.

Concretely, I am proposing something like the attached patches.

>From 8e93be7a0b4218a31f83b9fca53aa224bc9f5788 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <[email-masked]>
Date: Wed, 14 Jun 2023 10:54:05 -0700
Subject: [PATCH v1 1/2] partial revert of ff9618e82a

---
 doc/src/sgml/ref/analyze.sgml                 |  5 +---
 doc/src/sgml/ref/cluster.sgml                 |  6 +---
 doc/src/sgml/ref/lock.sgml                    |  5 +---
 doc/src/sgml/ref/reindex.sgml                 |  6 +---
 doc/src/sgml/ref/vacuum.sgml                  |  5 +---
 src/backend/commands/cluster.c                | 10 ++-----
 src/backend/commands/indexcmds.c              | 19 +++++-------
 src/backend/commands/lockcmds.c               |  8 -----
 src/backend/commands/tablecmds.c              | 29 +------------------
 src/backend/commands/vacuum.c                 |  6 +---
 src/include/commands/tablecmds.h              |  1 -
 .../expected/cluster-conflict-partition.out   | 14 ++++-----
 .../specs/cluster-conflict-partition.spec     |  7 +++--
 src/test/regress/expected/cluster.out         |  3 +-
 src/test/regress/expected/vacuum.out          | 18 ++++++++++++
 15 files changed, 49 insertions(+), 93 deletions(-)

diff --git a/doc/src/sgml/ref/analyze.sgml b/doc/src/sgml/ref/analyze.sgml
index 20c6f9939f..30a893230e 100644
--- a/doc/src/sgml/ref/analyze.sgml
+++ b/doc/src/sgml/ref/analyze.sgml
@@ -190,10 +190,7 @@ ANALYZE [ VERBOSE ] [ <replaceable class="parameter">table_and_columns</replacea
    analyze all tables in their databases, except shared catalogs.
    (The restriction for shared catalogs means that a true database-wide
    <command>ANALYZE</command> can only be performed by superusers and roles
-   with privileges of <literal>pg_maintain</literal>.)  If a role has
-   permission to <command>ANALYZE</command> a partitioned table, it is also
-   permitted to <command>ANALYZE</command> each of its partitions, regardless
-   of whether the role has the aforementioned privileges on the partition.
+   with privileges of <literal>pg_maintain</literal>.)
    <command>ANALYZE</command> will skip over any tables that the calling user
    does not have permission to analyze.
   </para>
diff --git a/doc/src/sgml/ref/cluster.sgml b/doc/src/sgml/ref/cluster.sgml
index 29f0f1fd90..c768175499 100644
--- a/doc/src/sgml/ref/cluster.sgml
+++ b/doc/src/sgml/ref/cluster.sgml
@@ -137,11 +137,7 @@ CLUSTER [VERBOSE]
     on the table or be the table's owner, a superuser, or a role with
     privileges of the
     <link linkend="predefined-roles-table"><literal>pg_maintain</literal></link>
-    role.  If a role has permission to <command>CLUSTER</command> a partitioned
-    table, it is also permitted to <command>CLUSTER</command> each of its
-    partitions, regardless of whether the role has the aforementioned
-    privileges on the partition.  <command>CLUSTER</command> will skip over any
-    tables that the calling user does not have permission to cluster.
+    role.
    </para>
 
    <para>
diff --git a/doc/src/sgml/ref/lock.sgml b/doc/src/sgml/ref/lock.sgml
index 5b3b2b793a..8524182211 100644
--- a/doc/src/sgml/ref/lock.sgml
+++ b/doc/src/sgml/ref/lock.sgml
@@ -177,10 +177,7 @@ LOCK [ TABLE ] [ ONLY ] <replaceable class="parameter">name</replaceable> [ * ]
     MODE</literal> (or a less-conflicting mode as described in <xref
     linkend="explicit-locking"/>) is permitted. If a user has
     <literal>SELECT</literal> privileges on the table, <literal>ACCESS SHARE
-    MODE</literal> is permitted.  If a role has permission to lock a
-    partitioned table, it is also permitted to lock each of its partitions,
-    regardless of whether the role has the aforementioned privileges on the
-    partition.
+    MODE</literal> is permitted.
    </para>
 
    <para>
diff --git a/doc/src/sgml/ref/reindex.sgml b/doc/src/sgml/ref/reindex.sgml
index 71455dfdc7..23f8c7630b 100644
--- a/doc/src/sgml/ref/reindex.sgml
+++ b/doc/src/sgml/ref/reindex.sgml
@@ -306,11 +306,7 @@ REINDEX [ ( <replaceable class="parameter">option</replaceable> [, ...] ) ] { DA
    indexes on shared catalogs will be skipped unless the user owns the
    catalog (which typically won't be the case), has privileges of the
    <literal>pg_maintain</literal> role, or has the <literal>MAINTAIN</literal>
-   privilege on the catalog.  If a role has permission to
-   <command>REINDEX</command> a partitioned table, it is also permitted to
-   <command>REINDEX</command> each of its partitions, regardless of whether the
-   role has the aforementioned privileges on the partition.  Of course,
-   superusers can always reindex anything.
+   privilege on the catalog.  Of course, superusers can always reindex anything.
   </para>
 
   <para>
diff --git a/doc/src/sgml/ref/vacuum.sgml b/doc/src/sgml/ref/vacuum.sgml
index 57bc4c23ec..445325e14c 100644
--- a/doc/src/sgml/ref/vacuum.sgml
+++ b/doc/src/sgml/ref/vacuum.sgml
@@ -452,10 +452,7 @@ VACUUM [ FULL ] [ FREEZE ] [ VERBOSE ] [ ANALYZE ] [ <replaceable class="paramet
     vacuum all tables in their databases, except shared catalogs.
     (The restriction for shared catalogs means that a true database-wide
     <command>VACUUM</command> can only be performed by superusers and roles
-    with privileges of <literal>pg_maintain</literal>.)  If a role has
-    permission to <command>VACUUM</command> a partitioned table, it is also
-    permitted to <command>VACUUM</command> each of its partitions, regardless
-    of whether the role has the aforementioned privileges on the partition.
+    with privileges of <literal>pg_maintain</literal>.)
     <command>VACUUM</command> will skip over any tables that the calling user
     does not have permission to vacuum.
    </para>
diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c
index 369fea7c04..38834356b9 100644
--- a/src/backend/commands/cluster.c
+++ b/src/backend/commands/cluster.c
@@ -1693,11 +1693,8 @@ get_tables_to_cluster_partitioned(MemoryContext cluster_context, Oid indexOid)
 		if (get_rel_relkind(indexrelid) != RELKIND_INDEX)
 			continue;
 
-		/*
-		 * We already checked that the user has privileges to CLUSTER the
-		 * partitioned table when we locked it earlier, so there's no need to
-		 * check the privileges again here.
-		 */
+		if (!cluster_is_permitted_for_relation(relid, GetUserId()))
+			continue;
 
 		/* Use a permanent memory context for the result list */
 		old_context = MemoryContextSwitchTo(cluster_context);
@@ -1720,8 +1717,7 @@ get_tables_to_cluster_partitioned(MemoryContext cluster_context, Oid indexOid)
 static bool
 cluster_is_permitted_for_relation(Oid relid, Oid userid)
 {
-	if (pg_class_aclcheck(relid, userid, ACL_MAINTAIN) == ACLCHECK_OK ||
-		has_partition_ancestor_privs(relid, userid, ACL_MAINTAIN))
+	if (pg_class_aclcheck(relid, userid, ACL_MAINTAIN) == ACLCHECK_OK)
 		return true;
 
 	ereport(WARNING,
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index a5168c9f09..56bba91c08 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -2854,8 +2854,7 @@ RangeVarCallbackForReindexIndex(const RangeVar *relation,
 	/* Check permissions */
 	table_oid = IndexGetRelation(relId, true);
 	if (OidIsValid(table_oid) &&
-		pg_class_aclcheck(table_oid, GetUserId(), ACL_MAINTAIN) != ACLCHECK_OK &&
-		!has_partition_ancestor_privs(table_oid, GetUserId(), ACL_MAINTAIN))
+		pg_class_aclcheck(table_oid, GetUserId(), ACL_MAINTAIN) != ACLCHECK_OK)
 		aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_INDEX,
 					   relation->relname);
 
@@ -3065,17 +3064,15 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
 
 		/*
 		 * The table can be reindexed if the user has been granted MAINTAIN on
-		 * the table or one of its partition ancestors or the user is a
-		 * superuser, the table owner, or the database/schema owner (but in
-		 * the latter case, only if it's not a shared relation).
-		 * pg_class_aclcheck includes the superuser case, and depending on
-		 * objectKind we already know that the user has permission to run
-		 * REINDEX on this database or schema per the permission checks at the
-		 * beginning of this routine.
+		 * the table or the user is a superuser, the table owner, or the
+		 * database/schema owner (but in the latter case, only if it's not a
+		 * shared relation).  pg_class_aclcheck includes the superuser case,
+		 * and depending on objectKind we already know that the user has
+		 * permission to run REINDEX on this database or schema per the
+		 * permission checks at the beginning of this routine.
 		 */
 		if (classtuple->relisshared &&
-			pg_class_aclcheck(relid, GetUserId(), ACL_MAINTAIN) != ACLCHECK_OK &&
-			!has_partition_ancestor_privs(relid, GetUserId(), ACL_MAINTAIN))
+			pg_class_aclcheck(relid, GetUserId(), ACL_MAINTAIN) != ACLCHECK_OK)
 			continue;
 
 		/*
diff --git a/src/backend/commands/lockcmds.c b/src/backend/commands/lockcmds.c
index 43c7d7f4bb..92662cbbc8 100644
--- a/src/backend/commands/lockcmds.c
+++ b/src/backend/commands/lockcmds.c
@@ -19,7 +19,6 @@
 #include "catalog/namespace.h"
 #include "catalog/pg_inherits.h"
 #include "commands/lockcmds.h"
-#include "commands/tablecmds.h"
 #include "miscadmin.h"
 #include "nodes/nodeFuncs.h"
 #include "parser/parse_clause.h"
@@ -297,12 +296,5 @@ LockTableAclCheck(Oid reloid, LOCKMODE lockmode, Oid userid)

-	/*
-	 * If this is a partition, check permissions of its ancestors if needed.
-	 */
-	if (aclresult != ACLCHECK_OK &&
-		has_partition_ancestor_privs(reloid, userid, ACL_MAINTAIN))
-		aclresult = ACLCHECK_OK;
-
 	return aclresult;
 }
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 4d49d70c33..29eece1c2c 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -17006,38 +17006,11 @@ RangeVarCallbackMaintainsTable(const RangeVar *relation,
 				 errmsg("\"%s\" is not a table or materialized view", relation->relname)));
 
 	/* Check permissions */
-	if (pg_class_aclcheck(relId, GetUserId(), ACL_MAINTAIN) != ACLCHECK_OK &&
-		!has_partition_ancestor_privs(relId, GetUserId(), ACL_MAINTAIN))
+	if (pg_class_aclcheck(relId, GetUserId(), ACL_MAINTAIN) != ACLCHECK_OK)
 		aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_TABLE,
 					   relation->relname);
 }
 
-/*
- * If relid is a partition, returns whether userid has any of the privileges
- * specified in acl on any of its ancestors.  Otherwise, returns false.
- */
-bool
-has_partition_ancestor_privs(Oid relid, Oid userid, AclMode acl)
-{
-	List	   *ancestors;
-	ListCell   *lc;
-
-	if (!get_rel_relispartition(relid))
-		return false;
-
-	ancestors = get_partition_ancestors(relid);
-	foreach(lc, ancestors)
-	{
-		Oid			ancestor = lfirst_oid(lc);
-
-		if (OidIsValid(ancestor) &&
-			pg_class_aclcheck(ancestor, userid, acl) == ACLCHECK_OK)
-			return true;
-	}
-
-	return false;
-}
-
 /*
  * Callback to RangeVarGetRelidExtended() for TRUNCATE processing.
  */
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index a843f9ad92..2e41a31173 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -41,7 +41,6 @@
 #include "catalog/pg_namespace.h"
 #include "commands/cluster.h"
 #include "commands/defrem.h"
-#include "commands/tablecmds.h"
 #include "commands/vacuum.h"
 #include "miscadmin.h"
 #include "nodes/makefuncs.h"
@@ -719,13 +718,10 @@ vacuum_is_permitted_for_relation(Oid relid, Form_pg_class reltuple,
 	 *   - the role owns the relation
 	 *   - the role owns the current database and the relation is not shared
 	 *   - the role has been granted the MAINTAIN privilege on the relation
-	 *   - the role has privileges to vacuum/analyze any of the relation's
-	 *     partition ancestors
 	 *----------
 	 */
 	if ((object_ownercheck(DatabaseRelationId, MyDatabaseId, GetUserId()) && !reltuple->relisshared) ||
-		pg_class_aclcheck(relid, GetUserId(), ACL_MAINTAIN) == ACLCHECK_OK ||
-		has_partition_ancestor_privs(relid, GetUserId(), ACL_MAINTAIN))
+		pg_class_aclcheck(relid, GetUserId(), ACL_MAINTAIN) == ACLCHECK_OK)
 		return true;
 
 	relname = NameStr(reltuple->relname);
diff --git a/src/include/commands/tablecmds.h b/src/include/commands/tablecmds.h
index 17b9404937..250d89ff88 100644
--- a/src/include/commands/tablecmds.h
+++ b/src/include/commands/tablecmds.h
@@ -99,7 +99,6 @@ extern void AtEOSubXact_on_commit_actions(bool isCommit,
 extern void RangeVarCallbackMaintainsTable(const RangeVar *relation,
 										   Oid relId, Oid oldRelId,
 										   void *arg);
-extern bool has_partition_ancestor_privs(Oid relid, Oid userid, AclMode acl);
 
 extern void RangeVarCallbackOwnsRelation(const RangeVar *relation,
 										 Oid relId, Oid oldRelId, void *arg);
diff --git a/src/test/isolation/expected/cluster-conflict-partition.out b/src/test/isolation/expected/cluster-conflict-partition.out
index 8d21276996..7be9e56ef1 100644
--- a/src/test/isolation/expected/cluster-conflict-partition.out
+++ b/src/test/isolation/expected/cluster-conflict-partition.out
@@ -3,7 +3,7 @@ Parsed test spec with 2 sessions
 starting permutation: s1_begin s1_lock_parent s2_auth s2_cluster s1_commit s2_reset
 step s1_begin: BEGIN;
 step s1_lock_parent: LOCK cluster_part_tab IN SHARE UPDATE EXCLUSIVE MODE;
-step s2_auth: SET ROLE regress_cluster_part;
+step s2_auth: SET ROLE regress_cluster_part; SET client_min_messages = ERROR;
 step s2_cluster: CLUSTER cluster_part_tab USING cluster_part_ind; <waiting ...>
 step s1_commit: COMMIT;
 step s2_cluster: <... completed>
@@ -11,7 +11,7 @@ step s2_reset: RESET ROLE;
 
 starting permutation: s1_begin s2_auth s1_lock_parent s2_cluster s1_commit s2_reset
 step s1_begin: BEGIN;
-step s2_auth: SET ROLE regress_cluster_part;
+step s2_auth: SET ROLE regress_cluster_part; SET client_min_messages = ERROR;
 step s1_lock_parent: LOCK cluster_part_tab IN SHARE UPDATE EXCLUSIVE MODE;
 step s2_cluster: CLUSTER cluster_part_tab USING cluster_part_ind; <waiting ...>
 step s1_commit: COMMIT;
@@ -21,17 +21,15 @@ step s2_reset: RESET ROLE;
 starting permutation: s1_begin s1_lock_child s2_auth s2_cluster s1_commit s2_reset
 step s1_begin: BEGIN;
 step s1_lock_child: LOCK cluster_part_tab1 IN SHARE UPDATE EXCLUSIVE MODE;
-step s2_auth: SET ROLE regress_cluster_part;
-step s2_cluster: CLUSTER cluster_part_tab USING cluster_part_ind; <waiting ...>
+step s2_auth: SET ROLE regress_cluster_part; SET client_min_messages = ERROR;
+step s2_cluster: CLUSTER cluster_part_tab USING cluster_part_ind;
 step s1_commit: COMMIT;
-step s2_cluster: <... completed>
 step s2_reset: RESET ROLE;
 
 starting permutation: s1_begin s2_auth s1_lock_child s2_cluster s1_commit s2_reset
 step s1_begin: BEGIN;
-step s2_auth: SET ROLE regress_cluster_part;
+step s2_auth: SET ROLE regress_cluster_part; SET client_min_messages = ERROR;
 step s1_lock_child: LOCK cluster_part_tab1 IN SHARE UPDATE EXCLUSIVE MODE;
-step s2_cluster: CLUSTER cluster_part_tab USING cluster_part_ind; <waiting ...>
+step s2_cluster: CLUSTER cluster_part_tab USING cluster_part_ind;
 step s1_commit: COMMIT;
-step s2_cluster: <... completed>
 step s2_reset: RESET ROLE;
diff --git a/src/test/isolation/specs/cluster-conflict-partition.spec b/src/test/isolation/specs/cluster-conflict-partition.spec
index ae38cb4ee3..4d38a7f49a 100644
--- a/src/test/isolation/specs/cluster-conflict-partition.spec
+++ b/src/test/isolation/specs/cluster-conflict-partition.spec
@@ -23,12 +23,15 @@ step s1_lock_child     { LOCK cluster_part_tab1 IN SHARE UPDATE EXCLUSIVE MODE;
 step s1_commit         { COMMIT; }
 
 session s2
-step s2_auth           { SET ROLE regress_cluster_part; }
+step s2_auth           { SET ROLE regress_cluster_part; SET client_min_messages = ERROR; }
 step s2_cluster        { CLUSTER cluster_part_tab USING cluster_part_ind; }
 step s2_reset          { RESET ROLE; }
 
-# CLUSTER waits if locked, passes for all cases.
+# CLUSTER on the parent waits if locked, passes for all cases.
 permutation s1_begin s1_lock_parent s2_auth s2_cluster s1_commit s2_reset
 permutation s1_begin s2_auth s1_lock_parent s2_cluster s1_commit s2_reset
+
+# When taking a lock on a partition leaf, CLUSTER on the parent skips
+# the leaf, passes for all cases.
 permutation s1_begin s1_lock_child s2_auth s2_cluster s1_commit s2_reset
 permutation s1_begin s2_auth s1_lock_child s2_cluster s1_commit s2_reset
diff --git a/src/test/regress/expected/cluster.out b/src/test/regress/expected/cluster.out
index 2eec483eaa..27a5dff5d4 100644
--- a/src/test/regress/expected/cluster.out
+++ b/src/test/regress/expected/cluster.out
@@ -508,6 +508,7 @@ CREATE TEMP TABLE ptnowner_oldnodes AS
   JOIN pg_class AS c ON c.oid=tree.relid;
 SET SESSION AUTHORIZATION regress_ptnowner;
 CLUSTER ptnowner USING ptnowner_i_idx;
+WARNING:  permission denied to cluster "ptnowner2", skipping it
 RESET SESSION AUTHORIZATION;
 SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a
   JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C";
@@ -515,7 +516,7 @@ SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a
 -----------+----------
  ptnowner  | t
  ptnowner1 | f
- ptnowner2 | f
+ ptnowner2 | t
 (3 rows)
 
 DROP TABLE ptnowner;
diff --git a/src/test/regress/expected/vacuum.out b/src/test/regress/expected/vacuum.out
index 41e020cf20..4def90b805 100644
--- a/src/test/regress/expected/vacuum.out
+++ b/src/test/regress/expected/vacuum.out
@@ -442,14 +442,20 @@ ALTER TABLE vacowned_parted OWNER TO regress_vacuum;
 ALTER TABLE vacowned_part1 OWNER TO regress_vacuum;
 SET ROLE regress_vacuum;
 VACUUM vacowned_parted;
+WARNING:  permission denied to vacuum "vacowned_part2", skipping it
 VACUUM vacowned_part1;
 VACUUM vacowned_part2;
+WARNING:  permission denied to vacuum "vacowned_part2", skipping it
 ANALYZE vacowned_parted;
+WARNING:  permission denied to analyze "vacowned_part2", skipping it
 ANALYZE vacowned_part1;
 ANALYZE vacowned_part2;
+WARNING:  permission denied to analyze "vacowned_part2", skipping it
 VACUUM (ANALYZE) vacowned_parted;
+WARNING:  permission denied to vacuum "vacowned_part2", skipping it
 VACUUM (ANALYZE) vacowned_part1;
 VACUUM (ANALYZE) vacowned_part2;
+WARNING:  permission denied to vacuum "vacowned_part2", skipping it
 RESET ROLE;
 -- Only one partition owned by other user.
 ALTER TABLE vacowned_parted OWNER TO CURRENT_USER;
@@ -478,14 +484,26 @@ ALTER TABLE vacowned_parted OWNER TO regress_vacuum;
 ALTER TABLE vacowned_part1 OWNER TO CURRENT_USER;
 SET ROLE regress_vacuum;
 VACUUM vacowned_parted;
+WARNING:  permission denied to vacuum "vacowned_part1", skipping it
+WARNING:  permission denied to vacuum "vacowned_part2", skipping it
 VACUUM vacowned_part1;
+WARNING:  permission denied to vacuum "vacowned_part1", skipping it
 VACUUM vacowned_part2;
+WARNING:  permission denied to vacuum "vacowned_part2", skipping it
 ANALYZE vacowned_parted;
+WARNING:  permission denied to analyze "vacowned_part1", skipping it
+WARNING:  permission denied to analyze "vacowned_part2", skipping it
 ANALYZE vacowned_part1;
+WARNING:  permission denied to analyze "vacowned_part1", skipping it
 ANALYZE vacowned_part2;
+WARNING:  permission denied to analyze "vacowned_part2", skipping it
 VACUUM (ANALYZE) vacowned_parted;
+WARNING:  permission denied to vacuum "vacowned_part1", skipping it
+WARNING:  permission denied to vacuum "vacowned_part2", skipping it
 VACUUM (ANALYZE) vacowned_part1;
+WARNING:  permission denied to vacuum "vacowned_part1", skipping it
 VACUUM (ANALYZE) vacowned_part2;
+WARNING:  permission denied to vacuum "vacowned_part2", skipping it
 RESET ROLE;
 DROP TABLE vacowned;
 DROP TABLE vacowned_parted;
-- 
2.25.1

>From 9979b315350f6eb45768f5af9a6fac1b7ba3c869 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <[email-masked]>
Date: Wed, 14 Jun 2023 11:08:03 -0700
Subject: [PATCH v1 2/2] convert boolean parameter into a VACOPT_* option

---
 src/backend/commands/vacuum.c | 26 ++++++++++++++++++--------
 src/include/commands/vacuum.h |  1 +
 2 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index 2e41a31173..2e697dd7b6 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -114,7 +114,7 @@ static void vac_truncate_clog(TransactionId frozenXID,
 							  TransactionId lastSaneFrozenXid,
 							  MultiXactId lastSaneMinMulti);
 static bool vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params,
-					   bool skip_privs, BufferAccessStrategy bstrategy);
+					   BufferAccessStrategy bstrategy);
 static double compute_parallel_delay(void);
 static VacOptValue get_vacoptval_from_boolean(DefElem *def);
 static bool vac_tid_reaped(ItemPointer itemptr, void *state);
@@ -619,8 +619,7 @@ vacuum(List *relations, VacuumParams *params, BufferAccessStrategy bstrategy,
 
 			if (params->options & VACOPT_VACUUM)
 			{
-				if (!vacuum_rel(vrel->oid, vrel->relation, params, false,
-								bstrategy))
+				if (!vacuum_rel(vrel->oid, vrel->relation, params, bstrategy))
 					continue;
 			}
 
@@ -711,6 +710,13 @@ vacuum_is_permitted_for_relation(Oid relid, Form_pg_class reltuple,
 
 	Assert((options & (VACOPT_VACUUM | VACOPT_ANALYZE)) != 0);
 
+	/*
+	 * Privilege checks are bypassed in some cases (e.g., when recursing to a
+	 * relation's TOAST table).
+	 */
+	if (options & VACOPT_SKIP_PRIVS)
+		return true;
+
 	/*----------
 	 * A role has privileges to vacuum or analyze the relation if any of the
 	 * following are true:
@@ -1949,7 +1955,7 @@ vac_truncate_clog(TransactionId frozenXID,
  */
 static bool
 vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params,
-		   bool skip_privs, BufferAccessStrategy bstrategy)
+		   BufferAccessStrategy bstrategy)
 {
 	LOCKMODE	lmode;
 	Relation	rel;
@@ -2036,8 +2042,7 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params,
 	 * happen across multiple transactions where privileges could have changed
 	 * in-between.  Make sure to only generate logs for VACUUM in this case.
 	 */
-	if (!skip_privs &&
-		!vacuum_is_permitted_for_relation(RelationGetRelid(rel),
+	if (!vacuum_is_permitted_for_relation(RelationGetRelid(rel),
 										  rel->rd_rel,
 										  params->options & VACOPT_VACUUM))
 	{
@@ -2225,11 +2230,16 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params,
 	{
 		VacuumParams toast_vacuum_params;
 
-		/* force VACOPT_PROCESS_MAIN so vacuum_rel() processes it */
+		/*
+		 * Force VACOPT_PROCESS_MAIN so vacuum_rel() processes it.  Likewise,
+		 * set VACOPT_SKIP_PRIVS since privileges on the main relation are
+		 * sufficient to process it.
+		 */
 		memcpy(&toast_vacuum_params, params, sizeof(VacuumParams));
 		toast_vacuum_params.options |= VACOPT_PROCESS_MAIN;
+		toast_vacuum_params.options |= VACOPT_SKIP_PRIVS;
 
-		vacuum_rel(toast_relid, NULL, &toast_vacuum_params, true, bstrategy);
+		vacuum_rel(toast_relid, NULL, &toast_vacuum_params, bstrategy);
 	}
 
 	/*
diff --git a/src/include/commands/vacuum.h b/src/include/commands/vacuum.h
index 17e9b4f68e..cb5b11ab31 100644
--- a/src/include/commands/vacuum.h
+++ b/src/include/commands/vacuum.h
@@ -191,6 +191,7 @@ typedef struct VacAttrStats
 #define VACOPT_DISABLE_PAGE_SKIPPING 0x100	/* don't skip any pages */
 #define VACOPT_SKIP_DATABASE_STATS 0x200	/* skip vac_update_datfrozenxid() */
 #define VACOPT_ONLY_DATABASE_STATS 0x400	/* only vac_update_datfrozenxid() */
+#define VACOPT_SKIP_PRIVS 0x800 /* skip privilege checks */
 
 /*
  * Values used by index_cleanup and truncate params.
-- 
2.25.1

---

From: Jeff Davis <[email-masked]>
Date: Wed, 14 Jun 2023 11:30:45 -0700

> inclined to partially revert ff9618e, leaving the TOAST-related parts

Agreed. Having it work on partition hierarchies is a nice-to-have, but
not central to the usability of the feature. If it's causing problems,
best to take that out and reconsider in 17 if worthwhile.

Regards,
	Jeff Davis

---

From: Michael Paquier <[email-masked]>
Date: Thu, 15 Jun 2023 09:46:33 +0900

> Concretely, I am proposing something like the attached patches.

The result after 0001 is applied is that a couple of
object_ownercheck() calls that existed before ff9618e are removed from
some ACL checks in the REINDEX, CLUSTER and VACUUM paths.  Is that OK
for shared relations and shouldn't cluster_is_permitted_for_relation()
include that?  vacuum_is_permitted_for_relation() is consistent on
this side.

Here are the paths that now differ:
cluster_rel
get_tables_to_cluster
get_tables_to_cluster_partitioned
RangeVarCallbackForReindexIndex
ReindexMultipleTables

0002 looks OK to retain the skip check for toast relations in the
VACUUM case.
--
Michael

---

From: Nathan Bossart <[email-masked]>
Date: Wed, 14 Jun 2023 21:10:44 -0700

> include that?  vacuum_is_permitted_for_relation() is consistent on

These object_ownercheck() calls were removed because they were redundant,
as owners have all privileges by default.  Privileges can be revoked from
the owner, so an extra ownership check would effectively bypass the
relation's ACL in that case.  I looked around and didn't see any other
examples of a combined ownership and ACL check like we were doing for
MAINTAIN.  The only thing that gives me pause is that the docs call out
ownership as sufficient for some maintenance commands.  With these patches,
that's only true as long as no one revokes privileges from the owner.  IMO
we should update the docs and leave out the ownership checks since MAINTAIN
is now a grantable privilege like any other.  WDYT?

---

From: Nathan Bossart <[email-masked]>
Date: Thu, 15 Jun 2023 16:57:00 -0700

On Wed, Jun 14, 2023 at 09:10:44PM -0700, Nathan Bossart wrote:
> IMO

Here's an attempt at adjusting the documentation as I proposed yesterday.
I think this is a good opportunity to simplify the privilege-related
sections for these maintenance commands.

>From 70f760018914ffff13d85ac97f54b787c2e1b508 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <[email-masked]>
Date: Wed, 14 Jun 2023 10:54:05 -0700
Subject: [PATCH v2 1/2] partial revert of ff9618e82a

---
 doc/src/sgml/ref/analyze.sgml                 | 13 ++-------
 doc/src/sgml/ref/cluster.sgml                 |  9 +-----
 doc/src/sgml/ref/lock.sgml                    | 24 ++++++---------
 .../sgml/ref/refresh_materialized_view.sgml   | 17 +++++------
 doc/src/sgml/ref/reindex.sgml                 | 28 +++++++-----------
 doc/src/sgml/ref/vacuum.sgml                  | 13 ++-------
 doc/src/sgml/user-manag.sgml                  |  3 +-
 src/backend/commands/cluster.c                | 10 ++-----
 src/backend/commands/indexcmds.c              | 17 ++++-------
 src/backend/commands/lockcmds.c               |  8 -----
 src/backend/commands/tablecmds.c              | 29 +------------------
 src/backend/commands/vacuum.c                 |  6 +---
 src/include/commands/tablecmds.h              |  1 -
 .../expected/cluster-conflict-partition.out   | 14 ++++-----
 .../specs/cluster-conflict-partition.spec     |  7 +++--
 src/test/regress/expected/cluster.out         |  3 +-
 src/test/regress/expected/vacuum.out          | 18 ++++++++++++
 17 files changed, 74 insertions(+), 146 deletions(-)

diff --git a/doc/src/sgml/ref/analyze.sgml b/doc/src/sgml/ref/analyze.sgml
index 20c6f9939f..ecc7c884b4 100644
--- a/doc/src/sgml/ref/analyze.sgml
+++ b/doc/src/sgml/ref/analyze.sgml
@@ -183,17 +183,8 @@ ANALYZE [ VERBOSE ] [ <replaceable class="parameter">table_and_columns</replacea
 
   <para>
    To analyze a table, one must ordinarily have the <literal>MAINTAIN</literal>
-   privilege on the table or be the table's owner, a superuser, or a role with
-   privileges of the
-   <link linkend="predefined-roles-table"><literal>pg_maintain</literal></link>
-   role.  However, database owners are allowed to
-   analyze all tables in their databases, except shared catalogs.
-   (The restriction for shared catalogs means that a true database-wide
-   <command>ANALYZE</command> can only be performed by superusers and roles
-   with privileges of <literal>pg_maintain</literal>.)  If a role has
-   permission to <command>ANALYZE</command> a partitioned table, it is also
-   permitted to <command>ANALYZE</command> each of its partitions, regardless
-   of whether the role has the aforementioned privileges on the partition.
+   privilege on the table.  However, database owners are allowed to analyze all
+   tables in their databases, except shared catalogs.
    <command>ANALYZE</command> will skip over any tables that the calling user
    does not have permission to analyze.
   </para>
diff --git a/doc/src/sgml/ref/cluster.sgml b/doc/src/sgml/ref/cluster.sgml
index 29f0f1fd90..06f3d269e6 100644
--- a/doc/src/sgml/ref/cluster.sgml
+++ b/doc/src/sgml/ref/cluster.sgml
@@ -134,14 +134,7 @@ CLUSTER [VERBOSE]
 
    <para>
     To cluster a table, one must have the <literal>MAINTAIN</literal> privilege
-    on the table or be the table's owner, a superuser, or a role with
-    privileges of the
-    <link linkend="predefined-roles-table"><literal>pg_maintain</literal></link>
-    role.  If a role has permission to <command>CLUSTER</command> a partitioned
-    table, it is also permitted to <command>CLUSTER</command> each of its
-    partitions, regardless of whether the role has the aforementioned
-    privileges on the partition.  <command>CLUSTER</command> will skip over any
-    tables that the calling user does not have permission to cluster.
+    on the table.
    </para>
 
    <para>
diff --git a/doc/src/sgml/ref/lock.sgml b/doc/src/sgml/ref/lock.sgml
index 5b3b2b793a..d22c6f8384 100644
--- a/doc/src/sgml/ref/lock.sgml
+++ b/doc/src/sgml/ref/lock.sgml
@@ -166,21 +166,15 @@ LOCK [ TABLE ] [ ONLY ] <replaceable class="parameter">name</replaceable> [ * ]
 
    <para>
     To lock a table, the user must have the right privilege for the specified
-    <replaceable class="parameter">lockmode</replaceable>, or be the table's
-    owner, a superuser, or a role with privileges of the <link
-    linkend="predefined-roles-table"><literal>pg_maintain</literal></link>
-    role. If the user has <literal>MAINTAIN</literal>,
-    <literal>UPDATE</literal>, <literal>DELETE</literal>, or
-    <literal>TRUNCATE</literal> privileges on the table, any <replaceable
-    class="parameter">lockmode</replaceable> is permitted. If the user has
-    <literal>INSERT</literal> privileges on the table, <literal>ROW EXCLUSIVE
-    MODE</literal> (or a less-conflicting mode as described in <xref
-    linkend="explicit-locking"/>) is permitted. If a user has
-    <literal>SELECT</literal> privileges on the table, <literal>ACCESS SHARE
-    MODE</literal> is permitted.  If a role has permission to lock a
-    partitioned table, it is also permitted to lock each of its partitions,
-    regardless of whether the role has the aforementioned privileges on the
-    partition.
+    <replaceable class="parameter">lockmode</replaceable>.  If the user has
+    <literal>MAINTAIN</literal>, <literal>UPDATE</literal>,
+    <literal>DELETE</literal>, or <literal>TRUNCATE</literal> privileges on the
+    table, any <replaceable class="parameter">lockmode</replaceable> is
+    permitted. If the user has <literal>INSERT</literal> privileges on the
+    table, <literal>ROW EXCLUSIVE MODE</literal> (or a less-conflicting mode as
+    described in <xref linkend="explicit-locking"/>) is permitted. If a user
+    has <literal>SELECT</literal> privileges on the table,
+    <literal>ACCESS SHARE MODE</literal> is permitted.
    </para>
 
    <para>
diff --git a/doc/src/sgml/ref/refresh_materialized_view.sgml b/doc/src/sgml/ref/refresh_materialized_view.sgml
index 4d79b6ae7f..199a577d36 100644
--- a/doc/src/sgml/ref/refresh_materialized_view.sgml
+++ b/doc/src/sgml/ref/refresh_materialized_view.sgml
@@ -31,16 +31,13 @@ REFRESH MATERIALIZED VIEW [ CONCURRENTLY ] <replaceable class="parameter">name</
 
   <para>
    <command>REFRESH MATERIALIZED VIEW</command> completely replaces the
-   contents of a materialized view.  To execute this command you must be the
-   owner of the materialized view, have privileges of the
-   <link linkend="predefined-roles-table"><literal>pg_maintain</literal></link>
-   role, or have the <literal>MAINTAIN</literal>
-   privilege on the materialized view.  The old contents are discarded.  If
-   <literal>WITH DATA</literal> is specified (or defaults) the backing query
-   is executed to provide the new data, and the materialized view is left in a
-   scannable state.  If <literal>WITH NO DATA</literal> is specified no new
-   data is generated and the materialized view is left in an unscannable
-   state.
+   contents of a materialized view.  To execute this command you must have the
+   <literal>MAINTAIN</literal> privilege on the materialized view.  The old
+   contents are discarded.  If <literal>WITH DATA</literal> is specified (or
+   defaults) the backing query is executed to provide the new data, and the
+   materialized view is left in a scannable state.  If
+   <literal>WITH NO DATA</literal> is specified no new data is generated and
+   the materialized view is left in an unscannable state.
   </para>
   <para>
    <literal>CONCURRENTLY</literal> and <literal>WITH NO DATA</literal> may not
diff --git a/doc/src/sgml/ref/reindex.sgml b/doc/src/sgml/ref/reindex.sgml
index 71455dfdc7..a64b021e66 100644
--- a/doc/src/sgml/ref/reindex.sgml
+++ b/doc/src/sgml/ref/reindex.sgml
@@ -292,25 +292,17 @@ REINDEX [ ( <replaceable class="parameter">option</replaceable> [, ...] ) ] { DA
   </para>
 
   <para>
-   Reindexing a single index or table requires being the owner of that
-   index or table, having privileges of the
+   Reindexing a single index or table requires having the
+   <literal>MAINTAIN</literal> privilege on the table.  Reindexing a schema or
+   database requires being the owner of that schema or database or having
+   privileges of the
    <link linkend="predefined-roles-table"><literal>pg_maintain</literal></link>
-   role, or having the <literal>MAINTAIN</literal> privilege on the
-   table.  Reindexing a schema or database requires being the
-   owner of that schema or database or having privileges of the
-   <literal>pg_maintain</literal> role.  Note specifically that it's thus
-   possible for non-superusers to rebuild indexes of tables owned by
-   other users.  However, as a special exception, when
-   <command>REINDEX DATABASE</command>, <command>REINDEX SCHEMA</command>
-   or <command>REINDEX SYSTEM</command> is issued by a non-superuser,
-   indexes on shared catalogs will be skipped unless the user owns the
-   catalog (which typically won't be the case), has privileges of the
-   <literal>pg_maintain</literal> role, or has the <literal>MAINTAIN</literal>
-   privilege on the catalog.  If a role has permission to
-   <command>REINDEX</command> a partitioned table, it is also permitted to
-   <command>REINDEX</command> each of its partitions, regardless of whether the
-   role has the aforementioned privileges on the partition.  Of course,
-   superusers can always reindex anything.
+   role.  Note specifically that it's thus possible for non-superusers to
+   rebuild indexes of tables owned by other users.  However, as a special
+   exception, <command>REINDEX DATABASE</command>,
+   <command>REINDEX SCHEMA</command>, and <command>REINDEX SYSTEM</command>
+   will skip indexes on shared catalogs unless the user has the
+   <literal>MAINTAIN</literal> privilege on the catalog.
   </para>
 
   <para>
diff --git a/doc/src/sgml/ref/vacuum.sgml b/doc/src/sgml/ref/vacuum.sgml
index 57bc4c23ec..fa2ee76e25 100644
--- a/doc/src/sgml/ref/vacuum.sgml
+++ b/doc/src/sgml/ref/vacuum.sgml
@@ -445,17 +445,8 @@ VACUUM [ FULL ] [ FREEZE ] [ VERBOSE ] [ ANALYZE ] [ <replaceable class="paramet
 
    <para>
     To vacuum a table, one must ordinarily have the <literal>MAINTAIN</literal>
-    privilege on the table or be the table's owner, a superuser, or a role with
-    privileges of the
-    <link linkend="predefined-roles-table"><literal>pg_maintain</literal></link>
-    role.  However, database owners are allowed to
-    vacuum all tables in their databases, except shared catalogs.
-    (The restriction for shared catalogs means that a true database-wide
-    <command>VACUUM</command> can only be performed by superusers and roles
-    with privileges of <literal>pg_maintain</literal>.)  If a role has
-    permission to <command>VACUUM</command> a partitioned table, it is also
-    permitted to <command>VACUUM</command> each of its partitions, regardless
-    of whether the role has the aforementioned privileges on the partition.
+    privilege on the table.  However, database owners are allowed to vacuum all
+    tables in their databases, except shared catalogs.
     <command>VACUUM</command> will skip over any tables that the calling user
     does not have permission to vacuum.
    </para>
diff --git a/doc/src/sgml/user-manag.sgml b/doc/src/sgml/user-manag.sgml
index b6c37ccef2..e1540dd481 100644
--- a/doc/src/sgml/user-manag.sgml
+++ b/doc/src/sgml/user-manag.sgml
@@ -692,7 +692,8 @@ DROP ROLE doomed_role;
        <link linkend="sql-refreshmaterializedview"><command>REFRESH MATERIALIZED VIEW</command></link>,
        <link linkend="sql-reindex"><command>REINDEX</command></link>,
        and <link linkend="sql-lock"><command>LOCK TABLE</command></link> on all
-       relations.</entry>
+       relations, as if having <literal>MAINTAIN</literal> rights on those
+       objects, even without having it explicitly.</entry>
       </row>
       <row>
        <entry>pg_use_reserved_connections</entry>
diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c
index 369fea7c04..38834356b9 100644
--- a/src/backend/commands/cluster.c
+++ b/src/backend/commands/cluster.c
@@ -1693,11 +1693,8 @@ get_tables_to_cluster_partitioned(MemoryContext cluster_context, Oid indexOid)
 		if (get_rel_relkind(indexrelid) != RELKIND_INDEX)
 			continue;
 
-		/*

 	ereport(WARNING,
index a5168c9f09..ed28d13a16 100644
 
@@ -3064,18 +3063,12 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
 			continue;
 
 		/*
-		 * The table can be reindexed if the user has been granted MAINTAIN on
-		 * the table or one of its partition ancestors or the user is a
-		 * superuser, the table owner, or the database/schema owner (but in
-		 * the latter case, only if it's not a shared relation).
-		 * pg_class_aclcheck includes the superuser case, and depending on
-		 * objectKind we already know that the user has permission to run
-		 * REINDEX on this database or schema per the permission checks at the
-		 * beginning of this routine.
+		 * We already checked privileges on the database or schema, but we
+		 * further restrict reindexing shared catalogs to roles with the
+		 * MAINTAIN privilege on the relation.
 		 */
 		if (classtuple->relisshared &&
-			pg_class_aclcheck(relid, GetUserId(), ACL_MAINTAIN) != ACLCHECK_OK &&
-			!has_partition_ancestor_privs(relid, GetUserId(), ACL_MAINTAIN))
+			pg_class_aclcheck(relid, GetUserId(), ACL_MAINTAIN) != ACLCHECK_OK)
 			continue;
 
 		/*

-	/*
 
 	/* Check permissions */
 
-/*

 

 session s2

 DROP TABLE ptnowner;

>From 6a564c1f5e4b0becb77b02a2b4b81d98a009795e Mon Sep 17 00:00:00 2001
From: Nathan Bossart <[email-masked]>
Date: Wed, 14 Jun 2023 11:08:03 -0700
Subject: [PATCH v2 2/2] convert boolean parameter into a VACOPT_* option

---

 

+	/*

 
 	/*
 
 /*

---

From: Nathan Bossart <[email-masked]>
Date: Thu, 15 Jun 2023 22:20:25 -0700

> sections for these maintenance commands.

I noticed that it was possible to make the documentation changes in 0001
easier to read.  Apologies for the noise.

>From ab9f6433d50a357d2df00f01567880d8f587a44c Mon Sep 17 00:00:00 2001
From: Nathan Bossart <[email-masked]>
Date: Wed, 14 Jun 2023 10:54:05 -0700
Subject: [PATCH v3 1/2] partial revert of ff9618e82a

---
 doc/src/sgml/ref/analyze.sgml                 | 11 +------
 doc/src/sgml/ref/cluster.sgml                 |  9 +-----
 doc/src/sgml/ref/lock.sgml                    | 11 ++-----
 .../sgml/ref/refresh_materialized_view.sgml   |  6 ++--
 doc/src/sgml/ref/reindex.sgml                 | 27 +++++++----------
 doc/src/sgml/ref/vacuum.sgml                  | 11 +------
 doc/src/sgml/user-manag.sgml                  |  3 +-
 src/backend/commands/cluster.c                | 10 ++-----
 src/backend/commands/indexcmds.c              | 17 ++++-------
 src/backend/commands/lockcmds.c               |  8 -----
 src/backend/commands/tablecmds.c              | 29 +------------------
 src/backend/commands/vacuum.c                 |  6 +---
 src/include/commands/tablecmds.h              |  1 -
 .../expected/cluster-conflict-partition.out   | 14 ++++-----
 .../specs/cluster-conflict-partition.spec     |  7 +++--
 src/test/regress/expected/cluster.out         |  3 +-
 src/test/regress/expected/vacuum.out          | 18 ++++++++++++
 17 files changed, 61 insertions(+), 130 deletions(-)

index 20c6f9939f..954491b5df 100644
 
   <para>
+   privilege on the table.  However, database owners are allowed to
 
    <para>
 
    <para>
diff --git a/doc/src/sgml/ref/lock.sgml b/doc/src/sgml/ref/lock.sgml
index 5b3b2b793a..070855da18 100644
--- a/doc/src/sgml/ref/lock.sgml
+++ b/doc/src/sgml/ref/lock.sgml
@@ -166,10 +166,8 @@ LOCK [ TABLE ] [ ONLY ] <replaceable class="parameter">name</replaceable> [ * ]
 
    <para>
     To lock a table, the user must have the right privilege for the specified
-    <replaceable class="parameter">lockmode</replaceable>, or be the table's
-    owner, a superuser, or a role with privileges of the <link
-    linkend="predefined-roles-table"><literal>pg_maintain</literal></link>
-    role. If the user has <literal>MAINTAIN</literal>,
+    <replaceable class="parameter">lockmode</replaceable>.
+    If the user has <literal>MAINTAIN</literal>,
     <literal>UPDATE</literal>, <literal>DELETE</literal>, or
     <literal>TRUNCATE</literal> privileges on the table, any <replaceable
     class="parameter">lockmode</replaceable> is permitted. If the user has
@@ -177,10 +175,7 @@ LOCK [ TABLE ] [ ONLY ] <replaceable class="parameter">name</replaceable> [ * ]
     MODE</literal> (or a less-conflicting mode as described in <xref
     linkend="explicit-locking"/>) is permitted. If a user has
     <literal>SELECT</literal> privileges on the table, <literal>ACCESS SHARE
-    MODE</literal> is permitted.  If a role has permission to lock a
-    partitioned table, it is also permitted to lock each of its partitions,
-    regardless of whether the role has the aforementioned privileges on the
-    partition.
+    MODE</literal> is permitted.
    </para>
 
    <para>
diff --git a/doc/src/sgml/ref/refresh_materialized_view.sgml b/doc/src/sgml/ref/refresh_materialized_view.sgml
index 4d79b6ae7f..19737668cd 100644
--- a/doc/src/sgml/ref/refresh_materialized_view.sgml
+++ b/doc/src/sgml/ref/refresh_materialized_view.sgml
@@ -31,10 +31,8 @@ REFRESH MATERIALIZED VIEW [ CONCURRENTLY ] <replaceable class="parameter">name</
 
   <para>
    <command>REFRESH MATERIALIZED VIEW</command> completely replaces the
-   contents of a materialized view.  To execute this command you must be the
-   owner of the materialized view, have privileges of the
-   <link linkend="predefined-roles-table"><literal>pg_maintain</literal></link>
-   role, or have the <literal>MAINTAIN</literal>
+   contents of a materialized view.  To execute this command you must have the
+   <literal>MAINTAIN</literal>
    privilege on the materialized view.  The old contents are discarded.  If
    <literal>WITH DATA</literal> is specified (or defaults) the backing query
    is executed to provide the new data, and the materialized view is left in a
diff --git a/doc/src/sgml/ref/reindex.sgml b/doc/src/sgml/ref/reindex.sgml
index 71455dfdc7..583553b8a3 100644
--- a/doc/src/sgml/ref/reindex.sgml
+++ b/doc/src/sgml/ref/reindex.sgml
@@ -292,25 +292,18 @@ REINDEX [ ( <replaceable class="parameter">option</replaceable> [, ...] ) ] { DA
   </para>
 
   <para>
-   Reindexing a single index or table requires being the owner of that
-   index or table, having privileges of the
+   Reindexing a single index or table requires having the
+   <literal>MAINTAIN</literal> privilege on the table.  Reindexing a schema or
+   database requires being the owner of that schema or database or having
+   privileges of the
    <link linkend="predefined-roles-table"><literal>pg_maintain</literal></link>
-   role, or having the <literal>MAINTAIN</literal> privilege on the
-   table.  Reindexing a schema or database requires being the
-   owner of that schema or database or having privileges of the
-   <literal>pg_maintain</literal> role.  Note specifically that it's thus
+   role.  Note specifically that it's thus
    possible for non-superusers to rebuild indexes of tables owned by
-   other users.  However, as a special exception, when
-   <command>REINDEX DATABASE</command>, <command>REINDEX SCHEMA</command>
-   or <command>REINDEX SYSTEM</command> is issued by a non-superuser,
-   indexes on shared catalogs will be skipped unless the user owns the
-   catalog (which typically won't be the case), has privileges of the
-   <literal>pg_maintain</literal> role, or has the <literal>MAINTAIN</literal>
-   privilege on the catalog.  If a role has permission to
-   <command>REINDEX</command> a partitioned table, it is also permitted to
-   <command>REINDEX</command> each of its partitions, regardless of whether the
-   role has the aforementioned privileges on the partition.  Of course,
-   superusers can always reindex anything.
+   other users.  However, as a special exception,
+   <command>REINDEX DATABASE</command>, <command>REINDEX SCHEMA</command>, and
+   <command>REINDEX SYSTEM</command> will skip indexes on shared catalogs
+   unless the user has the <literal>MAINTAIN</literal> privilege on the
+   catalog.
   </para>
 
   <para>
index 57bc4c23ec..c42bbea9e2 100644
 
    <para>
+    privilege on the table.  However, database owners are allowed to
 
-		/*

 	ereport(WARNING,

 		/*
 
 		/*

-	/*
 
 	/* Check permissions */
 
-/*

 

 session s2

 DROP TABLE ptnowner;

>From c36f4d2843b4316e247055c5df0ce1336c33e11e Mon Sep 17 00:00:00 2001
From: Nathan Bossart <[email-masked]>
Date: Wed, 14 Jun 2023 11:08:03 -0700
Subject: [PATCH v3 2/2] convert boolean parameter into a VACOPT_* option

---

 

+	/*

 
 	/*
 
 /*

---

From: Nathan Bossart <[email-masked]>
Date: Mon, 19 Jun 2023 14:55:34 -0700

> easier to read.  Apologies for the noise.

Yet more noise...

In v4 of the patch set, I moved the skip_privs flag refactoring to 0001.  I
intend to commit this tomorrow unless there is additional feedback.

I split out the documentation simplifications to 0003 since it seemed
independent.  Also, I adjusted some ACL-related error messages in 0002 to
appropriately use "permission denied" messages instead of "must be owner"
messages.  I'm hoping to commit 0002 and 0003 by the end of the week so
that these fixes are available in 16beta2.

>From fcf57f8641ef3b44f12a7af699b3e4e9613d5b76 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <[email-masked]>
Date: Wed, 14 Jun 2023 11:08:03 -0700
Subject: [PATCH v4 1/3] convert boolean parameter into a VACOPT_* option

---

diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index a843f9ad92..987b11d16c 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -115,7 +115,7 @@ static void vac_truncate_clog(TransactionId frozenXID,
 							  TransactionId lastSaneFrozenXid,
 							  MultiXactId lastSaneMinMulti);
 static bool vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params,
-					   bool skip_privs, BufferAccessStrategy bstrategy);
+					   BufferAccessStrategy bstrategy);
 static double compute_parallel_delay(void);
 static VacOptValue get_vacoptval_from_boolean(DefElem *def);
 static bool vac_tid_reaped(ItemPointer itemptr, void *state);
@@ -620,8 +620,7 @@ vacuum(List *relations, VacuumParams *params, BufferAccessStrategy bstrategy,

@@ -712,6 +711,13 @@ vacuum_is_permitted_for_relation(Oid relid, Form_pg_class reltuple,

+	/*
@@ -1953,7 +1959,7 @@ vac_truncate_clog(TransactionId frozenXID,
@@ -2040,8 +2046,7 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params,
@@ -2229,11 +2234,16 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params,

 
 	/*
 
 /*

>From 9f34ab133d021a18711f0a08cd7a1f636f8f3927 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <[email-masked]>
Date: Mon, 19 Jun 2023 13:57:57 -0700
Subject: [PATCH v4 2/3] partial revert of ff9618e82a

---
 doc/src/sgml/ref/analyze.sgml                 |  5 +--
 doc/src/sgml/ref/cluster.sgml                 |  5 +--
 doc/src/sgml/ref/lock.sgml                    |  5 +--
 doc/src/sgml/ref/reindex.sgml                 |  6 +---
 doc/src/sgml/ref/vacuum.sgml                  |  5 +--
 src/backend/commands/cluster.c                | 10 ++----
 src/backend/commands/indexcmds.c              | 27 +++++++--------
 src/backend/commands/lockcmds.c               |  8 -----
 src/backend/commands/tablecmds.c              | 34 +++----------------
 src/backend/commands/vacuum.c                 |  8 +----
 src/include/commands/tablecmds.h              |  1 -
 .../expected/cluster-conflict-partition.out   | 14 ++++----
 .../specs/cluster-conflict-partition.spec     |  7 ++--
 src/test/regress/expected/cluster.out         |  3 +-
 src/test/regress/expected/create_index.out    |  4 +--
 src/test/regress/expected/privileges.out      |  8 ++---
 src/test/regress/expected/vacuum.out          | 18 ++++++++++
 17 files changed, 62 insertions(+), 106 deletions(-)

index 29f0f1fd90..f0dd7faed5 100644
@@ -137,10 +137,7 @@ CLUSTER [VERBOSE]
+    role.  <command>CLUSTER</command> will skip over any
     tables that the calling user does not have permission to cluster.
    </para>

    <para>
 
   <para>
 
-		/*

 	ereport(WARNING,
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index a5168c9f09..9bc97e1fc2 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -2853,11 +2853,14 @@ RangeVarCallbackForReindexIndex(const RangeVar *relation,
 
 	/* Check permissions */
 	table_oid = IndexGetRelation(relId, true);
-	if (OidIsValid(table_oid) &&
-		pg_class_aclcheck(table_oid, GetUserId(), ACL_MAINTAIN) != ACLCHECK_OK &&
-		!has_partition_ancestor_privs(table_oid, GetUserId(), ACL_MAINTAIN))
-		aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_INDEX,
-					   relation->relname);
+	if (OidIsValid(table_oid))
+	{
+		AclResult	aclresult;
+
+		aclresult = pg_class_aclcheck(table_oid, GetUserId(), ACL_MAINTAIN);
+		if (aclresult != ACLCHECK_OK)
+			aclcheck_error(aclresult, OBJECT_INDEX, relation->relname);
+	}
 
 	/* Lock heap before index to avoid deadlock. */
 	if (relId != oldRelId)
@@ -3064,18 +3067,12 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
 			continue;
 
 		/*
 
 		/*

-	/*
-	 * If this is a partition, check permissions of its ancestors if needed.
-	 */
-	if (aclresult != ACLCHECK_OK &&
-		has_partition_ancestor_privs(reloid, userid, ACL_MAINTAIN))
-		aclresult = ACLCHECK_OK;
-
 	return aclresult;
 }
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 4d49d70c33..9b12bc44d7 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -16986,6 +16986,7 @@ RangeVarCallbackMaintainsTable(const RangeVar *relation,
 							   Oid relId, Oid oldRelId, void *arg)
 {
 	char		relkind;
+	AclResult	aclresult;
 
 	/* Nothing to do if the relation was not found. */
 	if (!OidIsValid(relId))
@@ -17006,36 +17007,9 @@ RangeVarCallbackMaintainsTable(const RangeVar *relation,
 				 errmsg("\"%s\" is not a table or materialized view", relation->relname)));
 
 	/* Check permissions */
-	if (pg_class_aclcheck(relId, GetUserId(), ACL_MAINTAIN) != ACLCHECK_OK &&
-		!has_partition_ancestor_privs(relId, GetUserId(), ACL_MAINTAIN))
-		aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_TABLE,
-					   relation->relname);
-}
-
-/*
- * If relid is a partition, returns whether userid has any of the privileges
- * specified in acl on any of its ancestors.  Otherwise, returns false.
- */
-bool
-has_partition_ancestor_privs(Oid relid, Oid userid, AclMode acl)
-{
-	List	   *ancestors;
-	ListCell   *lc;
-
-	if (!get_rel_relispartition(relid))
-		return false;
-
-	ancestors = get_partition_ancestors(relid);
-	foreach(lc, ancestors)
-	{
-		Oid			ancestor = lfirst_oid(lc);
-
-		if (OidIsValid(ancestor) &&
-			pg_class_aclcheck(ancestor, userid, acl) == ACLCHECK_OK)
-			return true;
-	}
-
-	return false;
+	aclresult = pg_class_aclcheck(relId, GetUserId(), ACL_MAINTAIN);
+	if (aclresult != ACLCHECK_OK)
+		aclcheck_error(aclresult, OBJECT_TABLE, relation->relname);
 }
 
 /*
index 987b11d16c..11c23de8c1 100644
@@ -721,17 +720,12 @@ vacuum_is_permitted_for_relation(Oid relid, Form_pg_class reltuple,
-	 *   - the role is a superuser
-	 *   - the role owns the relation

 

 session s2

 DROP TABLE ptnowner;
diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out
index acfd9d1f4f..1473bc3175 100644
--- a/src/test/regress/expected/create_index.out
+++ b/src/test/regress/expected/create_index.out
@@ -2831,9 +2831,9 @@ RESET ROLE;
 GRANT USAGE ON SCHEMA pg_toast TO regress_reindexuser;
 SET SESSION ROLE regress_reindexuser;
 REINDEX TABLE pg_toast.pg_toast_1260;
-ERROR:  must be owner of table pg_toast_1260
+ERROR:  permission denied for table pg_toast_1260
 REINDEX INDEX pg_toast.pg_toast_1260_index;
-ERROR:  must be owner of index pg_toast_1260_index
+ERROR:  permission denied for index pg_toast_1260_index
 -- Clean up
 RESET ROLE;
 REVOKE USAGE ON SCHEMA pg_toast FROM regress_reindexuser;
diff --git a/src/test/regress/expected/privileges.out b/src/test/regress/expected/privileges.out
index 3cf4ac8c9e..3e4dfcc2ec 100644
--- a/src/test/regress/expected/privileges.out
+++ b/src/test/regress/expected/privileges.out
@@ -2928,13 +2928,13 @@ WARNING:  permission denied to analyze "maintain_test", skipping it
 VACUUM (ANALYZE) maintain_test;
 WARNING:  permission denied to vacuum "maintain_test", skipping it
 CLUSTER maintain_test USING maintain_test_a_idx;
-ERROR:  must be owner of table maintain_test
+ERROR:  permission denied for table maintain_test
 REFRESH MATERIALIZED VIEW refresh_test;
-ERROR:  must be owner of table refresh_test
+ERROR:  permission denied for table refresh_test
 REINDEX TABLE maintain_test;
-ERROR:  must be owner of table maintain_test
+ERROR:  permission denied for table maintain_test
 REINDEX INDEX maintain_test_a_idx;
-ERROR:  must be owner of index maintain_test_a_idx
+ERROR:  permission denied for index maintain_test_a_idx
 REINDEX SCHEMA reindex_test;
 ERROR:  must be owner of schema reindex_test
 RESET ROLE;
diff --git a/src/test/regress/expected/vacuum.out b/src/test/regress/expected/vacuum.out
index 41e020cf20..4def90b805 100644
--- a/src/test/regress/expected/vacuum.out
+++ b/src/test/regress/expected/vacuum.out
@@ -442,14 +442,20 @@ ALTER TABLE vacowned_parted OWNER TO regress_vacuum;
 ALTER TABLE vacowned_part1 OWNER TO regress_vacuum;
 SET ROLE regress_vacuum;
 VACUUM vacowned_parted;
+WARNING:  permission denied to vacuum "vacowned_part2", skipping it
 VACUUM vacowned_part1;
 VACUUM vacowned_part2;
+WARNING:  permission denied to vacuum "vacowned_part2", skipping it
 ANALYZE vacowned_parted;
+WARNING:  permission denied to analyze "vacowned_part2", skipping it
 ANALYZE vacowned_part1;
 ANALYZE vacowned_part2;
+WARNING:  permission denied to analyze "vacowned_part2", skipping it
 VACUUM (ANALYZE) vacowned_parted;
+WARNING:  permission denied to vacuum "vacowned_part2", skipping it
 VACUUM (ANALYZE) vacowned_part1;
 VACUUM (ANALYZE) vacowned_part2;
+WARNING:  permission denied to vacuum "vacowned_part2", skipping it
 RESET ROLE;
 -- Only one partition owned by other user.
 ALTER TABLE vacowned_parted OWNER TO CURRENT_USER;
@@ -478,14 +484,26 @@ ALTER TABLE vacowned_parted OWNER TO regress_vacuum;
 ALTER TABLE vacowned_part1 OWNER TO CURRENT_USER;
 SET ROLE regress_vacuum;
 VACUUM vacowned_parted;
+WARNING:  permission denied to vacuum "vacowned_part1", skipping it
+WARNING:  permission denied to vacuum "vacowned_part2", skipping it
 VACUUM vacowned_part1;
+WARNING:  permission denied to vacuum "vacowned_part1", skipping it
 VACUUM vacowned_part2;
+WARNING:  permission denied to vacuum "vacowned_part2", skipping it
 ANALYZE vacowned_parted;
+WARNING:  permission denied to analyze "vacowned_part1", skipping it
+WARNING:  permission denied to analyze "vacowned_part2", skipping it
 ANALYZE vacowned_part1;
+WARNING:  permission denied to analyze "vacowned_part1", skipping it
 ANALYZE vacowned_part2;
+WARNING:  permission denied to analyze "vacowned_part2", skipping it
 VACUUM (ANALYZE) vacowned_parted;
+WARNING:  permission denied to vacuum "vacowned_part1", skipping it
+WARNING:  permission denied to vacuum "vacowned_part2", skipping it
 VACUUM (ANALYZE) vacowned_part1;
+WARNING:  permission denied to vacuum "vacowned_part1", skipping it
 VACUUM (ANALYZE) vacowned_part2;
+WARNING:  permission denied to vacuum "vacowned_part2", skipping it
 RESET ROLE;
 DROP TABLE vacowned;
 DROP TABLE vacowned_parted;
-- 
2.25.1

>From dc282791b34bbca6895e12f01724f05cb0c84740 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <[email-masked]>
Date: Mon, 19 Jun 2023 14:07:26 -0700
Subject: [PATCH v4 3/3] simplify privilege-related documentation for
 maintenance commands

---
 doc/src/sgml/ref/analyze.sgml                 |  8 +------
 doc/src/sgml/ref/cluster.sgml                 |  6 +-----
 doc/src/sgml/ref/lock.sgml                    |  6 ++----
 .../sgml/ref/refresh_materialized_view.sgml   |  6 ++----
 doc/src/sgml/ref/reindex.sgml                 | 21 ++++++++-----------
 doc/src/sgml/ref/vacuum.sgml                  |  8 +------
 doc/src/sgml/user-manag.sgml                  |  3 ++-
 7 files changed, 18 insertions(+), 40 deletions(-)

diff --git a/doc/src/sgml/ref/analyze.sgml b/doc/src/sgml/ref/analyze.sgml
index 30a893230e..954491b5df 100644
--- a/doc/src/sgml/ref/analyze.sgml
+++ b/doc/src/sgml/ref/analyze.sgml
@@ -183,14 +183,8 @@ ANALYZE [ VERBOSE ] [ <replaceable class="parameter">table_and_columns</replacea
 
   <para>
index f0dd7faed5..06f3d269e6 100644
@@ -134,11 +134,7 @@ CLUSTER [VERBOSE]
 
    <para>
-    role.  <command>CLUSTER</command> will skip over any
 
    <para>
index 8524182211..070855da18 100644
 
    <para>
 
   <para>
index 23f8c7630b..bef539cf49 100644
@@ -292,21 +292,18 @@ REINDEX [ ( <replaceable class="parameter">option</replaceable> [, ...] ) ] { DA
   </para>
 
   <para>
-   Reindexing a single index or table requires being the owner of that
-   index or table, having privileges of the
-   <link linkend="predefined-roles-table"><literal>pg_maintain</literal></link>
-   role, or having the <literal>MAINTAIN</literal> privilege on the
+   Reindexing a single index or table requires
+   having the <literal>MAINTAIN</literal> privilege on the
    table.  Reindexing a schema or database requires being the
    owner of that schema or database or having privileges of the
-   <literal>pg_maintain</literal> role.  Note specifically that it's thus
+   <link linkend="predefined-roles-table"><literal>pg_maintain</literal></link>
+   role.  Note specifically that it's thus
    possible for non-superusers to rebuild indexes of tables owned by
-   other users.  However, as a special exception, when
-   <command>REINDEX DATABASE</command>, <command>REINDEX SCHEMA</command>
-   or <command>REINDEX SYSTEM</command> is issued by a non-superuser,
-   indexes on shared catalogs will be skipped unless the user owns the
-   catalog (which typically won't be the case), has privileges of the
-   <literal>pg_maintain</literal> role, or has the <literal>MAINTAIN</literal>
-   privilege on the catalog.  Of course, superusers can always reindex anything.
+   other users.  However, as a special exception,
+   <command>REINDEX DATABASE</command>, <command>REINDEX SCHEMA</command>,
+   and <command>REINDEX SYSTEM</command> will skip indexes on shared catalogs
+   unless the user has the <literal>MAINTAIN</literal> privilege on the
+   catalog.
   </para>
 
   <para>
diff --git a/doc/src/sgml/ref/vacuum.sgml b/doc/src/sgml/ref/vacuum.sgml
index 445325e14c..c42bbea9e2 100644
--- a/doc/src/sgml/ref/vacuum.sgml
+++ b/doc/src/sgml/ref/vacuum.sgml
@@ -445,14 +445,8 @@ VACUUM [ FULL ] [ FREEZE ] [ VERBOSE ] [ ANALYZE ] [ <replaceable class="paramet
 
    <para>

---

From: Michael Paquier <[email-masked]>
Date: Tue, 20 Jun 2023 14:26:42 +0900

> intend to commit this tomorrow unless there is additional feedback.

Fine by me.  0001 looks OK seen from here.

> is now a grantable privilege like any other.  WDYT?

TBH, I have a mixed feeling about this line of reasoning because
MAINTAIN is much broader and less specific than TRUNCATE, for
instance, being spawned across so much more operations.  As you say,
owners of a relation have the MAINTAIN right by default, but they
would not be able to run any maintenance operations if somebody has
revoked their MAINTAIN right to do so, even if they are the owners of
the so-said relation.  Perhaps that's OK in the long run, still I have
mixed feeling about whether that's the best decision we can take here, 
especially because MAINTAIN impacts VACUUM, ANALYZE, CLUSTER, REFRESH
MATVIEW, REINDEX and LOCK.  Some users may find that surprising as they
used to have more control over these operations as owners of the
relations worked on.
--
Michael

---

From: Jeff Davis <[email-masked]>
Date: Tue, 20 Jun 2023 10:04:37 -0700

> intend to commit this tomorrow unless there is additional feedback.

I think v4-0001 broke the handling of toast tables? It looks like you
removed the check for !skip_privs but need to add it to the flags in
vacuum_is_permitted_for_relation(). 

Regards,
	Jeff Davis

---

From: Nathan Bossart <[email-masked]>
Date: Tue, 20 Jun 2023 10:40:32 -0700

> vacuum_is_permitted_for_relation(). 

Good catch.  I'm not sure why some of the calls to
vacuum_is_permitted_for_relation() are masking the options.  AFAICT we can
simply remove the masks.  I've done so in the attached patch.

>From b7528138025f6fae062008fe4e166957789bdc25 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <[email-masked]>
Date: Wed, 14 Jun 2023 11:08:03 -0700
Subject: [PATCH v5 1/1] convert boolean parameter into a VACOPT_* option

---
 src/backend/commands/analyze.c |  2 +-
 src/backend/commands/vacuum.c  | 28 +++++++++++++++++++---------
 src/include/commands/vacuum.h  |  1 +
 3 files changed, 21 insertions(+), 10 deletions(-)

diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c
index 52ef462dba..08f7ba1623 100644
--- a/src/backend/commands/analyze.c
+++ b/src/backend/commands/analyze.c
@@ -167,7 +167,7 @@ analyze_rel(Oid relid, RangeVar *relation,
 	 */
 	if (!vacuum_is_permitted_for_relation(RelationGetRelid(onerel),
 										  onerel->rd_rel,
-										  params->options & VACOPT_ANALYZE))
+										  params->options))
 	{
 		relation_close(onerel, ShareUpdateExclusiveLock);
 		return;
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index a843f9ad92..108b5640d1 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -115,7 +115,7 @@ static void vac_truncate_clog(TransactionId frozenXID,
 							  TransactionId lastSaneFrozenXid,
 							  MultiXactId lastSaneMinMulti);
 static bool vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params,
-					   bool skip_privs, BufferAccessStrategy bstrategy);
+					   BufferAccessStrategy bstrategy);
 static double compute_parallel_delay(void);
 static VacOptValue get_vacoptval_from_boolean(DefElem *def);
 static bool vac_tid_reaped(ItemPointer itemptr, void *state);
@@ -620,8 +620,7 @@ vacuum(List *relations, VacuumParams *params, BufferAccessStrategy bstrategy,

 
 
+	/*
@@ -2040,10 +2046,9 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params,
-										  params->options & VACOPT_VACUUM))
+										  params->options))
 		relation_close(rel, lmode);
 		PopActiveSnapshot();

 
 	/*
 
 /*

---

From: Jeff Davis <[email-masked]>
Date: Tue, 20 Jun 2023 11:43:05 -0700

On Mon, 2023-06-19 at 14:55 -0700, Nathan Bossart wrote:
> I'm hoping to commit 0002 and 0003 by the end of the week so

A few observations for the case where a user does have the MAINTAIN
privilege on a partitioned table but not the partitions:

 * they can LOCK TABLE on the partitioned table
 * ANALYZE works on the inheritance tree but not the individual
partitions
 * CLUSTER and VACUUM are useless because they skip all of the
partitions. That's consistent with the purpose of this thread -- to
avoid the locking problems trying to support those operations on
partitioned tables.
 * REINDEX TABLE applies to all indexes in all partitions, which seems
a bit inconsistent.

The only behavior I'm worried about is REINDEX. I'm not sure what we
should do about it, or if we even want to do something about it. If we
want REINDEX to fail in this case, we should be sure to check
permissions on everything up-front to avoid doing a lot of work. The
only other option I can think of is to REINDEX only those indexes
declared on the partitioned table (not the individual partitions),
which seems consistent but might be confusing to users.

Regards,
	Jeff Davis