From d3125b3bfb8a3dc23e38f38bcf850ca6fc36f492 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut Date: Sat, 12 Dec 2020 09:17:55 +0100 Subject: [PATCH] Convert reindex options to struct --- src/backend/catalog/index.c | 19 ++++--- src/backend/commands/cluster.c | 2 +- src/backend/commands/indexcmds.c | 94 ++++++++++++++++---------------- src/backend/commands/tablecmds.c | 2 +- src/backend/tcop/utility.c | 4 +- src/include/catalog/index.h | 16 +++--- src/include/commands/defrem.h | 9 +-- 7 files changed, 74 insertions(+), 72 deletions(-) diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c index 731610c701..06342fddf1 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,7 @@ 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; + bool progress = options.REINDEXOPT_REPORT_PROGRESS; pg_rusage_init(&ru0); @@ -3611,12 +3611,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.REINDEXOPT_MISSING_OK); /* if relation is missing, leave */ if (!OidIsValid(heapId)) return; - if ((options & REINDEXOPT_MISSING_OK) != 0) + if (options.REINDEXOPT_MISSING_OK) heapRelation = try_table_open(heapId, ShareLock); else heapRelation = table_open(heapId, ShareLock); @@ -3792,7 +3792,7 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence, } /* Log what we did */ - if (options & REINDEXOPT_VERBOSE) + if (options.REINDEXOPT_VERBOSE) ereport(INFO, (errmsg("index \"%s\" was reindexed", get_rel_name(indexId)), @@ -3846,7 +3846,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 +3861,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.REINDEXOPT_MISSING_OK) rel = try_table_open(relid, ShareLock); else rel = table_open(relid, ShareLock); @@ -3965,8 +3965,9 @@ reindex_relation(Oid relid, int flags, int options) * Note that this should fail if the toast relation is missing, so * reset REINDEXOPT_MISSING_OK. */ - result |= reindex_relation(toast_relid, flags, - options & ~(REINDEXOPT_MISSING_OK)); + ReindexOptions newoptions = options; + newoptions.REINDEXOPT_MISSING_OK = false; + result |= reindex_relation(toast_relid, flags, newoptions); } return result; diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c index fd5a6eec86..b0aa3536d1 100644 --- a/src/backend/commands/cluster.c +++ b/src/backend/commands/cluster.c @@ -1412,7 +1412,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, (ReindexOptions){}); /* 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..cd8eaa732d 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 = {}; /* 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.REINDEXOPT_VERBOSE = defGetBoolean(opt); else if (strcmp(opt->defname, "concurrently") == 0) - concurrently = defGetBoolean(opt); + options.REINDEXOPT_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; @@ -2512,7 +2506,7 @@ ReindexIndex(RangeVar *indexRelation, int options, bool isTopLevel) state.options = options; state.locked_table_oid = InvalidOid; indOid = RangeVarGetRelidExtended(indexRelation, - (options & REINDEXOPT_CONCURRENTLY) != 0 ? + options.REINDEXOPT_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.REINDEXOPT_CONCURRENTLY && persistence != RELPERSISTENCE_TEMP) ReindexRelationConcurrently(indOid, options); else - reindex_index(indOid, false, persistence, - options | REINDEXOPT_REPORT_PROGRESS); + { + ReindexOptions newoptions = options; + newoptions.REINDEXOPT_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.REINDEXOPT_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.REINDEXOPT_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.REINDEXOPT_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.REINDEXOPT_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.REINDEXOPT_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.REINDEXOPT_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.REINDEXOPT_CONCURRENTLY && relpersistence != RELPERSISTENCE_TEMP) { - (void) ReindexRelationConcurrently(relid, - options | - REINDEXOPT_MISSING_OK); + ReindexOptions newoptions = options; + newoptions.REINDEXOPT_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.REINDEXOPT_REPORT_PROGRESS = true; + newoptions.REINDEXOPT_MISSING_OK = true; + reindex_index(relid, false, relpersistence, newoptions); PopActiveSnapshot(); /* reindex_index() does the verbose output */ } else { bool result; + ReindexOptions newoptions = options; + newoptions.REINDEXOPT_REPORT_PROGRESS = true; + newoptions.REINDEXOPT_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.REINDEXOPT_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.REINDEXOPT_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.REINDEXOPT_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.REINDEXOPT_MISSING_OK); Relation heapRelation; /* if relation is missing, leave */ @@ -3262,7 +3262,7 @@ ReindexRelationConcurrently(Oid relationOid, int options) * to rebuild is not complete yet, and REINDEXOPT_MISSING_OK * should not be used once all the session locks are taken. */ - if ((options & REINDEXOPT_MISSING_OK) != 0) + if (options.REINDEXOPT_MISSING_OK) { heapRelation = try_table_open(heapId, ShareUpdateExclusiveLock); @@ -3754,7 +3754,7 @@ ReindexRelationConcurrently(Oid relationOid, int options) StartTransactionCommand(); /* Log what we did */ - if (options & REINDEXOPT_VERBOSE) + if (options.REINDEXOPT_VERBOSE) { if (relkind == RELKIND_INDEX) ereport(INFO, diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 1fa9f19f08..9f218ac0e4 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -1891,7 +1891,7 @@ 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, (ReindexOptions){}); } pgstat_count_truncate(rel); diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c index a42ead7d69..e5a4e8f662 100644 --- a/src/backend/tcop/utility.c +++ b/src/backend/tcop/utility.c @@ -919,10 +919,10 @@ 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.REINDEXOPT_CONCURRENTLY) PreventInTransactionBlock(isTopLevel, "REINDEX CONCURRENTLY"); diff --git a/src/include/catalog/index.h b/src/include/catalog/index.h index c041628049..81e3de4d22 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 REINDEXOPT_VERBOSE; /* print progress info */ + bool REINDEXOPT_REPORT_PROGRESS; /* report pgstat progress */ + bool REINDEXOPT_MISSING_OK; /* skip missing relations */ + bool REINDEXOPT_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/defrem.h b/src/include/commands/defrem.h index 1133ae1143..5b1a60d5fa 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, -- 2.29.2