Move bool parameter for vacuum_rel() to option bits.

ff9618e82a introduced the skip_privs parameter, which is used to
skip privilege checks when recursing to a relation's TOAST table.
This parameter should have been added as a flag bit in
VacuumParams->options instead.

Suggested-by: Michael Paquier
Reviewed-by: Michael Paquier, Jeff Davis
Discussion: https://postgr.es/m/ZIj4v1CwqlDVJZfB%40paquier.xyz
This commit is contained in:
Nathan Bossart 2023-06-20 15:14:58 -07:00
parent 45392626c9
commit 5b1a879943
3 changed files with 21 additions and 10 deletions

View File

@ -167,7 +167,7 @@ analyze_rel(Oid relid, RangeVar *relation,
*/ */
if (!vacuum_is_permitted_for_relation(RelationGetRelid(onerel), if (!vacuum_is_permitted_for_relation(RelationGetRelid(onerel),
onerel->rd_rel, onerel->rd_rel,
params->options & VACOPT_ANALYZE)) params->options & ~VACOPT_VACUUM))
{ {
relation_close(onerel, ShareUpdateExclusiveLock); relation_close(onerel, ShareUpdateExclusiveLock);
return; return;

View File

@ -115,7 +115,7 @@ static void vac_truncate_clog(TransactionId frozenXID,
TransactionId lastSaneFrozenXid, TransactionId lastSaneFrozenXid,
MultiXactId lastSaneMinMulti); MultiXactId lastSaneMinMulti);
static bool vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params, static bool vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params,
bool skip_privs, BufferAccessStrategy bstrategy); BufferAccessStrategy bstrategy);
static double compute_parallel_delay(void); static double compute_parallel_delay(void);
static VacOptValue get_vacoptval_from_boolean(DefElem *def); static VacOptValue get_vacoptval_from_boolean(DefElem *def);
static bool vac_tid_reaped(ItemPointer itemptr, void *state); 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 (params->options & VACOPT_VACUUM)
{ {
if (!vacuum_rel(vrel->oid, vrel->relation, params, false, if (!vacuum_rel(vrel->oid, vrel->relation, params, bstrategy))
bstrategy))
continue; continue;
} }
@ -712,6 +711,13 @@ vacuum_is_permitted_for_relation(Oid relid, Form_pg_class reltuple,
Assert((options & (VACOPT_VACUUM | VACOPT_ANALYZE)) != 0); 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 * A role has privileges to vacuum or analyze the relation if any of the
* following are true: * following are true:
@ -1953,7 +1959,7 @@ vac_truncate_clog(TransactionId frozenXID,
*/ */
static bool static bool
vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params, vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params,
bool skip_privs, BufferAccessStrategy bstrategy) BufferAccessStrategy bstrategy)
{ {
LOCKMODE lmode; LOCKMODE lmode;
Relation rel; Relation rel;
@ -2040,10 +2046,9 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params,
* happen across multiple transactions where privileges could have changed * happen across multiple transactions where privileges could have changed
* in-between. Make sure to only generate logs for VACUUM in this case. * in-between. Make sure to only generate logs for VACUUM in this case.
*/ */
if (!skip_privs && if (!vacuum_is_permitted_for_relation(RelationGetRelid(rel),
!vacuum_is_permitted_for_relation(RelationGetRelid(rel),
rel->rd_rel, rel->rd_rel,
params->options & VACOPT_VACUUM)) params->options & ~VACOPT_ANALYZE))
{ {
relation_close(rel, lmode); relation_close(rel, lmode);
PopActiveSnapshot(); PopActiveSnapshot();
@ -2229,11 +2234,16 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params,
{ {
VacuumParams toast_vacuum_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)); memcpy(&toast_vacuum_params, params, sizeof(VacuumParams));
toast_vacuum_params.options |= VACOPT_PROCESS_MAIN; 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);
} }
/* /*

View File

@ -191,6 +191,7 @@ typedef struct VacAttrStats
#define VACOPT_DISABLE_PAGE_SKIPPING 0x100 /* don't skip any pages */ #define VACOPT_DISABLE_PAGE_SKIPPING 0x100 /* don't skip any pages */
#define VACOPT_SKIP_DATABASE_STATS 0x200 /* skip vac_update_datfrozenxid() */ #define VACOPT_SKIP_DATABASE_STATS 0x200 /* skip vac_update_datfrozenxid() */
#define VACOPT_ONLY_DATABASE_STATS 0x400 /* only 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. * Values used by index_cleanup and truncate params.