On Mon, Dec 14, 2020 at 06:14:17PM -0600, Justin Pryzby wrote:
> On Sat, Dec 12, 2020 at 01:45:26PM -0600, Justin Pryzby wrote:
> > On Sat, Dec 12, 2020 at 09:20:35AM +0100, Peter Eisentraut wrote:
> > > On 2020-12-11 21:27, Alvaro Herrera wrote:
> > > > By the way-- What did you think of the idea of explictly marking the
> > > > types used for bitmasks using types bits32 and friends, instead of plain
> > > > int, which is harder to spot?
> > >
> > > If we want to make it clearer, why not turn the thing into a struct, as in
> > > the attached patch, and avoid the bit fiddling altogether.
> >
> > I like this.
> > It's a lot like what I wrote as [PATCH v31 1/5] ExecReindex and
> > ReindexParams
> > In my v31 patch, I moved ReindexOptions to a private structure in
> > indexcmds.c,
> > with an "int options" bitmask which is passed to reindex_index() et al.
> > Your
> > patch keeps/puts ReindexOptions index.h, so it also applies to
> > reindex_index,
> > which I think is good.
> >
> > So I've rebased this branch on your patch.
> >
> > Some thoughts:
> >
> > - what about removing the REINDEXOPT_* prefix ?
> > - You created local vars with initialization like "={}". But I thought it's
> > needed to include at least one struct member like "={false}", or else
> > they're not guaranteed to be zerod ?
> > - You passed the structure across function calls. The usual convention is
> > to
> > pass a pointer.
>
> I think maybe Michael missed this message (?)
> I had applied some changes on top of Peter's patch.
>
> I squished those commits now, and also handled ClusterOption and VacuumOption
> in the same style.
>
> Some more thoughts:
> - should the structures be named in plural ? "ReindexOptions" etc. Since
> they
> define *all* the options, not just a single bit.
> - For vacuum, do we even need a separate structure, or should the members be
> directly within VacuumParams ? It's a bit odd to write
> params.options.verbose. Especially since there's also ternary options
> which
> are directly within params.
> - Then, for cluster, I think it should be called ClusterParams, and
> eventually
> include the tablespaceOid, like what we're doing for Reindex.
>
> I am awaiting feedback on these before going further since I've done too much
> rebasing with these ideas going back and forth and back.
With Alexey's agreement, I propose something like this.
I've merged some commits and kept separate the ones which are more likely to be
disputed/amended. But it may be best to read the series as a single commit,
like "git diff origin.."
--
Justin
>From ee702a6f49392e84bb51ec4324444d0974c7369b Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
Date: Tue, 15 Dec 2020 17:55:21 -0600
Subject: [PATCH 1/4] Convert options to struct: Reindex/Cluster/Vacuum
Remove prefixes and lowercase structure member variables;
Rename to plural: Vacuum/ClusterOptions;
---
src/backend/access/heap/vacuumlazy.c | 8 +-
src/backend/catalog/index.c | 26 +++---
src/backend/commands/analyze.c | 15 ++--
src/backend/commands/cluster.c | 29 +++---
src/backend/commands/indexcmds.c | 100 ++++++++++-----------
src/backend/commands/tablecmds.c | 4 +-
src/backend/commands/vacuum.c | 128 +++++++++++++--------------
src/backend/postmaster/autovacuum.c | 14 +--
src/backend/tcop/utility.c | 10 +--
src/include/catalog/index.h | 16 ++--
src/include/commands/cluster.h | 10 +--
src/include/commands/defrem.h | 9 +-
src/include/commands/vacuum.h | 29 +++---
13 files changed, 198 insertions(+), 200 deletions(-)
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index 25f2d5df1b..6bfed2b2da 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -456,7 +456,7 @@ heap_vacuum_rel(Relation onerel, VacuumParams *params,
starttime = GetCurrentTimestamp();
}
- if (params->options & VACOPT_VERBOSE)
+ if (params->options.verbose)
elevel = INFO;
else
elevel = DEBUG2;
@@ -484,7 +484,7 @@ heap_vacuum_rel(Relation onerel, VacuumParams *params,
xidFullScanLimit);
aggressive |= MultiXactIdPrecedesOrEquals(onerel->rd_rel->relminmxid,
mxactFullScanLimit);
- if (params->options & VACOPT_DISABLE_PAGE_SKIPPING)
+ if (params->options.disable_page_skipping)
aggressive = true;
vacrelstats = (LVRelStats *) palloc0(sizeof(LVRelStats));
@@ -902,7 +902,7 @@ lazy_scan_heap(Relation onerel, VacuumParams *params, LVRelStats *vacrelstats,
* be replayed on any hot standby, where it can be disruptive.
*/
next_unskippable_block = 0;
- if ((params->options & VACOPT_DISABLE_PAGE_SKIPPING) == 0)
+ if (!params->options.disable_page_skipping)
{
while (next_unskippable_block < nblocks)
{
@@ -960,7 +960,7 @@ lazy_scan_heap(Relation onerel, VacuumParams *params, LVRelStats *vacrelstats,
{
/* Time to advance next_unskippable_block */
next_unskippable_block++;
- if ((params->options & VACOPT_DISABLE_PAGE_SKIPPING) == 0)
+ if (!params->options.disable_page_skipping)
{
while (next_unskippable_block < nblocks)
{
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index 731610c701..ecd66ff3df 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -3594,7 +3594,7 @@ IndexGetRelation(Oid indexId, bool missing_ok)
*/
void
reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
- int options)
+ ReindexOptions *options)
{
Relation iRel,
heapRelation;
@@ -3602,7 +3602,6 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
IndexInfo *indexInfo;
volatile bool skipped_constraint = false;
PGRUsage ru0;
- bool progress = (options & REINDEXOPT_REPORT_PROGRESS) != 0;
pg_rusage_init(&ru0);
@@ -3611,12 +3610,12 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
* we only need to be sure no schema or data changes are going on.
*/
heapId = IndexGetRelation(indexId,
- (options & REINDEXOPT_MISSING_OK) != 0);
+ options->missing_ok);
/* if relation is missing, leave */
if (!OidIsValid(heapId))
return;
- if ((options & REINDEXOPT_MISSING_OK) != 0)
+ if (options->missing_ok)
heapRelation = try_table_open(heapId, ShareLock);
else
heapRelation = table_open(heapId, ShareLock);
@@ -3625,7 +3624,7 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
if (!heapRelation)
return;
- if (progress)
+ if (options->report_progress)
{
pgstat_progress_start_command(PROGRESS_COMMAND_CREATE_INDEX,
heapId);
@@ -3641,7 +3640,7 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
*/
iRel = index_open(indexId, AccessExclusiveLock);
- if (progress)
+ if (options->report_progress)
pgstat_progress_update_param(PROGRESS_CREATEIDX_ACCESS_METHOD_OID,
iRel->rd_rel->relam);
@@ -3792,14 +3791,14 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
}
/* Log what we did */
- if (options & REINDEXOPT_VERBOSE)
+ if (options->verbose)
ereport(INFO,
(errmsg("index \"%s\" was reindexed",
get_rel_name(indexId)),
errdetail_internal("%s",
pg_rusage_show(&ru0))));
- if (progress)
+ if (options->report_progress)
pgstat_progress_end_command();
/* Close rels, but keep locks */
@@ -3846,7 +3845,7 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
* index rebuild.
*/
bool
-reindex_relation(Oid relid, int flags, int options)
+reindex_relation(Oid relid, int flags, ReindexOptions *options)
{
Relation rel;
Oid toast_relid;
@@ -3861,7 +3860,7 @@ reindex_relation(Oid relid, int flags, int options)
* to prevent schema and data changes in it. The lock level used here
* should match ReindexTable().
*/
- if ((options & REINDEXOPT_MISSING_OK) != 0)
+ if (options->missing_ok)
rel = try_table_open(relid, ShareLock);
else
rel = table_open(relid, ShareLock);
@@ -3963,10 +3962,11 @@ reindex_relation(Oid relid, int flags, int options)
{
/*
* Note that this should fail if the toast relation is missing, so
- * reset REINDEXOPT_MISSING_OK.
+ * reset missing_ok.
*/
- result |= reindex_relation(toast_relid, flags,
- options & ~(REINDEXOPT_MISSING_OK));
+ ReindexOptions newoptions = *options;
+ newoptions.missing_ok = false;
+ result |= reindex_relation(toast_relid, flags, &newoptions);
}
return result;
diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c
index 8af12b5c6b..884bfeee21 100644
--- a/src/backend/commands/analyze.c
+++ b/src/backend/commands/analyze.c
@@ -124,9 +124,10 @@ analyze_rel(Oid relid, RangeVar *relation,
int elevel;
AcquireSampleRowsFunc acquirefunc = NULL;
BlockNumber relpages = 0;
+ VacuumOptions newoptions;
/* Select logging level */
- if (params->options & VACOPT_VERBOSE)
+ if (params->options.verbose)
elevel = INFO;
else
elevel = DEBUG2;
@@ -148,7 +149,9 @@ analyze_rel(Oid relid, RangeVar *relation,
*
* Make sure to generate only logs for ANALYZE in this case.
*/
- onerel = vacuum_open_relation(relid, relation, params->options & ~(VACOPT_VACUUM),
+ newoptions = params->options;
+ newoptions.vacuum = false;
+ onerel = vacuum_open_relation(relid, relation, &newoptions,
params->log_min_duration >= 0,
ShareUpdateExclusiveLock);
@@ -166,7 +169,7 @@ analyze_rel(Oid relid, RangeVar *relation,
*/
if (!vacuum_is_relation_owner(RelationGetRelid(onerel),
onerel->rd_rel,
- params->options & VACOPT_ANALYZE))
+ &newoptions))
{
relation_close(onerel, ShareUpdateExclusiveLock);
return;
@@ -238,7 +241,7 @@ analyze_rel(Oid relid, RangeVar *relation,
else
{
/* No need for a WARNING if we already complained during VACUUM */
- if (!(params->options & VACOPT_VACUUM))
+ if (!params->options.vacuum)
ereport(WARNING,
(errmsg("skipping \"%s\" --- cannot analyze non-tables or special system tables",
RelationGetRelationName(onerel))));
@@ -624,7 +627,7 @@ do_analyze_rel(Relation onerel, VacuumParams *params,
* VACUUM ANALYZE, don't overwrite the accurate count already inserted by
* VACUUM.
*/
- if (!inh && !(params->options & VACOPT_VACUUM))
+ if (!inh && !params->options.vacuum)
{
for (ind = 0; ind < nindexes; ind++)
{
@@ -655,7 +658,7 @@ do_analyze_rel(Relation onerel, VacuumParams *params,
(va_cols == NIL));
/* If this isn't part of VACUUM ANALYZE, let index AMs do cleanup */
- if (!(params->options & VACOPT_VACUUM))
+ if (!params->options.vacuum)
{
for (ind = 0; ind < nindexes; ind++)
{
diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c
index fd5a6eec86..e57bef42aa 100644
--- a/src/backend/commands/cluster.c
+++ b/src/backend/commands/cluster.c
@@ -103,8 +103,7 @@ void
cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel)
{
ListCell *lc;
- int options = 0;
- bool verbose = false;
+ ClusterOptions options = {false};
/* Parse option list */
foreach(lc, stmt->params)
@@ -112,7 +111,7 @@ cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel)
DefElem *opt = (DefElem *) lfirst(lc);
if (strcmp(opt->defname, "verbose") == 0)
- verbose = defGetBoolean(opt);
+ options.verbose = defGetBoolean(opt);
else
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
@@ -121,8 +120,6 @@ cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel)
parser_errposition(pstate, opt->location)));
}
- options = (verbose ? CLUOPT_VERBOSE : 0);
-
if (stmt->relation != NULL)
{
/* This is the single-relation case. */
@@ -192,7 +189,7 @@ cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel)
table_close(rel, NoLock);
/* Do the job. */
- cluster_rel(tableOid, indexOid, options);
+ cluster_rel(tableOid, indexOid, &options);
}
else
{
@@ -234,14 +231,15 @@ cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel)
foreach(rv, rvs)
{
RelToCluster *rvtc = (RelToCluster *) lfirst(rv);
+ ClusterOptions newoptions = options;
+ newoptions.recheck = true;
/* Start a new transaction for each relation. */
StartTransactionCommand();
/* functions in indexes may want a snapshot set */
PushActiveSnapshot(GetTransactionSnapshot());
/* Do the job. */
- cluster_rel(rvtc->tableOid, rvtc->indexOid,
- options | CLUOPT_RECHECK);
+ cluster_rel(rvtc->tableOid, rvtc->indexOid, &newoptions);
PopActiveSnapshot();
CommitTransactionCommand();
}
@@ -272,11 +270,9 @@ cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel)
* and error messages should refer to the operation as VACUUM not CLUSTER.
*/
void
-cluster_rel(Oid tableOid, Oid indexOid, int options)
+cluster_rel(Oid tableOid, Oid indexOid, ClusterOptions *options)
{
Relation OldHeap;
- bool verbose = ((options & CLUOPT_VERBOSE) != 0);
- bool recheck = ((options & CLUOPT_RECHECK) != 0);
/* Check for user-requested abort. */
CHECK_FOR_INTERRUPTS();
@@ -312,7 +308,7 @@ cluster_rel(Oid tableOid, Oid indexOid, int options)
* *must* skip the one on indisclustered since it would reject an attempt
* to cluster a not-previously-clustered index.
*/
- if (recheck)
+ if (options->recheck)
{
/* Check that the user still owns the relation */
if (!pg_class_ownercheck(tableOid, GetUserId()))
@@ -396,7 +392,9 @@ cluster_rel(Oid tableOid, Oid indexOid, int options)
/* Check heap and index are valid to cluster on */
if (OidIsValid(indexOid))
- check_index_is_clusterable(OldHeap, indexOid, recheck, AccessExclusiveLock);
+ check_index_is_clusterable(OldHeap, indexOid, options->recheck,
+ AccessExclusiveLock);
+
/*
* Quietly ignore the request if this is a materialized view which has not
@@ -422,7 +420,7 @@ cluster_rel(Oid tableOid, Oid indexOid, int options)
TransferPredicateLocksToHeapRelation(OldHeap);
/* rebuild_relation does all the dirty work */
- rebuild_relation(OldHeap, indexOid, verbose);
+ rebuild_relation(OldHeap, indexOid, options->verbose);
/* NB: rebuild_relation does table_close() on OldHeap */
@@ -1353,6 +1351,7 @@ finish_heap_swap(Oid OIDOldHeap, Oid OIDNewHeap,
char newrelpersistence)
{
ObjectAddress object;
+ ReindexOptions reindexopts = {false}; /* Default options are all false */
Oid mapped_tables[4];
int reindex_flags;
int i;
@@ -1412,7 +1411,7 @@ finish_heap_swap(Oid OIDOldHeap, Oid OIDNewHeap,
pgstat_progress_update_param(PROGRESS_CLUSTER_PHASE,
PROGRESS_CLUSTER_PHASE_REBUILD_INDEX);
- reindex_relation(OIDOldHeap, reindex_flags, 0);
+ reindex_relation(OIDOldHeap, reindex_flags, &reindexopts);
/* Report that we are now doing clean up */
pgstat_progress_update_param(PROGRESS_CLUSTER_PHASE,
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 14d24b3cc4..13e463da90 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -89,9 +89,9 @@ static List *ChooseIndexColumnNames(List *indexElems);
static void RangeVarCallbackForReindexIndex(const RangeVar *relation,
Oid relId, Oid oldRelId, void *arg);
static void reindex_error_callback(void *args);
-static void ReindexPartitions(Oid relid, int options, bool isTopLevel);
-static void ReindexMultipleInternal(List *relids, int options);
-static bool ReindexRelationConcurrently(Oid relationOid, int options);
+static void ReindexPartitions(Oid relid, ReindexOptions *options, bool isTopLevel);
+static void ReindexMultipleInternal(List *relids, ReindexOptions *options);
+static bool ReindexRelationConcurrently(Oid relationOid, ReindexOptions *options);
static void update_relispartition(Oid relationId, bool newval);
static inline void set_indexsafe_procflags(void);
@@ -100,7 +100,7 @@ static inline void set_indexsafe_procflags(void);
*/
struct ReindexIndexCallbackState
{
- int options; /* options from statement */
+ ReindexOptions options; /* options from statement */
Oid locked_table_oid; /* tracks previously locked table */
};
@@ -2455,13 +2455,11 @@ ChooseIndexColumnNames(List *indexElems)
* ReindexParseOptions
* Parse list of REINDEX options, returning a bitmask of ReindexOption.
*/
-int
+ReindexOptions
ReindexParseOptions(ParseState *pstate, ReindexStmt *stmt)
{
ListCell *lc;
- int options = 0;
- bool concurrently = false;
- bool verbose = false;
+ ReindexOptions options = {false};
/* Parse option list */
foreach(lc, stmt->params)
@@ -2469,9 +2467,9 @@ ReindexParseOptions(ParseState *pstate, ReindexStmt *stmt)
DefElem *opt = (DefElem *) lfirst(lc);
if (strcmp(opt->defname, "verbose") == 0)
- verbose = defGetBoolean(opt);
+ options.verbose = defGetBoolean(opt);
else if (strcmp(opt->defname, "concurrently") == 0)
- concurrently = defGetBoolean(opt);
+ options.concurrently = defGetBoolean(opt);
else
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
@@ -2480,10 +2478,6 @@ ReindexParseOptions(ParseState *pstate, ReindexStmt *stmt)
parser_errposition(pstate, opt->location)));
}
- options =
- (verbose ? REINDEXOPT_VERBOSE : 0) |
- (concurrently ? REINDEXOPT_CONCURRENTLY : 0);
-
return options;
}
@@ -2492,7 +2486,7 @@ ReindexParseOptions(ParseState *pstate, ReindexStmt *stmt)
* Recreate a specific index.
*/
void
-ReindexIndex(RangeVar *indexRelation, int options, bool isTopLevel)
+ReindexIndex(RangeVar *indexRelation, ReindexOptions *options, bool isTopLevel)
{
struct ReindexIndexCallbackState state;
Oid indOid;
@@ -2509,10 +2503,10 @@ ReindexIndex(RangeVar *indexRelation, int options, bool isTopLevel)
* upgrade the lock, but that's OK, because other sessions can't hold
* locks on our temporary table.
*/
- state.options = options;
+ state.options = *options;
state.locked_table_oid = InvalidOid;
indOid = RangeVarGetRelidExtended(indexRelation,
- (options & REINDEXOPT_CONCURRENTLY) != 0 ?
+ options->concurrently ?
ShareUpdateExclusiveLock : AccessExclusiveLock,
0,
RangeVarCallbackForReindexIndex,
@@ -2527,12 +2521,15 @@ ReindexIndex(RangeVar *indexRelation, int options, bool isTopLevel)
if (relkind == RELKIND_PARTITIONED_INDEX)
ReindexPartitions(indOid, options, isTopLevel);
- else if ((options & REINDEXOPT_CONCURRENTLY) != 0 &&
+ else if (options->concurrently &&
persistence != RELPERSISTENCE_TEMP)
ReindexRelationConcurrently(indOid, options);
else
- reindex_index(indOid, false, persistence,
- options | REINDEXOPT_REPORT_PROGRESS);
+ {
+ ReindexOptions newoptions = *options;
+ newoptions.report_progress = true;
+ reindex_index(indOid, false, persistence, &newoptions);
+ }
}
/*
@@ -2553,7 +2550,7 @@ RangeVarCallbackForReindexIndex(const RangeVar *relation,
* non-concurrent case and table locks used by index_concurrently_*() for
* concurrent case.
*/
- table_lockmode = ((state->options & REINDEXOPT_CONCURRENTLY) != 0) ?
+ table_lockmode = state->options.concurrently ?
ShareUpdateExclusiveLock : ShareLock;
/*
@@ -2611,7 +2608,7 @@ RangeVarCallbackForReindexIndex(const RangeVar *relation,
* Recreate all indexes of a table (and of its toast table, if any)
*/
Oid
-ReindexTable(RangeVar *relation, int options, bool isTopLevel)
+ReindexTable(RangeVar *relation, ReindexOptions *options, bool isTopLevel)
{
Oid heapOid;
bool result;
@@ -2625,14 +2622,14 @@ ReindexTable(RangeVar *relation, int options, bool isTopLevel)
* locks on our temporary table.
*/
heapOid = RangeVarGetRelidExtended(relation,
- (options & REINDEXOPT_CONCURRENTLY) != 0 ?
+ options->concurrently ?
ShareUpdateExclusiveLock : ShareLock,
0,
RangeVarCallbackOwnsTable, NULL);
if (get_rel_relkind(heapOid) == RELKIND_PARTITIONED_TABLE)
ReindexPartitions(heapOid, options, isTopLevel);
- else if ((options & REINDEXOPT_CONCURRENTLY) != 0 &&
+ else if (options->concurrently &&
get_rel_persistence(heapOid) != RELPERSISTENCE_TEMP)
{
result = ReindexRelationConcurrently(heapOid, options);
@@ -2644,10 +2641,12 @@ ReindexTable(RangeVar *relation, int options, bool isTopLevel)
}
else
{
+ ReindexOptions newoptions = *options;
+ newoptions.report_progress = true;
result = reindex_relation(heapOid,
REINDEX_REL_PROCESS_TOAST |
REINDEX_REL_CHECK_CONSTRAINTS,
- options | REINDEXOPT_REPORT_PROGRESS);
+ &newoptions);
if (!result)
ereport(NOTICE,
(errmsg("table \"%s\" has no indexes to reindex",
@@ -2667,7 +2666,7 @@ ReindexTable(RangeVar *relation, int options, bool isTopLevel)
*/
void
ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
- int options)
+ ReindexOptions *options)
{
Oid objectOid;
Relation relationRelation;
@@ -2686,7 +2685,7 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
objectKind == REINDEX_OBJECT_DATABASE);
if (objectKind == REINDEX_OBJECT_SYSTEM &&
- (options & REINDEXOPT_CONCURRENTLY) != 0)
+ options->concurrently)
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("cannot reindex system catalogs concurrently")));
@@ -2794,7 +2793,7 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
* Skip system tables, since index_create() would reject indexing them
* concurrently (and it would likely fail if we tried).
*/
- if ((options & REINDEXOPT_CONCURRENTLY) != 0 &&
+ if (options->concurrently &&
IsCatalogRelationOid(relid))
{
if (!concurrent_warning)
@@ -2860,7 +2859,7 @@ reindex_error_callback(void *arg)
* by the caller.
*/
static void
-ReindexPartitions(Oid relid, int options, bool isTopLevel)
+ReindexPartitions(Oid relid, ReindexOptions *options, bool isTopLevel)
{
List *partitions = NIL;
char relkind = get_rel_relkind(relid);
@@ -2955,7 +2954,7 @@ ReindexPartitions(Oid relid, int options, bool isTopLevel)
* and starts a new transaction when finished.
*/
static void
-ReindexMultipleInternal(List *relids, int options)
+ReindexMultipleInternal(List *relids, ReindexOptions *options)
{
ListCell *l;
@@ -2991,35 +2990,36 @@ ReindexMultipleInternal(List *relids, int options)
Assert(relkind != RELKIND_PARTITIONED_INDEX &&
relkind != RELKIND_PARTITIONED_TABLE);
- if ((options & REINDEXOPT_CONCURRENTLY) != 0 &&
+ if (options->concurrently &&
relpersistence != RELPERSISTENCE_TEMP)
{
- (void) ReindexRelationConcurrently(relid,
- options |
- REINDEXOPT_MISSING_OK);
+ ReindexOptions newoptions = *options;
+ newoptions.missing_ok = true;
+ (void) ReindexRelationConcurrently(relid, &newoptions);
/* ReindexRelationConcurrently() does the verbose output */
}
else if (relkind == RELKIND_INDEX)
{
- reindex_index(relid, false, relpersistence,
- options |
- REINDEXOPT_REPORT_PROGRESS |
- REINDEXOPT_MISSING_OK);
+ ReindexOptions newoptions = *options;
+ newoptions.report_progress = true;
+ newoptions.missing_ok = true;
+ reindex_index(relid, false, relpersistence, &newoptions);
PopActiveSnapshot();
/* reindex_index() does the verbose output */
}
else
{
bool result;
+ ReindexOptions newoptions = *options;
+ newoptions.report_progress = true;
+ newoptions.missing_ok = true;
result = reindex_relation(relid,
REINDEX_REL_PROCESS_TOAST |
REINDEX_REL_CHECK_CONSTRAINTS,
- options |
- REINDEXOPT_REPORT_PROGRESS |
- REINDEXOPT_MISSING_OK);
+ &newoptions);
- if (result && (options & REINDEXOPT_VERBOSE))
+ if (result && options->verbose)
ereport(INFO,
(errmsg("table \"%s.%s\" was reindexed",
get_namespace_name(get_rel_namespace(relid)),
@@ -3059,7 +3059,7 @@ ReindexMultipleInternal(List *relids, int options)
* anyway, and a non-concurrent reindex is more efficient.
*/
static bool
-ReindexRelationConcurrently(Oid relationOid, int options)
+ReindexRelationConcurrently(Oid relationOid, ReindexOptions *options)
{
List *heapRelationIds = NIL;
List *indexIds = NIL;
@@ -3092,7 +3092,7 @@ ReindexRelationConcurrently(Oid relationOid, int options)
"ReindexConcurrent",
ALLOCSET_SMALL_SIZES);
- if (options & REINDEXOPT_VERBOSE)
+ if (options->verbose)
{
/* Save data needed by REINDEX VERBOSE in private context */
oldcontext = MemoryContextSwitchTo(private_context);
@@ -3137,7 +3137,7 @@ ReindexRelationConcurrently(Oid relationOid, int options)
errmsg("cannot reindex system catalogs concurrently")));
/* Open relation to get its indexes */
- if ((options & REINDEXOPT_MISSING_OK) != 0)
+ if (options->missing_ok)
{
heapRelation = try_table_open(relationOid,
ShareUpdateExclusiveLock);
@@ -3233,7 +3233,7 @@ ReindexRelationConcurrently(Oid relationOid, int options)
case RELKIND_INDEX:
{
Oid heapId = IndexGetRelation(relationOid,
- (options & REINDEXOPT_MISSING_OK) != 0);
+ options->missing_ok);
Relation heapRelation;
/* if relation is missing, leave */
@@ -3259,10 +3259,10 @@ ReindexRelationConcurrently(Oid relationOid, int options)
/*
* Check if parent relation can be locked and if it exists,
* this needs to be done at this stage as the list of indexes
- * to rebuild is not complete yet, and REINDEXOPT_MISSING_OK
+ * to rebuild is not complete yet, and missing_ok
* should not be used once all the session locks are taken.
*/
- if ((options & REINDEXOPT_MISSING_OK) != 0)
+ if (options->missing_ok)
{
heapRelation = try_table_open(heapId,
ShareUpdateExclusiveLock);
@@ -3303,7 +3303,7 @@ ReindexRelationConcurrently(Oid relationOid, int options)
/*
* Definitely no indexes, so leave. Any checks based on
- * REINDEXOPT_MISSING_OK should be done only while the list of indexes to
+ * missing_ok should be done only while the list of indexes to
* work on is built as the session locks taken before this transaction
* commits will make sure that they cannot be dropped by a concurrent
* session until this operation completes.
@@ -3754,7 +3754,7 @@ ReindexRelationConcurrently(Oid relationOid, int options)
StartTransactionCommand();
/* Log what we did */
- if (options & REINDEXOPT_VERBOSE)
+ if (options->verbose)
{
if (relkind == RELKIND_INDEX)
ereport(INFO,
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 1fa9f19f08..e0f62d3c77 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -1854,6 +1854,7 @@ ExecuteTruncateGuts(List *explicit_rels, List *relids, List *relids_logged,
{
Oid heap_relid;
Oid toast_relid;
+ ReindexOptions reindexopts = {false}; /* Default options are all false */
/*
* This effectively deletes all rows in the table, and may be done
@@ -1891,7 +1892,8 @@ ExecuteTruncateGuts(List *explicit_rels, List *relids, List *relids_logged,
/*
* Reconstruct the indexes to match, and we're done.
*/
- reindex_relation(heap_relid, REINDEX_REL_PROCESS_TOAST, 0);
+ reindex_relation(heap_relid, REINDEX_REL_PROCESS_TOAST,
+ &reindexopts);
}
pgstat_count_truncate(rel);
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index 98270a1049..99b0345338 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -78,8 +78,8 @@ pg_atomic_uint32 *VacuumActiveNWorkers = NULL;
int VacuumCostBalanceLocal = 0;
/* non-export function prototypes */
-static List *expand_vacuum_rel(VacuumRelation *vrel, int options);
-static List *get_all_vacuum_rels(int options);
+static List *expand_vacuum_rel(VacuumRelation *vrel, VacuumOptions *options);
+static List *get_all_vacuum_rels(VacuumOptions *options);
static void vac_truncate_clog(TransactionId frozenXID,
MultiXactId minMulti,
TransactionId lastSaneFrozenXid,
@@ -97,13 +97,7 @@ static VacOptTernaryValue get_vacopt_ternary_value(DefElem *def);
void
ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel)
{
- VacuumParams params;
- bool verbose = false;
- bool skip_locked = false;
- bool analyze = false;
- bool freeze = false;
- bool full = false;
- bool disable_page_skipping = false;
+ VacuumParams params = {.options={false} };
ListCell *lc;
/* Set default value */
@@ -120,9 +114,9 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel)
/* Parse common options for VACUUM and ANALYZE */
if (strcmp(opt->defname, "verbose") == 0)
- verbose = defGetBoolean(opt);
+ params.options.verbose = defGetBoolean(opt);
else if (strcmp(opt->defname, "skip_locked") == 0)
- skip_locked = defGetBoolean(opt);
+ params.options.skip_locked = defGetBoolean(opt);
else if (!vacstmt->is_vacuumcmd)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
@@ -131,13 +125,13 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel)
/* Parse options available on VACUUM */
else if (strcmp(opt->defname, "analyze") == 0)
- analyze = defGetBoolean(opt);
+ params.options.analyze = defGetBoolean(opt);
else if (strcmp(opt->defname, "freeze") == 0)
- freeze = defGetBoolean(opt);
+ params.options.freeze = defGetBoolean(opt);
else if (strcmp(opt->defname, "full") == 0)
- full = defGetBoolean(opt);
+ params.options.full = defGetBoolean(opt);
else if (strcmp(opt->defname, "disable_page_skipping") == 0)
- disable_page_skipping = defGetBoolean(opt);
+ params.options.disable_page_skipping = defGetBoolean(opt);
else if (strcmp(opt->defname, "index_cleanup") == 0)
params.index_cleanup = get_vacopt_ternary_value(opt);
else if (strcmp(opt->defname, "truncate") == 0)
@@ -181,31 +175,26 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel)
parser_errposition(pstate, opt->location)));
}
- /* Set vacuum options */
- params.options =
- (vacstmt->is_vacuumcmd ? VACOPT_VACUUM : VACOPT_ANALYZE) |
- (verbose ? VACOPT_VERBOSE : 0) |
- (skip_locked ? VACOPT_SKIP_LOCKED : 0) |
- (analyze ? VACOPT_ANALYZE : 0) |
- (freeze ? VACOPT_FREEZE : 0) |
- (full ? VACOPT_FULL : 0) |
- (disable_page_skipping ? VACOPT_DISABLE_PAGE_SKIPPING : 0);
+ if (vacstmt->is_vacuumcmd)
+ params.options.vacuum = true;
+ else
+ params.options.analyze = true;
/* sanity checks on options */
- Assert(params.options & (VACOPT_VACUUM | VACOPT_ANALYZE));
- Assert((params.options & VACOPT_VACUUM) ||
- !(params.options & (VACOPT_FULL | VACOPT_FREEZE)));
- Assert(!(params.options & VACOPT_SKIPTOAST));
+ Assert(params.options.vacuum || params.options.analyze);
+ Assert(params.options.vacuum ||
+ !(params.options.full || params.options.freeze));
+ Assert(!params.options.skiptoast);
- if ((params.options & VACOPT_FULL) && params.nworkers > 0)
+ if (params.options.full && params.nworkers > 0)
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("VACUUM FULL cannot be performed in parallel")));
/*
- * Make sure VACOPT_ANALYZE is specified if any column lists are present.
+ * Make sure analyze is specified if any column lists are present.
*/
- if (!(params.options & VACOPT_ANALYZE))
+ if (!params.options.analyze)
{
ListCell *lc;
@@ -224,7 +213,7 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel)
* All freeze ages are zero if the FREEZE option is given; otherwise pass
* them as -1 which means to use the default values.
*/
- if (params.options & VACOPT_FREEZE)
+ if (params.options.freeze)
{
params.freeze_min_age = 0;
params.freeze_table_age = 0;
@@ -280,7 +269,7 @@ vacuum(List *relations, VacuumParams *params,
Assert(params != NULL);
- stmttype = (params->options & VACOPT_VACUUM) ? "VACUUM" : "ANALYZE";
+ stmttype = params->options.vacuum ? "VACUUM" : "ANALYZE";
/*
* We cannot run VACUUM inside a user transaction block; if we were inside
@@ -290,7 +279,7 @@ vacuum(List *relations, VacuumParams *params,
*
* ANALYZE (without VACUUM) can run either way.
*/
- if (params->options & VACOPT_VACUUM)
+ if (params->options.vacuum)
{
PreventInTransactionBlock(isTopLevel, stmttype);
in_outer_xact = false;
@@ -312,8 +301,8 @@ vacuum(List *relations, VacuumParams *params,
/*
* Sanity check DISABLE_PAGE_SKIPPING option.
*/
- if ((params->options & VACOPT_FULL) != 0 &&
- (params->options & VACOPT_DISABLE_PAGE_SKIPPING) != 0)
+ if (params->options.full &&
+ params->options.disable_page_skipping)
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("VACUUM option DISABLE_PAGE_SKIPPING cannot be used with FULL")));
@@ -322,7 +311,7 @@ vacuum(List *relations, VacuumParams *params,
* Send info about dead objects to the statistics collector, unless we are
* in autovacuum --- autovacuum.c does this for itself.
*/
- if ((params->options & VACOPT_VACUUM) && !IsAutoVacuumWorkerProcess())
+ if (params->options.vacuum && !IsAutoVacuumWorkerProcess())
pgstat_vacuum_stat();
/*
@@ -363,7 +352,7 @@ vacuum(List *relations, VacuumParams *params,
List *sublist;
MemoryContext old_context;
- sublist = expand_vacuum_rel(vrel, params->options);
+ sublist = expand_vacuum_rel(vrel, ¶ms->options);
old_context = MemoryContextSwitchTo(vac_context);
newrels = list_concat(newrels, sublist);
MemoryContextSwitchTo(old_context);
@@ -371,7 +360,7 @@ vacuum(List *relations, VacuumParams *params,
relations = newrels;
}
else
- relations = get_all_vacuum_rels(params->options);
+ relations = get_all_vacuum_rels(¶ms->options);
/*
* Decide whether we need to start/commit our own transactions.
@@ -387,11 +376,11 @@ vacuum(List *relations, VacuumParams *params,
* transaction block, and also in an autovacuum worker, use own
* transactions so we can release locks sooner.
*/
- if (params->options & VACOPT_VACUUM)
+ if (params->options.vacuum)
use_own_xacts = true;
else
{
- Assert(params->options & VACOPT_ANALYZE);
+ Assert(params->options.analyze);
if (IsAutoVacuumWorkerProcess())
use_own_xacts = true;
else if (in_outer_xact)
@@ -444,13 +433,13 @@ vacuum(List *relations, VacuumParams *params,
{
VacuumRelation *vrel = lfirst_node(VacuumRelation, cur);
- if (params->options & VACOPT_VACUUM)
+ if (params->options.vacuum)
{
if (!vacuum_rel(vrel->oid, vrel->relation, params))
continue;
}
- if (params->options & VACOPT_ANALYZE)
+ if (params->options.analyze)
{
/*
* If using separate xacts, start one for analyze. Otherwise,
@@ -504,7 +493,7 @@ vacuum(List *relations, VacuumParams *params,
StartTransactionCommand();
}
- if ((params->options & VACOPT_VACUUM) && !IsAutoVacuumWorkerProcess())
+ if (params->options.vacuum && !IsAutoVacuumWorkerProcess())
{
/*
* Update pg_database.datfrozenxid, and truncate pg_xact if possible.
@@ -530,11 +519,11 @@ vacuum(List *relations, VacuumParams *params,
* ANALYZE.
*/
bool
-vacuum_is_relation_owner(Oid relid, Form_pg_class reltuple, int options)
+vacuum_is_relation_owner(Oid relid, Form_pg_class reltuple, VacuumOptions *options)
{
char *relname;
- Assert((options & (VACOPT_VACUUM | VACOPT_ANALYZE)) != 0);
+ Assert(options->vacuum || options->analyze);
/*
* Check permissions.
@@ -553,7 +542,7 @@ vacuum_is_relation_owner(Oid relid, Form_pg_class reltuple, int options)
relname = NameStr(reltuple->relname);
- if ((options & VACOPT_VACUUM) != 0)
+ if (options->vacuum)
{
if (reltuple->relisshared)
ereport(WARNING,
@@ -576,7 +565,7 @@ vacuum_is_relation_owner(Oid relid, Form_pg_class reltuple, int options)
return false;
}
- if ((options & VACOPT_ANALYZE) != 0)
+ if (options->analyze)
{
if (reltuple->relisshared)
ereport(WARNING,
@@ -604,14 +593,14 @@ vacuum_is_relation_owner(Oid relid, Form_pg_class reltuple, int options)
* or locked, a log is emitted if possible.
*/
Relation
-vacuum_open_relation(Oid relid, RangeVar *relation, int options,
+vacuum_open_relation(Oid relid, RangeVar *relation, VacuumOptions *options,
bool verbose, LOCKMODE lmode)
{
Relation onerel;
bool rel_lock = true;
int elevel;
- Assert((options & (VACOPT_VACUUM | VACOPT_ANALYZE)) != 0);
+ Assert(options->vacuum || options->analyze);
/*
* Open the relation and get the appropriate lock on it.
@@ -622,7 +611,7 @@ vacuum_open_relation(Oid relid, RangeVar *relation, int options,
* If we've been asked not to wait for the relation lock, acquire it first
* in non-blocking mode, before calling try_relation_open().
*/
- if (!(options & VACOPT_SKIP_LOCKED))
+ if (!options->skip_locked)
onerel = try_relation_open(relid, lmode);
else if (ConditionalLockRelationOid(relid, lmode))
onerel = try_relation_open(relid, NoLock);
@@ -662,7 +651,7 @@ vacuum_open_relation(Oid relid, RangeVar *relation, int options,
else
return NULL;
- if ((options & VACOPT_VACUUM) != 0)
+ if (options->vacuum)
{
if (!rel_lock)
ereport(elevel,
@@ -683,7 +672,7 @@ vacuum_open_relation(Oid relid, RangeVar *relation, int options,
return NULL;
}
- if ((options & VACOPT_ANALYZE) != 0)
+ if (options->analyze)
{
if (!rel_lock)
ereport(elevel,
@@ -716,7 +705,7 @@ vacuum_open_relation(Oid relid, RangeVar *relation, int options,
* are made in vac_context.
*/
static List *
-expand_vacuum_rel(VacuumRelation *vrel, int options)
+expand_vacuum_rel(VacuumRelation *vrel, VacuumOptions *options)
{
List *vacrels = NIL;
MemoryContext oldcontext;
@@ -748,7 +737,7 @@ expand_vacuum_rel(VacuumRelation *vrel, int options)
* below, as well as find_all_inheritors's expectation that the caller
* holds some lock on the starting relation.
*/
- rvr_opts = (options & VACOPT_SKIP_LOCKED) ? RVR_SKIP_LOCKED : 0;
+ rvr_opts = (options->skip_locked) ? RVR_SKIP_LOCKED : 0;
relid = RangeVarGetRelidExtended(vrel->relation,
AccessShareLock,
rvr_opts,
@@ -760,7 +749,7 @@ expand_vacuum_rel(VacuumRelation *vrel, int options)
*/
if (!OidIsValid(relid))
{
- if (options & VACOPT_VACUUM)
+ if (options->vacuum)
ereport(WARNING,
(errcode(ERRCODE_LOCK_NOT_AVAILABLE),
errmsg("skipping vacuum of \"%s\" --- lock not available",
@@ -855,7 +844,7 @@ expand_vacuum_rel(VacuumRelation *vrel, int options)
* the current database. The list is built in vac_context.
*/
static List *
-get_all_vacuum_rels(int options)
+get_all_vacuum_rels(VacuumOptions *options)
{
List *vacrels = NIL;
Relation pgclass;
@@ -1722,13 +1711,14 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params)
Oid save_userid;
int save_sec_context;
int save_nestlevel;
+ VacuumOptions newoptions;
Assert(params != NULL);
/* Begin a transaction for vacuuming this relation */
StartTransactionCommand();
- if (!(params->options & VACOPT_FULL))
+ if (!params->options.full)
{
/*
* In lazy vacuum, we can set the PROC_IN_VACUUM flag, which lets
@@ -1778,11 +1768,11 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params)
* vacuum, but just ShareUpdateExclusiveLock for concurrent vacuum. Either
* way, we can be sure that no other backend is vacuuming the same table.
*/
- lmode = (params->options & VACOPT_FULL) ?
+ lmode = (params->options.full) ?
AccessExclusiveLock : ShareUpdateExclusiveLock;
/* open the relation and get the appropriate lock on it */
- onerel = vacuum_open_relation(relid, relation, params->options,
+ onerel = vacuum_open_relation(relid, relation, ¶ms->options,
params->log_min_duration >= 0, lmode);
/* leave if relation could not be opened or locked */
@@ -1801,9 +1791,11 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params)
* changed in-between. Make sure to only generate logs for VACUUM in this
* case.
*/
+ newoptions = params->options;
+ newoptions.analyze = false;
if (!vacuum_is_relation_owner(RelationGetRelid(onerel),
onerel->rd_rel,
- params->options & VACOPT_VACUUM))
+ &newoptions))
{
relation_close(onerel, lmode);
PopActiveSnapshot();
@@ -1895,7 +1887,7 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params)
* us to process it. In VACUUM FULL, though, the toast table is
* automatically rebuilt by cluster_rel so we shouldn't recurse to it.
*/
- if (!(params->options & VACOPT_SKIPTOAST) && !(params->options & VACOPT_FULL))
+ if (!params->options.skiptoast && !params->options.full)
toast_relid = onerel->rd_rel->reltoastrelid;
else
toast_relid = InvalidOid;
@@ -1914,19 +1906,19 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params)
/*
* Do the actual work --- either FULL or "lazy" vacuum
*/
- if (params->options & VACOPT_FULL)
+ if (params->options.full)
{
- int cluster_options = 0;
+ ClusterOptions cluster_options = {
+ .verbose = params->options.verbose,
+ /* Other members initialized to false/0/NULL */
+ };
/* close relation before vacuuming, but hold lock until commit */
relation_close(onerel, NoLock);
onerel = NULL;
- if ((params->options & VACOPT_VERBOSE) != 0)
- cluster_options |= CLUOPT_VERBOSE;
-
/* VACUUM FULL is now a variant of CLUSTER; see cluster.c */
- cluster_rel(relid, InvalidOid, cluster_options);
+ cluster_rel(relid, InvalidOid, &cluster_options);
}
else
table_relation_vacuum(onerel, params, vac_strategy);
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index ed127a1032..8b151bd994 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -2504,7 +2504,7 @@ do_autovacuum(void)
* next table in our list.
*/
HOLD_INTERRUPTS();
- if (tab->at_params.options & VACOPT_VACUUM)
+ if (tab->at_params.options.vacuum)
errcontext("automatic vacuum of table \"%s.%s.%s\"",
tab->at_datname, tab->at_nspname, tab->at_relname);
else
@@ -2918,10 +2918,10 @@ table_recheck_autovac(Oid relid, HTAB *table_toast_map,
tab = palloc(sizeof(autovac_table));
tab->at_relid = relid;
tab->at_sharedrel = classForm->relisshared;
- tab->at_params.options = VACOPT_SKIPTOAST |
- (dovacuum ? VACOPT_VACUUM : 0) |
- (doanalyze ? VACOPT_ANALYZE : 0) |
- (!wraparound ? VACOPT_SKIP_LOCKED : 0);
+ tab->at_params.options.skiptoast = true;
+ tab->at_params.options.vacuum = dovacuum;
+ tab->at_params.options.analyze = doanalyze;
+ tab->at_params.options.skip_locked = !wraparound;
tab->at_params.index_cleanup = VACOPT_TERNARY_DEFAULT;
tab->at_params.truncate = VACOPT_TERNARY_DEFAULT;
/* As of now, we don't support parallel vacuum for autovacuum */
@@ -3249,10 +3249,10 @@ autovac_report_activity(autovac_table *tab)
int len;
/* Report the command and possible options */
- if (tab->at_params.options & VACOPT_VACUUM)
+ if (tab->at_params.options.vacuum)
snprintf(activity, MAX_AUTOVAC_ACTIV_LEN,
"autovacuum: VACUUM%s",
- tab->at_params.options & VACOPT_ANALYZE ? " ANALYZE" : "");
+ tab->at_params.options.analyze ? " ANALYZE" : "");
else
snprintf(activity, MAX_AUTOVAC_ACTIV_LEN,
"autovacuum: ANALYZE");
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index a42ead7d69..a6c42ee599 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -919,20 +919,20 @@ standard_ProcessUtility(PlannedStmt *pstmt,
case T_ReindexStmt:
{
ReindexStmt *stmt = (ReindexStmt *) parsetree;
- int options;
+ ReindexOptions options;
options = ReindexParseOptions(pstate, stmt);
- if ((options & REINDEXOPT_CONCURRENTLY) != 0)
+ if (options.concurrently)
PreventInTransactionBlock(isTopLevel,
"REINDEX CONCURRENTLY");
switch (stmt->kind)
{
case REINDEX_OBJECT_INDEX:
- ReindexIndex(stmt->relation, options, isTopLevel);
+ ReindexIndex(stmt->relation, &options, isTopLevel);
break;
case REINDEX_OBJECT_TABLE:
- ReindexTable(stmt->relation, options, isTopLevel);
+ ReindexTable(stmt->relation, &options, isTopLevel);
break;
case REINDEX_OBJECT_SCHEMA:
case REINDEX_OBJECT_SYSTEM:
@@ -948,7 +948,7 @@ standard_ProcessUtility(PlannedStmt *pstmt,
(stmt->kind == REINDEX_OBJECT_SCHEMA) ? "REINDEX SCHEMA" :
(stmt->kind == REINDEX_OBJECT_SYSTEM) ? "REINDEX SYSTEM" :
"REINDEX DATABASE");
- ReindexMultipleTables(stmt->name, stmt->kind, options);
+ ReindexMultipleTables(stmt->name, stmt->kind, &options);
break;
default:
elog(ERROR, "unrecognized object type: %d",
diff --git a/src/include/catalog/index.h b/src/include/catalog/index.h
index c041628049..d7e7e0dd24 100644
--- a/src/include/catalog/index.h
+++ b/src/include/catalog/index.h
@@ -30,13 +30,13 @@ typedef enum
} IndexStateFlagsAction;
/* options for REINDEX */
-typedef enum ReindexOption
+typedef struct ReindexOptions
{
- REINDEXOPT_VERBOSE = 1 << 0, /* print progress info */
- REINDEXOPT_REPORT_PROGRESS = 1 << 1, /* report pgstat progress */
- REINDEXOPT_MISSING_OK = 1 << 2, /* skip missing relations */
- REINDEXOPT_CONCURRENTLY = 1 << 3 /* concurrent mode */
-} ReindexOption;
+ bool verbose; /* print progress info */
+ bool report_progress; /* report pgstat progress */
+ bool missing_ok; /* skip missing relations */
+ bool concurrently; /* concurrent mode */
+} ReindexOptions;
/* state info for validate_index bulkdelete callback */
typedef struct ValidateIndexState
@@ -146,7 +146,7 @@ extern void index_set_state_flags(Oid indexId, IndexStateFlagsAction action);
extern Oid IndexGetRelation(Oid indexId, bool missing_ok);
extern void reindex_index(Oid indexId, bool skip_constraint_checks,
- char relpersistence, int options);
+ char relpersistence, ReindexOptions *options);
/* Flag bits for reindex_relation(): */
#define REINDEX_REL_PROCESS_TOAST 0x01
@@ -155,7 +155,7 @@ extern void reindex_index(Oid indexId, bool skip_constraint_checks,
#define REINDEX_REL_FORCE_INDEXES_UNLOGGED 0x08
#define REINDEX_REL_FORCE_INDEXES_PERMANENT 0x10
-extern bool reindex_relation(Oid relid, int flags, int options);
+extern bool reindex_relation(Oid relid, int flags, ReindexOptions *options);
extern bool ReindexIsProcessingHeap(Oid heapOid);
extern bool ReindexIsProcessingIndex(Oid indexOid);
diff --git a/src/include/commands/cluster.h b/src/include/commands/cluster.h
index 7cfb37c9b2..0e661f6ce9 100644
--- a/src/include/commands/cluster.h
+++ b/src/include/commands/cluster.h
@@ -20,14 +20,14 @@
/* options for CLUSTER */
-typedef enum ClusterOption
+typedef struct ClusterOptions
{
- CLUOPT_RECHECK = 1 << 0, /* recheck relation state */
- CLUOPT_VERBOSE = 1 << 1 /* print progress info */
-} ClusterOption;
+ bool recheck; /* recheck relation state */
+ bool verbose; /* print progress info */
+} ClusterOptions;
extern void cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel);
-extern void cluster_rel(Oid tableOid, Oid indexOid, int options);
+extern void cluster_rel(Oid tableOid, Oid indexOid, ClusterOptions *options);
extern void check_index_is_clusterable(Relation OldHeap, Oid indexOid,
bool recheck, LOCKMODE lockmode);
extern void mark_index_clustered(Relation rel, Oid indexOid, bool is_internal);
diff --git a/src/include/commands/defrem.h b/src/include/commands/defrem.h
index 1133ae1143..33df5d5780 100644
--- a/src/include/commands/defrem.h
+++ b/src/include/commands/defrem.h
@@ -14,6 +14,7 @@
#ifndef DEFREM_H
#define DEFREM_H
+#include "catalog/index.h"
#include "catalog/objectaddress.h"
#include "nodes/params.h"
#include "parser/parse_node.h"
@@ -34,11 +35,11 @@ extern ObjectAddress DefineIndex(Oid relationId,
bool check_not_in_use,
bool skip_build,
bool quiet);
-extern int ReindexParseOptions(ParseState *pstate, ReindexStmt *stmt);
-extern void ReindexIndex(RangeVar *indexRelation, int options, bool isTopLevel);
-extern Oid ReindexTable(RangeVar *relation, int options, bool isTopLevel);
+extern ReindexOptions ReindexParseOptions(ParseState *pstate, ReindexStmt *stmt);
+extern void ReindexIndex(RangeVar *indexRelation, ReindexOptions *options, bool isTopLevel);
+extern Oid ReindexTable(RangeVar *relation, ReindexOptions *options, bool isTopLevel);
extern void ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
- int options);
+ ReindexOptions *options);
extern char *makeObjectName(const char *name1, const char *name2,
const char *label);
extern char *ChooseRelationName(const char *name1, const char *name2,
diff --git a/src/include/commands/vacuum.h b/src/include/commands/vacuum.h
index a4cd721400..d9e5485a1e 100644
--- a/src/include/commands/vacuum.h
+++ b/src/include/commands/vacuum.h
@@ -174,17 +174,17 @@ typedef struct VacAttrStats
int rowstride;
} VacAttrStats;
-typedef enum VacuumOption
+typedef struct VacuumOptions
{
- VACOPT_VACUUM = 1 << 0, /* do VACUUM */
- VACOPT_ANALYZE = 1 << 1, /* do ANALYZE */
- VACOPT_VERBOSE = 1 << 2, /* print progress info */
- VACOPT_FREEZE = 1 << 3, /* FREEZE option */
- VACOPT_FULL = 1 << 4, /* FULL (non-concurrent) vacuum */
- VACOPT_SKIP_LOCKED = 1 << 5, /* skip if cannot get lock */
- VACOPT_SKIPTOAST = 1 << 6, /* don't process the TOAST table, if any */
- VACOPT_DISABLE_PAGE_SKIPPING = 1 << 7 /* don't skip any pages */
-} VacuumOption;
+ bool vacuum; /* do VACUUM */
+ bool analyze; /* do ANALYZE */
+ bool verbose; /* print progress info */
+ bool freeze; /* FREEZE option */
+ bool full; /* FULL (non-concurrent) vacuum */
+ bool skip_locked; /* skip if cannot get lock */
+ bool skiptoast; /* don't process the TOAST table, if any */
+ bool disable_page_skipping; /* don't skip any pages */
+} VacuumOptions;
/*
* A ternary value used by vacuum parameters.
@@ -202,12 +202,12 @@ typedef enum VacOptTernaryValue
/*
* Parameters customizing behavior of VACUUM and ANALYZE.
*
- * Note that at least one of VACOPT_VACUUM and VACOPT_ANALYZE must be set
+ * Note that at least one of VACUUM and ANALYZE must be set
* in options.
*/
typedef struct VacuumParams
{
- int options; /* bitmask of VacuumOption */
+ VacuumOptions options; /* struct of boolean options */
int freeze_min_age; /* min freeze age, -1 to use default */
int freeze_table_age; /* age at which to scan whole table */
int multixact_freeze_min_age; /* min multixact freeze age, -1 to
@@ -275,9 +275,10 @@ extern void vacuum_set_xid_limits(Relation rel,
extern void vac_update_datfrozenxid(void);
extern void vacuum_delay_point(void);
extern bool vacuum_is_relation_owner(Oid relid, Form_pg_class reltuple,
- int options);
+ VacuumOptions *options);
extern Relation vacuum_open_relation(Oid relid, RangeVar *relation,
- int options, bool verbose, LOCKMODE lmode);
+ VacuumOptions *options, bool verbose,
+ LOCKMODE lmode);
/* in commands/analyze.c */
extern void analyze_rel(Oid relid, RangeVar *relation,
--
2.17.0
>From b0228c88490a8dc7824252b3f2f15eeb1640f8fe Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
Date: Tue, 15 Dec 2020 11:43:57 -0600
Subject: [PATCH 2/4] Change vacuum_open_relation/vacuum_is_relation_owner to
accept booleans rather than VacuumOptions
---
src/backend/commands/analyze.c | 9 ++++----
src/backend/commands/vacuum.c | 39 +++++++++++++++++-----------------
src/include/commands/vacuum.h | 5 +++--
3 files changed, 27 insertions(+), 26 deletions(-)
diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c
index 884bfeee21..14c436b8c0 100644
--- a/src/backend/commands/analyze.c
+++ b/src/backend/commands/analyze.c
@@ -124,7 +124,6 @@ analyze_rel(Oid relid, RangeVar *relation,
int elevel;
AcquireSampleRowsFunc acquirefunc = NULL;
BlockNumber relpages = 0;
- VacuumOptions newoptions;
/* Select logging level */
if (params->options.verbose)
@@ -149,10 +148,10 @@ analyze_rel(Oid relid, RangeVar *relation,
*
* Make sure to generate only logs for ANALYZE in this case.
*/
- newoptions = params->options;
- newoptions.vacuum = false;
- onerel = vacuum_open_relation(relid, relation, &newoptions,
+ Assert(params->options.analyze);
+ onerel = vacuum_open_relation(relid, relation, false, true,
params->log_min_duration >= 0,
+ params->options.skip_locked,
ShareUpdateExclusiveLock);
/* leave if relation could not be opened or locked */
@@ -169,7 +168,7 @@ analyze_rel(Oid relid, RangeVar *relation,
*/
if (!vacuum_is_relation_owner(RelationGetRelid(onerel),
onerel->rd_rel,
- &newoptions))
+ false, true))
{
relation_close(onerel, ShareUpdateExclusiveLock);
return;
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index 99b0345338..2652880474 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -519,11 +519,12 @@ vacuum(List *relations, VacuumParams *params,
* ANALYZE.
*/
bool
-vacuum_is_relation_owner(Oid relid, Form_pg_class reltuple, VacuumOptions *options)
+vacuum_is_relation_owner(Oid relid, Form_pg_class reltuple, bool do_vacuum,
+ bool do_analyze)
{
char *relname;
- Assert(options->vacuum || options->analyze);
+ Assert(do_vacuum || do_analyze);
/*
* Check permissions.
@@ -542,7 +543,7 @@ vacuum_is_relation_owner(Oid relid, Form_pg_class reltuple, VacuumOptions *optio
relname = NameStr(reltuple->relname);
- if (options->vacuum)
+ if (do_vacuum)
{
if (reltuple->relisshared)
ereport(WARNING,
@@ -565,7 +566,7 @@ vacuum_is_relation_owner(Oid relid, Form_pg_class reltuple, VacuumOptions *optio
return false;
}
- if (options->analyze)
+ if (do_analyze)
{
if (reltuple->relisshared)
ereport(WARNING,
@@ -593,14 +594,15 @@ vacuum_is_relation_owner(Oid relid, Form_pg_class reltuple, VacuumOptions *optio
* or locked, a log is emitted if possible.
*/
Relation
-vacuum_open_relation(Oid relid, RangeVar *relation, VacuumOptions *options,
- bool verbose, LOCKMODE lmode)
+vacuum_open_relation(Oid relid, RangeVar *relation, bool do_vacuum,
+ bool do_analyze, bool verbose, bool skip_locked,
+ LOCKMODE lmode)
{
Relation onerel;
bool rel_lock = true;
int elevel;
- Assert(options->vacuum || options->analyze);
+ Assert(do_vacuum || do_analyze);
/*
* Open the relation and get the appropriate lock on it.
@@ -611,7 +613,7 @@ vacuum_open_relation(Oid relid, RangeVar *relation, VacuumOptions *options,
* If we've been asked not to wait for the relation lock, acquire it first
* in non-blocking mode, before calling try_relation_open().
*/
- if (!options->skip_locked)
+ if (!skip_locked)
onerel = try_relation_open(relid, lmode);
else if (ConditionalLockRelationOid(relid, lmode))
onerel = try_relation_open(relid, NoLock);
@@ -651,7 +653,7 @@ vacuum_open_relation(Oid relid, RangeVar *relation, VacuumOptions *options,
else
return NULL;
- if (options->vacuum)
+ if (do_vacuum)
{
if (!rel_lock)
ereport(elevel,
@@ -672,7 +674,7 @@ vacuum_open_relation(Oid relid, RangeVar *relation, VacuumOptions *options,
return NULL;
}
- if (options->analyze)
+ if (do_analyze)
{
if (!rel_lock)
ereport(elevel,
@@ -775,7 +777,8 @@ expand_vacuum_rel(VacuumRelation *vrel, VacuumOptions *options)
* Make a returnable VacuumRelation for this rel if user is a proper
* owner.
*/
- if (vacuum_is_relation_owner(relid, classForm, options))
+ if (vacuum_is_relation_owner(relid, classForm, options->vacuum,
+ options->analyze))
{
oldcontext = MemoryContextSwitchTo(vac_context);
vacrels = lappend(vacrels, makeVacuumRelation(vrel->relation,
@@ -862,7 +865,8 @@ get_all_vacuum_rels(VacuumOptions *options)
Oid relid = classForm->oid;
/* check permissions of relation */
- if (!vacuum_is_relation_owner(relid, classForm, options))
+ if (!vacuum_is_relation_owner(relid, classForm, options->vacuum,
+ options->analyze))
continue;
/*
@@ -1711,7 +1715,6 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params)
Oid save_userid;
int save_sec_context;
int save_nestlevel;
- VacuumOptions newoptions;
Assert(params != NULL);
@@ -1772,8 +1775,9 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params)
AccessExclusiveLock : ShareUpdateExclusiveLock;
/* open the relation and get the appropriate lock on it */
- onerel = vacuum_open_relation(relid, relation, ¶ms->options,
- params->log_min_duration >= 0, lmode);
+ Assert(params->options.vacuum);
+ onerel = vacuum_open_relation(relid, relation, true, false,
+ params->log_min_duration >= 0, params->options.skip_locked, lmode);
/* leave if relation could not be opened or locked */
if (!onerel)
@@ -1791,11 +1795,8 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params)
* changed in-between. Make sure to only generate logs for VACUUM in this
* case.
*/
- newoptions = params->options;
- newoptions.analyze = false;
if (!vacuum_is_relation_owner(RelationGetRelid(onerel),
- onerel->rd_rel,
- &newoptions))
+ onerel->rd_rel, true, false))
{
relation_close(onerel, lmode);
PopActiveSnapshot();
diff --git a/src/include/commands/vacuum.h b/src/include/commands/vacuum.h
index d9e5485a1e..740fa7aaa1 100644
--- a/src/include/commands/vacuum.h
+++ b/src/include/commands/vacuum.h
@@ -275,9 +275,10 @@ extern void vacuum_set_xid_limits(Relation rel,
extern void vac_update_datfrozenxid(void);
extern void vacuum_delay_point(void);
extern bool vacuum_is_relation_owner(Oid relid, Form_pg_class reltuple,
- VacuumOptions *options);
+ bool do_vacuum, bool do_analyze);
extern Relation vacuum_open_relation(Oid relid, RangeVar *relation,
- VacuumOptions *options, bool verbose,
+ bool do_vacuum, bool do_analyze,
+ bool verbose, bool skip_locked,
LOCKMODE lmode);
/* in commands/analyze.c */
--
2.17.0
>From cc8ed3c40ef36dc19fb30f38d445983baac40d1e Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
Date: Mon, 14 Dec 2020 21:18:36 -0600
Subject: [PATCH 3/4] Move booleans directly into VacuumParams
---
src/backend/access/heap/vacuumlazy.c | 8 +--
src/backend/commands/analyze.c | 12 ++--
src/backend/commands/vacuum.c | 90 ++++++++++++++--------------
src/backend/postmaster/autovacuum.c | 14 ++---
src/include/commands/vacuum.h | 26 ++++----
5 files changed, 73 insertions(+), 77 deletions(-)
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index 6bfed2b2da..f7995b33a8 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -456,7 +456,7 @@ heap_vacuum_rel(Relation onerel, VacuumParams *params,
starttime = GetCurrentTimestamp();
}
- if (params->options.verbose)
+ if (params->verbose)
elevel = INFO;
else
elevel = DEBUG2;
@@ -484,7 +484,7 @@ heap_vacuum_rel(Relation onerel, VacuumParams *params,
xidFullScanLimit);
aggressive |= MultiXactIdPrecedesOrEquals(onerel->rd_rel->relminmxid,
mxactFullScanLimit);
- if (params->options.disable_page_skipping)
+ if (params->disable_page_skipping)
aggressive = true;
vacrelstats = (LVRelStats *) palloc0(sizeof(LVRelStats));
@@ -902,7 +902,7 @@ lazy_scan_heap(Relation onerel, VacuumParams *params, LVRelStats *vacrelstats,
* be replayed on any hot standby, where it can be disruptive.
*/
next_unskippable_block = 0;
- if (!params->options.disable_page_skipping)
+ if (!params->disable_page_skipping)
{
while (next_unskippable_block < nblocks)
{
@@ -960,7 +960,7 @@ lazy_scan_heap(Relation onerel, VacuumParams *params, LVRelStats *vacrelstats,
{
/* Time to advance next_unskippable_block */
next_unskippable_block++;
- if (!params->options.disable_page_skipping)
+ if (!params->disable_page_skipping)
{
while (next_unskippable_block < nblocks)
{
diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c
index 14c436b8c0..175309a834 100644
--- a/src/backend/commands/analyze.c
+++ b/src/backend/commands/analyze.c
@@ -126,7 +126,7 @@ analyze_rel(Oid relid, RangeVar *relation,
BlockNumber relpages = 0;
/* Select logging level */
- if (params->options.verbose)
+ if (params->verbose)
elevel = INFO;
else
elevel = DEBUG2;
@@ -148,10 +148,10 @@ analyze_rel(Oid relid, RangeVar *relation,
*
* Make sure to generate only logs for ANALYZE in this case.
*/
- Assert(params->options.analyze);
+ Assert(params->do_analyze);
onerel = vacuum_open_relation(relid, relation, false, true,
params->log_min_duration >= 0,
- params->options.skip_locked,
+ params->skip_locked,
ShareUpdateExclusiveLock);
/* leave if relation could not be opened or locked */
@@ -240,7 +240,7 @@ analyze_rel(Oid relid, RangeVar *relation,
else
{
/* No need for a WARNING if we already complained during VACUUM */
- if (!params->options.vacuum)
+ if (!params->do_vacuum)
ereport(WARNING,
(errmsg("skipping \"%s\" --- cannot analyze non-tables or special system tables",
RelationGetRelationName(onerel))));
@@ -626,7 +626,7 @@ do_analyze_rel(Relation onerel, VacuumParams *params,
* VACUUM ANALYZE, don't overwrite the accurate count already inserted by
* VACUUM.
*/
- if (!inh && !params->options.vacuum)
+ if (!inh && !params->do_vacuum)
{
for (ind = 0; ind < nindexes; ind++)
{
@@ -657,7 +657,7 @@ do_analyze_rel(Relation onerel, VacuumParams *params,
(va_cols == NIL));
/* If this isn't part of VACUUM ANALYZE, let index AMs do cleanup */
- if (!params->options.vacuum)
+ if (!params->do_vacuum)
{
for (ind = 0; ind < nindexes; ind++)
{
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index 2652880474..5fa88de650 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -78,8 +78,8 @@ pg_atomic_uint32 *VacuumActiveNWorkers = NULL;
int VacuumCostBalanceLocal = 0;
/* non-export function prototypes */
-static List *expand_vacuum_rel(VacuumRelation *vrel, VacuumOptions *options);
-static List *get_all_vacuum_rels(VacuumOptions *options);
+static List *expand_vacuum_rel(VacuumRelation *vrel, VacuumParams *params);
+static List *get_all_vacuum_rels(VacuumParams *params);
static void vac_truncate_clog(TransactionId frozenXID,
MultiXactId minMulti,
TransactionId lastSaneFrozenXid,
@@ -97,7 +97,7 @@ static VacOptTernaryValue get_vacopt_ternary_value(DefElem *def);
void
ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel)
{
- VacuumParams params = {.options={false} };
+ VacuumParams params = {false};
ListCell *lc;
/* Set default value */
@@ -114,9 +114,9 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel)
/* Parse common options for VACUUM and ANALYZE */
if (strcmp(opt->defname, "verbose") == 0)
- params.options.verbose = defGetBoolean(opt);
+ params.verbose = defGetBoolean(opt);
else if (strcmp(opt->defname, "skip_locked") == 0)
- params.options.skip_locked = defGetBoolean(opt);
+ params.skip_locked = defGetBoolean(opt);
else if (!vacstmt->is_vacuumcmd)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
@@ -125,13 +125,13 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel)
/* Parse options available on VACUUM */
else if (strcmp(opt->defname, "analyze") == 0)
- params.options.analyze = defGetBoolean(opt);
+ params.do_analyze = defGetBoolean(opt);
else if (strcmp(opt->defname, "freeze") == 0)
- params.options.freeze = defGetBoolean(opt);
+ params.freeze = defGetBoolean(opt);
else if (strcmp(opt->defname, "full") == 0)
- params.options.full = defGetBoolean(opt);
+ params.full = defGetBoolean(opt);
else if (strcmp(opt->defname, "disable_page_skipping") == 0)
- params.options.disable_page_skipping = defGetBoolean(opt);
+ params.disable_page_skipping = defGetBoolean(opt);
else if (strcmp(opt->defname, "index_cleanup") == 0)
params.index_cleanup = get_vacopt_ternary_value(opt);
else if (strcmp(opt->defname, "truncate") == 0)
@@ -176,17 +176,17 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel)
}
if (vacstmt->is_vacuumcmd)
- params.options.vacuum = true;
+ params.do_vacuum = true;
else
- params.options.analyze = true;
+ params.do_analyze = true;
/* sanity checks on options */
- Assert(params.options.vacuum || params.options.analyze);
- Assert(params.options.vacuum ||
- !(params.options.full || params.options.freeze));
- Assert(!params.options.skiptoast);
+ Assert(params.do_vacuum || params.do_analyze);
+ Assert(params.do_vacuum ||
+ !(params.full || params.freeze));
+ Assert(!params.skiptoast);
- if (params.options.full && params.nworkers > 0)
+ if (params.full && params.nworkers > 0)
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("VACUUM FULL cannot be performed in parallel")));
@@ -194,7 +194,7 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel)
/*
* Make sure analyze is specified if any column lists are present.
*/
- if (!params.options.analyze)
+ if (!params.do_analyze)
{
ListCell *lc;
@@ -213,7 +213,7 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel)
* All freeze ages are zero if the FREEZE option is given; otherwise pass
* them as -1 which means to use the default values.
*/
- if (params.options.freeze)
+ if (params.freeze)
{
params.freeze_min_age = 0;
params.freeze_table_age = 0;
@@ -269,7 +269,7 @@ vacuum(List *relations, VacuumParams *params,
Assert(params != NULL);
- stmttype = params->options.vacuum ? "VACUUM" : "ANALYZE";
+ stmttype = params->do_vacuum ? "VACUUM" : "ANALYZE";
/*
* We cannot run VACUUM inside a user transaction block; if we were inside
@@ -279,7 +279,7 @@ vacuum(List *relations, VacuumParams *params,
*
* ANALYZE (without VACUUM) can run either way.
*/
- if (params->options.vacuum)
+ if (params->do_vacuum)
{
PreventInTransactionBlock(isTopLevel, stmttype);
in_outer_xact = false;
@@ -301,8 +301,8 @@ vacuum(List *relations, VacuumParams *params,
/*
* Sanity check DISABLE_PAGE_SKIPPING option.
*/
- if (params->options.full &&
- params->options.disable_page_skipping)
+ if (params->full &&
+ params->disable_page_skipping)
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("VACUUM option DISABLE_PAGE_SKIPPING cannot be used with FULL")));
@@ -311,7 +311,7 @@ vacuum(List *relations, VacuumParams *params,
* Send info about dead objects to the statistics collector, unless we are
* in autovacuum --- autovacuum.c does this for itself.
*/
- if (params->options.vacuum && !IsAutoVacuumWorkerProcess())
+ if (params->do_vacuum && !IsAutoVacuumWorkerProcess())
pgstat_vacuum_stat();
/*
@@ -352,7 +352,7 @@ vacuum(List *relations, VacuumParams *params,
List *sublist;
MemoryContext old_context;
- sublist = expand_vacuum_rel(vrel, ¶ms->options);
+ sublist = expand_vacuum_rel(vrel, params);
old_context = MemoryContextSwitchTo(vac_context);
newrels = list_concat(newrels, sublist);
MemoryContextSwitchTo(old_context);
@@ -360,7 +360,7 @@ vacuum(List *relations, VacuumParams *params,
relations = newrels;
}
else
- relations = get_all_vacuum_rels(¶ms->options);
+ relations = get_all_vacuum_rels(params);
/*
* Decide whether we need to start/commit our own transactions.
@@ -376,11 +376,11 @@ vacuum(List *relations, VacuumParams *params,
* transaction block, and also in an autovacuum worker, use own
* transactions so we can release locks sooner.
*/
- if (params->options.vacuum)
+ if (params->do_vacuum)
use_own_xacts = true;
else
{
- Assert(params->options.analyze);
+ Assert(params->do_analyze);
if (IsAutoVacuumWorkerProcess())
use_own_xacts = true;
else if (in_outer_xact)
@@ -433,13 +433,13 @@ vacuum(List *relations, VacuumParams *params,
{
VacuumRelation *vrel = lfirst_node(VacuumRelation, cur);
- if (params->options.vacuum)
+ if (params->do_vacuum)
{
if (!vacuum_rel(vrel->oid, vrel->relation, params))
continue;
}
- if (params->options.analyze)
+ if (params->do_analyze)
{
/*
* If using separate xacts, start one for analyze. Otherwise,
@@ -493,7 +493,7 @@ vacuum(List *relations, VacuumParams *params,
StartTransactionCommand();
}
- if (params->options.vacuum && !IsAutoVacuumWorkerProcess())
+ if (params->do_vacuum && !IsAutoVacuumWorkerProcess())
{
/*
* Update pg_database.datfrozenxid, and truncate pg_xact if possible.
@@ -707,7 +707,7 @@ vacuum_open_relation(Oid relid, RangeVar *relation, bool do_vacuum,
* are made in vac_context.
*/
static List *
-expand_vacuum_rel(VacuumRelation *vrel, VacuumOptions *options)
+expand_vacuum_rel(VacuumRelation *vrel, VacuumParams *params)
{
List *vacrels = NIL;
MemoryContext oldcontext;
@@ -739,7 +739,7 @@ expand_vacuum_rel(VacuumRelation *vrel, VacuumOptions *options)
* below, as well as find_all_inheritors's expectation that the caller
* holds some lock on the starting relation.
*/
- rvr_opts = (options->skip_locked) ? RVR_SKIP_LOCKED : 0;
+ rvr_opts = (params->skip_locked) ? RVR_SKIP_LOCKED : 0;
relid = RangeVarGetRelidExtended(vrel->relation,
AccessShareLock,
rvr_opts,
@@ -751,7 +751,7 @@ expand_vacuum_rel(VacuumRelation *vrel, VacuumOptions *options)
*/
if (!OidIsValid(relid))
{
- if (options->vacuum)
+ if (params->do_vacuum)
ereport(WARNING,
(errcode(ERRCODE_LOCK_NOT_AVAILABLE),
errmsg("skipping vacuum of \"%s\" --- lock not available",
@@ -777,8 +777,8 @@ expand_vacuum_rel(VacuumRelation *vrel, VacuumOptions *options)
* Make a returnable VacuumRelation for this rel if user is a proper
* owner.
*/
- if (vacuum_is_relation_owner(relid, classForm, options->vacuum,
- options->analyze))
+ if (vacuum_is_relation_owner(relid, classForm, params->do_vacuum,
+ params->do_analyze))
{
oldcontext = MemoryContextSwitchTo(vac_context);
vacrels = lappend(vacrels, makeVacuumRelation(vrel->relation,
@@ -847,7 +847,7 @@ expand_vacuum_rel(VacuumRelation *vrel, VacuumOptions *options)
* the current database. The list is built in vac_context.
*/
static List *
-get_all_vacuum_rels(VacuumOptions *options)
+get_all_vacuum_rels(VacuumParams *params)
{
List *vacrels = NIL;
Relation pgclass;
@@ -865,8 +865,8 @@ get_all_vacuum_rels(VacuumOptions *options)
Oid relid = classForm->oid;
/* check permissions of relation */
- if (!vacuum_is_relation_owner(relid, classForm, options->vacuum,
- options->analyze))
+ if (!vacuum_is_relation_owner(relid, classForm, params->do_vacuum,
+ params->do_analyze))
continue;
/*
@@ -1721,7 +1721,7 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params)
/* Begin a transaction for vacuuming this relation */
StartTransactionCommand();
- if (!params->options.full)
+ if (!params->full)
{
/*
* In lazy vacuum, we can set the PROC_IN_VACUUM flag, which lets
@@ -1771,13 +1771,13 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params)
* vacuum, but just ShareUpdateExclusiveLock for concurrent vacuum. Either
* way, we can be sure that no other backend is vacuuming the same table.
*/
- lmode = (params->options.full) ?
+ lmode = (params->full) ?
AccessExclusiveLock : ShareUpdateExclusiveLock;
/* open the relation and get the appropriate lock on it */
- Assert(params->options.vacuum);
+ Assert(params->do_vacuum);
onerel = vacuum_open_relation(relid, relation, true, false,
- params->log_min_duration >= 0, params->options.skip_locked, lmode);
+ params->log_min_duration >= 0, params->skip_locked, lmode);
/* leave if relation could not be opened or locked */
if (!onerel)
@@ -1888,7 +1888,7 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params)
* us to process it. In VACUUM FULL, though, the toast table is
* automatically rebuilt by cluster_rel so we shouldn't recurse to it.
*/
- if (!params->options.skiptoast && !params->options.full)
+ if (!params->skiptoast && !params->full)
toast_relid = onerel->rd_rel->reltoastrelid;
else
toast_relid = InvalidOid;
@@ -1907,10 +1907,10 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params)
/*
* Do the actual work --- either FULL or "lazy" vacuum
*/
- if (params->options.full)
+ if (params->full)
{
ClusterOptions cluster_options = {
- .verbose = params->options.verbose,
+ .verbose = params->verbose,
/* Other members initialized to false/0/NULL */
};
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index 8b151bd994..1f8d0d1b88 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -2504,7 +2504,7 @@ do_autovacuum(void)
* next table in our list.
*/
HOLD_INTERRUPTS();
- if (tab->at_params.options.vacuum)
+ if (tab->at_params.do_vacuum)
errcontext("automatic vacuum of table \"%s.%s.%s\"",
tab->at_datname, tab->at_nspname, tab->at_relname);
else
@@ -2918,10 +2918,10 @@ table_recheck_autovac(Oid relid, HTAB *table_toast_map,
tab = palloc(sizeof(autovac_table));
tab->at_relid = relid;
tab->at_sharedrel = classForm->relisshared;
- tab->at_params.options.skiptoast = true;
- tab->at_params.options.vacuum = dovacuum;
- tab->at_params.options.analyze = doanalyze;
- tab->at_params.options.skip_locked = !wraparound;
+ tab->at_params.skiptoast = true;
+ tab->at_params.do_vacuum = dovacuum;
+ tab->at_params.do_analyze = doanalyze;
+ tab->at_params.skip_locked = !wraparound;
tab->at_params.index_cleanup = VACOPT_TERNARY_DEFAULT;
tab->at_params.truncate = VACOPT_TERNARY_DEFAULT;
/* As of now, we don't support parallel vacuum for autovacuum */
@@ -3249,10 +3249,10 @@ autovac_report_activity(autovac_table *tab)
int len;
/* Report the command and possible options */
- if (tab->at_params.options.vacuum)
+ if (tab->at_params.do_vacuum)
snprintf(activity, MAX_AUTOVAC_ACTIV_LEN,
"autovacuum: VACUUM%s",
- tab->at_params.options.analyze ? " ANALYZE" : "");
+ tab->at_params.do_analyze ? " ANALYZE" : "");
else
snprintf(activity, MAX_AUTOVAC_ACTIV_LEN,
"autovacuum: ANALYZE");
diff --git a/src/include/commands/vacuum.h b/src/include/commands/vacuum.h
index 740fa7aaa1..e51f2adcd5 100644
--- a/src/include/commands/vacuum.h
+++ b/src/include/commands/vacuum.h
@@ -174,18 +174,6 @@ typedef struct VacAttrStats
int rowstride;
} VacAttrStats;
-typedef struct VacuumOptions
-{
- bool vacuum; /* do VACUUM */
- bool analyze; /* do ANALYZE */
- bool verbose; /* print progress info */
- bool freeze; /* FREEZE option */
- bool full; /* FULL (non-concurrent) vacuum */
- bool skip_locked; /* skip if cannot get lock */
- bool skiptoast; /* don't process the TOAST table, if any */
- bool disable_page_skipping; /* don't skip any pages */
-} VacuumOptions;
-
/*
* A ternary value used by vacuum parameters.
*
@@ -202,12 +190,20 @@ typedef enum VacOptTernaryValue
/*
* Parameters customizing behavior of VACUUM and ANALYZE.
*
- * Note that at least one of VACUUM and ANALYZE must be set
- * in options.
+ * Note that at least one of VACUUM and ANALYZE must be set.
*/
typedef struct VacuumParams
{
- VacuumOptions options; /* struct of boolean options */
+ /* boolean options */
+ bool do_vacuum; /* do VACUUM */
+ bool do_analyze; /* do ANALYZE */
+ bool verbose; /* print progress info */
+ bool freeze; /* FREEZE option */
+ bool full; /* FULL (non-concurrent) vacuum */
+ bool skip_locked; /* skip if cannot get lock */
+ bool skiptoast; /* don't process the TOAST table, if any */
+ bool disable_page_skipping; /* don't skip any pages */
+
int freeze_min_age; /* min freeze age, -1 to use default */
int freeze_table_age; /* age at which to scan whole table */
int multixact_freeze_min_age; /* min multixact freeze age, -1 to
--
2.17.0
>From ae50745e9339b286bf355d1af3095461f324185a Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
Date: Sat, 12 Dec 2020 11:42:14 -0600
Subject: [PATCH 4/4] ExecReindex and ReindexParams
---
src/backend/commands/indexcmds.c | 56 +++++++++++++++++++++++++++-----
src/backend/tcop/utility.c | 40 +----------------------
src/include/commands/defrem.h | 6 +---
3 files changed, 50 insertions(+), 52 deletions(-)
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 13e463da90..8b688a5a65 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -86,6 +86,11 @@ static char *ChooseIndexName(const char *tabname, Oid namespaceId,
bool primary, bool isconstraint);
static char *ChooseIndexNameAddition(List *colnames);
static List *ChooseIndexColumnNames(List *indexElems);
+static void ReindexIndex(RangeVar *indexRelation, ReindexOptions *options,
+ bool isTopLevel);
+static Oid ReindexTable(RangeVar *relation, ReindexOptions *options, bool isTopLevel);
+static void ReindexMultipleTables(const char *objectName,
+ ReindexObjectType objectKind, ReindexOptions *options);
static void RangeVarCallbackForReindexIndex(const RangeVar *relation,
Oid relId, Oid oldRelId, void *arg);
static void reindex_error_callback(void *args);
@@ -2452,11 +2457,14 @@ ChooseIndexColumnNames(List *indexElems)
}
/*
- * ReindexParseOptions
- * Parse list of REINDEX options, returning a bitmask of ReindexOption.
+ * Reindex accordinging to stmt.
+ * This calls the intermediate routines: ReindexIndex, ReindexTable, ReindexMultipleTables,
+ * which ultimately call reindex_index, reindex_relation, ReindexRelationConcurrently.
+ * Note that partitioned relations are handled by ReindexPartitions, except that
+ * ReindexRelationConcurrently handles concurrently reindexing a table.
*/
-ReindexOptions
-ReindexParseOptions(ParseState *pstate, ReindexStmt *stmt)
+void
+ExecReindex(ParseState *pstate, ReindexStmt *stmt, bool isTopLevel)
{
ListCell *lc;
ReindexOptions options = {false};
@@ -2478,14 +2486,46 @@ ReindexParseOptions(ParseState *pstate, ReindexStmt *stmt)
parser_errposition(pstate, opt->location)));
}
- return options;
+ if (options.concurrently)
+ PreventInTransactionBlock(isTopLevel,
+ "REINDEX CONCURRENTLY");
+
+ switch (stmt->kind)
+ {
+ case REINDEX_OBJECT_INDEX:
+ ReindexIndex(stmt->relation, &options, isTopLevel);
+ break;
+ case REINDEX_OBJECT_TABLE:
+ ReindexTable(stmt->relation, &options, isTopLevel);
+ break;
+ case REINDEX_OBJECT_SCHEMA:
+ case REINDEX_OBJECT_SYSTEM:
+ case REINDEX_OBJECT_DATABASE:
+
+ /*
+ * This cannot run inside a user transaction block; if
+ * we were inside a transaction, then its commit- and
+ * start-transaction-command calls would not have the
+ * intended effect!
+ */
+ PreventInTransactionBlock(isTopLevel,
+ (stmt->kind == REINDEX_OBJECT_SCHEMA) ? "REINDEX SCHEMA" :
+ (stmt->kind == REINDEX_OBJECT_SYSTEM) ? "REINDEX SYSTEM" :
+ "REINDEX DATABASE");
+ ReindexMultipleTables(stmt->name, stmt->kind, &options);
+ break;
+ default:
+ elog(ERROR, "unrecognized object type: %d",
+ (int) stmt->kind);
+ break;
+ }
}
/*
* ReindexIndex
* Recreate a specific index.
*/
-void
+static void
ReindexIndex(RangeVar *indexRelation, ReindexOptions *options, bool isTopLevel)
{
struct ReindexIndexCallbackState state;
@@ -2607,7 +2647,7 @@ RangeVarCallbackForReindexIndex(const RangeVar *relation,
* ReindexTable
* Recreate all indexes of a table (and of its toast table, if any)
*/
-Oid
+static Oid
ReindexTable(RangeVar *relation, ReindexOptions *options, bool isTopLevel)
{
Oid heapOid;
@@ -2664,7 +2704,7 @@ ReindexTable(RangeVar *relation, ReindexOptions *options, bool isTopLevel)
* separate transaction, so we can release the lock on it right away.
* That means this must not be called within a user transaction block!
*/
-void
+static void
ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
ReindexOptions *options)
{
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index a6c42ee599..3991a834b4 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -917,45 +917,7 @@ standard_ProcessUtility(PlannedStmt *pstmt,
break;
case T_ReindexStmt:
- {
- ReindexStmt *stmt = (ReindexStmt *) parsetree;
- ReindexOptions options;
-
- options = ReindexParseOptions(pstate, stmt);
- if (options.concurrently)
- PreventInTransactionBlock(isTopLevel,
- "REINDEX CONCURRENTLY");
-
- switch (stmt->kind)
- {
- case REINDEX_OBJECT_INDEX:
- ReindexIndex(stmt->relation, &options, isTopLevel);
- break;
- case REINDEX_OBJECT_TABLE:
- ReindexTable(stmt->relation, &options, isTopLevel);
- break;
- case REINDEX_OBJECT_SCHEMA:
- case REINDEX_OBJECT_SYSTEM:
- case REINDEX_OBJECT_DATABASE:
-
- /*
- * This cannot run inside a user transaction block; if
- * we were inside a transaction, then its commit- and
- * start-transaction-command calls would not have the
- * intended effect!
- */
- PreventInTransactionBlock(isTopLevel,
- (stmt->kind == REINDEX_OBJECT_SCHEMA) ? "REINDEX SCHEMA" :
- (stmt->kind == REINDEX_OBJECT_SYSTEM) ? "REINDEX SYSTEM" :
- "REINDEX DATABASE");
- ReindexMultipleTables(stmt->name, stmt->kind, &options);
- break;
- default:
- elog(ERROR, "unrecognized object type: %d",
- (int) stmt->kind);
- break;
- }
- }
+ ExecReindex(pstate, (ReindexStmt *)parsetree, isTopLevel);
break;
/*
diff --git a/src/include/commands/defrem.h b/src/include/commands/defrem.h
index 33df5d5780..d4ea57e757 100644
--- a/src/include/commands/defrem.h
+++ b/src/include/commands/defrem.h
@@ -35,11 +35,7 @@ extern ObjectAddress DefineIndex(Oid relationId,
bool check_not_in_use,
bool skip_build,
bool quiet);
-extern ReindexOptions ReindexParseOptions(ParseState *pstate, ReindexStmt *stmt);
-extern void ReindexIndex(RangeVar *indexRelation, ReindexOptions *options, bool isTopLevel);
-extern Oid ReindexTable(RangeVar *relation, ReindexOptions *options, bool isTopLevel);
-extern void ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
- ReindexOptions *options);
+extern void ExecReindex(ParseState *pstate, ReindexStmt *stmt, bool isTopLevel);
extern char *makeObjectName(const char *name1, const char *name2,
const char *label);
extern char *ChooseRelationName(const char *name1, const char *name2,
--
2.17.0
