On Tue, Jun 13, 2023 at 04:54:42PM -0700, Nathan Bossart wrote:
> On Wed, Jun 14, 2023 at 08:16:15AM +0900, Michael Paquier wrote:
>> So, yes, agreed about the removal of has_partition_ancestor_privs().
>> 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.
Concretely, I am proposing something like the attached patches.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
>From 8e93be7a0b4218a31f83b9fca53aa224bc9f5788 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nat...@postgresql.org>
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)
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..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 <nat...@postgresql.org>
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
