Move bool parameter for vacuum_rel() to option bits.

nathan-bossart · nathan-bossart · commit 5b1a87994312 · 2023-06-20T15:14:58.000-07:00
ff9618e 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
diff --git 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 & ~VACOPT_VACUUM))
 	{
 		relation_close(onerel, ShareUpdateExclusiveLock);
 		return;
diff --git 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 & ~VACOPT_ANALYZE))
 	{
 		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
@@ -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.

Original file line number	Diff line number	Diff line change
`@@ -167,7 +167,7 @@ analyze_rel(Oid relid, RangeVar *relation,`
`167`	`167`	`*/`
`168`	`168`	`if (!vacuum_is_permitted_for_relation(RelationGetRelid(onerel),`
`169`	`169`	`onerel->rd_rel,`
`170`		`- params->options & VACOPT_ANALYZE))`
	`170`	`+ params->options & ~VACOPT_VACUUM))`
`171`	`171`	`{`
`172`	`172`	`relation_close(onerel, ShareUpdateExclusiveLock);`
`173`	`173`	`return;`