On Thu, Jun 15, 2023 at 10:20:25PM -0700, Nathan Bossart wrote:
> I noticed that it was possible to make the documentation changes in 0001
> 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.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
>From fcf57f8641ef3b44f12a7af699b3e4e9613d5b76 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nat...@postgresql.org>
Date: Wed, 14 Jun 2023 11:08:03 -0700
Subject: [PATCH v4 1/3] 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 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,
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;
}
@@ -712,6 +711,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:
@@ -1953,7 +1959,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;
@@ -2040,8 +2046,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))
{
@@ -2229,11 +2234,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 9f34ab133d021a18711f0a08cd7a1f636f8f3927 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nat...@postgresql.org>
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(-)
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..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.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..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.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)
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 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.c
index 987b11d16c..11c23de8c1 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
*----------
*/
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/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 <nat...@postgresql.org>
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>
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.sgml
index 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.sgml
index 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.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 23f8c7630b..bef539cf49 100644
--- a/doc/src/sgml/ref/reindex.sgml
+++ b/doc/src/sgml/ref/reindex.sgml
@@ -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>
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.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>
--
2.25.1
