public inbox for [email protected]
help / color / mirror / Atom feedFrom: Nathan Bossart <[email protected]>
To: Jeff Davis <[email protected]>
Cc: Michael Paquier <[email protected]>
Cc: Ted Yu <[email protected]>
Cc: Pavel Luzanov <[email protected]>
Cc: Justin Pryzby <[email protected]>
Cc: [email protected]
Subject: Re: allow granting CLUSTER, REFRESH MATERIALIZED VIEW, and REINDEX
Date: Tue, 20 Jun 2023 10:40:32 -0700
Message-ID: <20230620174032.GA471329@nathanxps13> (raw)
In-Reply-To: <[email protected]>
References: <20230613211246.GA219055@nathanxps13>
<[email protected]>
<20230613235442.GA222795@nathanxps13>
<20230614181711.GA488295@nathanxps13>
<[email protected]>
<20230615041044.GA736001@nathanxps13>
<20230615235700.GA877311@nathanxps13>
<20230616052025.GA1026700@nathanxps13>
<20230619215534.GA442477@nathanxps13>
<[email protected]>
On Tue, Jun 20, 2023 at 10:04:37AM -0700, Jeff Davis wrote:
> 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().
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.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
Attachments:
[text/x-diff] v5-0001-convert-boolean-parameter-into-a-VACOPT_-option.patch (4.4K, ../20230620174032.GA471329@nathanxps13/2-v5-0001-convert-boolean-parameter-into-a-VACOPT_-option.patch)
download | inline diff:
From b7528138025f6fae062008fe4e166957789bdc25 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <[email protected]>
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,
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,10 +2046,9 @@ 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))
+ params->options))
{
relation_close(rel, lmode);
PopActiveSnapshot();
@@ -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
view thread (18+ messages) latest in thread
reply
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: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: allow granting CLUSTER, REFRESH MATERIALIZED VIEW, and REINDEX
In-Reply-To: <20230620174032.GA471329@nathanxps13>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox