From: Nathan Bossart <nathandbossart@gmail.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: Jeff Davis <pgsql@j-davis.com>
Cc: Ted Yu <yuzhihong@gmail.com>
Cc: Pavel Luzanov <p.luzanov@postgrespro.ru>
Cc: Justin Pryzby <pryzby@telsasoft.com>
Cc: pgsql-hackers@postgresql.org
Subject: Re: allow granting CLUSTER, REFRESH MATERIALIZED VIEW, and REINDEX
Date: Wed, 21 Jun 2023 20:06:06 -0700
Message-ID: <20230622030606.GA1074545@nathanxps13> (raw)
In-Reply-To: <ZJOoAWxmbnrgx+er@paquier.xyz>
References: <20230615041044.GA736001@nathanxps13>
<20230615235700.GA877311@nathanxps13>
<20230616052025.GA1026700@nathanxps13>
<20230619215534.GA442477@nathanxps13>
<edf8dc01333f4b3355aa91001cb26029cab1eb2e.camel@j-davis.com>
<20230620225257.GA771663@nathanxps13>
<ZJJQgBkTEvL8RpwP@paquier.xyz>
<20230621041518.GA774124@nathanxps13>
<20230621171624.GA881253@nathanxps13>
<ZJOoAWxmbnrgx+er@paquier.xyz>
On Thu, Jun 22, 2023 at 10:46:41AM +0900, Michael Paquier wrote:
> On Wed, Jun 21, 2023 at 10:16:24AM -0700, Nathan Bossart wrote:
>>> I think that there is a testing gap with the coverage of CLUSTER.
>>> "Ownership of partitions is checked" is a test that looks for the case
>>> where regress_ptnowner owns the partitioned table and one of its
>>> partitions, checking that the leaf not owned is skipped, but we don't
>>> have a test where we attempt a CLUSTER on the partitioned table with
>>> regress_ptnowner *not* owning the partitioned table, only one or more
>>> of its partitions owned by regress_ptnowner. In this case, the
>>> command would fail.
>>
>> We could add something for this, but it'd really just exercise the checks
>> in RangeVarCallbackMaintainsTable(), which already has a decent amount of
>> coverage.
>
> It seems to me that this has some value for the CLUSTER path, so I
> would add a small thing for it.
Done.
> - /*
> - * 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;
> I would add a comment here that this ACL recheck for the leaves is an
> important thing to keep around as it impacts the case where the leaves
> have a different owner than the parent, and the owner of the parent
> clusters it. The only place in the tests where this has an influence
> is the isolation test cluster-conflict-partition.
Done.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.comAttachments:
[text/x-diff] v9-0001-partial-revert-of-ff9618e82a.patch (23.0K, ../20230622030606.GA1074545@nathanxps13/2-v9-0001-partial-revert-of-ff9618e82a.patch)
download | inline diff:
From 941371500f66690bbf200dfb555cc17d947cf0d1 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@postgresql.org>
Date: Mon, 19 Jun 2023 13:57:57 -0700
Subject: [PATCH v9 1/2] 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 | 12 ++++---
src/backend/commands/indexcmds.c | 27 +++++++--------
src/backend/commands/lockcmds.c | 8 -----
src/backend/commands/tablecmds.c | 34 +++----------------
src/backend/commands/vacuum.c | 10 ++----
src/include/commands/tablecmds.h | 1 -
.../expected/cluster-conflict-partition.out | 14 ++++----
.../specs/cluster-conflict-partition.spec | 7 ++--
src/test/regress/expected/cluster.out | 7 +++-
src/test/regress/expected/create_index.out | 4 +--
src/test/regress/expected/privileges.out | 8 ++---
src/test/regress/expected/vacuum.out | 18 ++++++++++
src/test/regress/sql/cluster.sql | 3 ++
18 files changed, 74 insertions(+), 105 deletions(-)
diff --git a/doc/src/sgml/ref/analyze.sgml b/doc/src/sgml/ref/analyze.sgmlindex 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.sgmlindex 29f0f1fd90..f0dd7faed5 100644--- a/doc/src/sgml/ref/cluster.sgml+++ b/doc/src/sgml/ref/cluster.sgml@@ -137,10 +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+ role. <command>CLUSTER</command> will skip over any
tables that the calling user does not have permission to cluster.
</para>
diff --git a/doc/src/sgml/ref/lock.sgml b/doc/src/sgml/ref/lock.sgmlindex 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.sgmlindex 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.sgmlindex 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.cindex 369fea7c04..3bfabb6d10 100644--- a/src/backend/commands/cluster.c+++ b/src/backend/commands/cluster.c@@ -1694,10 +1694,13 @@ get_tables_to_cluster_partitioned(MemoryContext cluster_context, Oid indexOid)
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.+ * It's possible that the user does not have privileges to CLUSTER the+ * leaf partition despite having such privileges on the partitioned+ * table. We skip any partitions which the user is not permitted to+ * CLUSTER.
*/
+ 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 +1723,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.cindex 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;
/*
- * 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;
/*
diff --git a/src/backend/commands/lockcmds.c b/src/backend/commands/lockcmds.cindex 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)
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.cindex 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);
}
/*
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.cindex bb79de4da6..7fe6a54c06 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"
@@ -721,17 +720,12 @@ vacuum_is_permitted_for_relation(Oid relid, Form_pg_class reltuple,
/*----------
* A role has privileges to vacuum or analyze the relation if any of the
* following are true:
- * - the role is a superuser- * - 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+ * - the role has the MAINTAIN privilege on the relation
*----------
*/
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.hindex 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.outindex 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.specindex 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.outindex 2eec483eaa..a13aafff0b 100644--- a/src/test/regress/expected/cluster.out+++ b/src/test/regress/expected/cluster.out@@ -502,12 +502,17 @@ CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1);
CREATE ROLE regress_ptnowner;
CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2);
ALTER TABLE ptnowner1 OWNER TO regress_ptnowner;
+SET SESSION AUTHORIZATION regress_ptnowner;+CLUSTER ptnowner USING ptnowner_i_idx;+ERROR: permission denied for table ptnowner+RESET SESSION AUTHORIZATION;
ALTER TABLE ptnowner OWNER TO regress_ptnowner;
CREATE TEMP TABLE ptnowner_oldnodes AS
SELECT oid, relname, relfilenode FROM pg_partition_tree('ptnowner') AS tree
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 +520,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/create_index.out b/src/test/regress/expected/create_index.outindex 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.outindex 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.outindex 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;
diff --git a/src/test/regress/sql/cluster.sql b/src/test/regress/sql/cluster.sqlindex a4cfaae807..b7115f8610 100644--- a/src/test/regress/sql/cluster.sql+++ b/src/test/regress/sql/cluster.sql@@ -238,6 +238,9 @@ CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1);
CREATE ROLE regress_ptnowner;
CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2);
ALTER TABLE ptnowner1 OWNER TO regress_ptnowner;
+SET SESSION AUTHORIZATION regress_ptnowner;+CLUSTER ptnowner USING ptnowner_i_idx;+RESET SESSION AUTHORIZATION;
ALTER TABLE ptnowner OWNER TO regress_ptnowner;
CREATE TEMP TABLE ptnowner_oldnodes AS
SELECT oid, relname, relfilenode FROM pg_partition_tree('ptnowner') AS tree
--
2.25.1
[text/x-diff] v9-0002-simplify-privilege-related-documentation-for-main.patch (8.3K, ../20230622030606.GA1074545@nathanxps13/3-v9-0002-simplify-privilege-related-documentation-for-main.patch)
download | inline diff:
From 99b0179198080d556d402c614792ace465f94a61 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@postgresql.org>
Date: Mon, 19 Jun 2023 14:07:26 -0700
Subject: [PATCH v9 2/2] 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 | 26 +++++++++----------
doc/src/sgml/ref/vacuum.sgml | 8 +-----
doc/src/sgml/user-manag.sgml | 3 ++-
7 files changed, 22 insertions(+), 41 deletions(-)
diff --git a/doc/src/sgml/ref/analyze.sgml b/doc/src/sgml/ref/analyze.sgmlindex 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>
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+ privilege on the table. 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>.)
<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.sgmlindex f0dd7faed5..06f3d269e6 100644--- a/doc/src/sgml/ref/cluster.sgml+++ b/doc/src/sgml/ref/cluster.sgml@@ -134,11 +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. <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.sgmlindex 8524182211..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
diff --git a/doc/src/sgml/ref/refresh_materialized_view.sgml b/doc/src/sgml/ref/refresh_materialized_view.sgmlindex 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.sgmlindex 23f8c7630b..bef3486843 100644--- a/doc/src/sgml/ref/reindex.sgml+++ b/doc/src/sgml/ref/reindex.sgml@@ -292,21 +292,21 @@ 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- table. Reindexing a schema or database requires being the+ Reindexing a single index or table requires+ having the <literal>MAINTAIN</literal> privilege on the+ table. Note that while <command>REINDEX</command> on a partitioned index or+ table requires having the <literal>MAINTAIN</literal> privilege on the+ partitioned table, such commands skip the privilege checks when processing+ the individual partitions. 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.sgmlindex 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>
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+ privilege on the table. 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>.)
<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.sgmlindex 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>
--
2.25.1
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: nathandbossart@gmail.com, michael@paquier.xyz, pgsql@j-davis.com, yuzhihong@gmail.com, p.luzanov@postgrespro.ru, pryzby@telsasoft.com
Subject: Re: allow granting CLUSTER, REFRESH MATERIALIZED VIEW, and REINDEX
In-Reply-To: <20230622030606.GA1074545@nathanxps13>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox