From 34bf8abaca50d39cb9fd00b21752f931b05a62f3 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut Date: Sat, 12 Dec 2020 09:17:55 +0100 Subject: [PATCH 1/4] Convert reindex options to struct --- src/backend/catalog/index.c | 24 ++++---- src/backend/commands/cluster.c | 3 +- src/backend/commands/indexcmds.c | 96 ++++++++++++++++---------------- src/backend/commands/tablecmds.c | 4 +- src/backend/tcop/utility.c | 10 ++-- src/include/catalog/index.h | 16 +++--- src/include/commands/defrem.h | 9 +-- 7 files changed, 83 insertions(+), 79 deletions(-) diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c index 731610c701..da2f45b796 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->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); @@ -3625,7 +3624,7 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence, if (!heapRelation) return; - if (progress) + if (options->REINDEXOPT_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->REINDEXOPT_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->REINDEXOPT_VERBOSE) ereport(INFO, (errmsg("index \"%s\" was reindexed", get_rel_name(indexId)), errdetail_internal("%s", pg_rusage_show(&ru0)))); - if (progress) + if (options->REINDEXOPT_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->REINDEXOPT_MISSING_OK) rel = try_table_open(relid, ShareLock); else rel = table_open(relid, ShareLock); @@ -3965,8 +3964,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..272723e050 100644 --- a/src/backend/commands/cluster.c +++ b/src/backend/commands/cluster.c @@ -1353,6 +1353,7 @@ finish_heap_swap(Oid OIDOldHeap, Oid OIDNewHeap, char newrelpersistence) { ObjectAddress object; + ReindexOptions reindexopts = {false}; Oid mapped_tables[4]; int reindex_flags; int i; @@ -1412,7 +1413,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..80fa39112a 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.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; @@ -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->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..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/tcop/utility.c b/src/backend/tcop/utility.c index a42ead7d69..23612b7a90 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.REINDEXOPT_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..3a8671f558 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..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, -- 2.17.0