From 844c05abc3f1c1703bf17cf44ab66351ed9711d2 Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Fri, 4 Sep 2020 10:36:35 +0900 Subject: [PATCH] Remove variable "concurrent" from ReindexStmt This node already handles multiple options using a bitmask, so having a separate boolean flag is not necessary. This simplifies the code a bit with less arguments to give to the reindex routines, by replacing the boolean with an equivalent bitmask value. Reviewed-by: Julien Rouhaud Discussion: https://postgr.es/m/20200902110326.GA14963@paquier.xyz --- src/backend/commands/indexcmds.c | 33 +++++++++++++++++++------------- src/backend/nodes/copyfuncs.c | 1 - src/backend/nodes/equalfuncs.c | 1 - src/backend/parser/gram.y | 12 ++++++++---- src/backend/tcop/utility.c | 8 ++++---- src/include/commands/defrem.h | 6 +++--- src/include/nodes/parsenodes.h | 2 +- 7 files changed, 36 insertions(+), 27 deletions(-) diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index b3a92381f9..430e88b4c9 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -97,7 +97,7 @@ static bool CompareOpclassOptions(Datum *opts1, Datum *opts2, int natts); */ struct ReindexIndexCallbackState { - bool concurrent; /* flag from statement */ + int options; /* options from statement */ Oid locked_table_oid; /* tracks previously locked table */ }; @@ -2420,7 +2420,7 @@ ChooseIndexColumnNames(List *indexElems) * Recreate a specific index. */ void -ReindexIndex(RangeVar *indexRelation, int options, bool concurrent) +ReindexIndex(RangeVar *indexRelation, int options) { struct ReindexIndexCallbackState state; Oid indOid; @@ -2437,10 +2437,11 @@ ReindexIndex(RangeVar *indexRelation, int options, bool concurrent) * upgrade the lock, but that's OK, because other sessions can't hold * locks on our temporary table. */ - state.concurrent = concurrent; + state.options = options; state.locked_table_oid = InvalidOid; indOid = RangeVarGetRelidExtended(indexRelation, - concurrent ? ShareUpdateExclusiveLock : AccessExclusiveLock, + (options & REINDEXOPT_CONCURRENTLY) != 0 ? + ShareUpdateExclusiveLock : AccessExclusiveLock, 0, RangeVarCallbackForReindexIndex, &state); @@ -2460,7 +2461,8 @@ ReindexIndex(RangeVar *indexRelation, int options, bool concurrent) persistence = irel->rd_rel->relpersistence; index_close(irel, NoLock); - if (concurrent && persistence != RELPERSISTENCE_TEMP) + if ((options & REINDEXOPT_CONCURRENTLY) != 0 && + persistence != RELPERSISTENCE_TEMP) ReindexRelationConcurrently(indOid, options); else reindex_index(indOid, false, persistence, @@ -2485,7 +2487,8 @@ RangeVarCallbackForReindexIndex(const RangeVar *relation, * non-concurrent case and table locks used by index_concurrently_*() for * concurrent case. */ - table_lockmode = state->concurrent ? ShareUpdateExclusiveLock : ShareLock; + table_lockmode = ((state->options & REINDEXOPT_CONCURRENTLY) != 0) ? + ShareUpdateExclusiveLock : ShareLock; /* * If we previously locked some other index's heap, and the name we're @@ -2542,7 +2545,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 concurrent) +ReindexTable(RangeVar *relation, int options) { Oid heapOid; bool result; @@ -2556,11 +2559,13 @@ ReindexTable(RangeVar *relation, int options, bool concurrent) * locks on our temporary table. */ heapOid = RangeVarGetRelidExtended(relation, - concurrent ? ShareUpdateExclusiveLock : ShareLock, + (options & REINDEXOPT_CONCURRENTLY) != 0 ? + ShareUpdateExclusiveLock : ShareLock, 0, RangeVarCallbackOwnsTable, NULL); - if (concurrent && get_rel_persistence(heapOid) != RELPERSISTENCE_TEMP) + if ((options & REINDEXOPT_CONCURRENTLY) != 0 && + get_rel_persistence(heapOid) != RELPERSISTENCE_TEMP) { result = ReindexRelationConcurrently(heapOid, options); @@ -2594,7 +2599,7 @@ ReindexTable(RangeVar *relation, int options, bool concurrent) */ void ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind, - int options, bool concurrent) + int options) { Oid objectOid; Relation relationRelation; @@ -2613,7 +2618,8 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind, objectKind == REINDEX_OBJECT_SYSTEM || objectKind == REINDEX_OBJECT_DATABASE); - if (objectKind == REINDEX_OBJECT_SYSTEM && concurrent) + if (objectKind == REINDEX_OBJECT_SYSTEM && + (options & REINDEXOPT_CONCURRENTLY) != 0) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("cannot reindex system catalogs concurrently"))); @@ -2724,7 +2730,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 (concurrent && + if ((options & REINDEXOPT_CONCURRENTLY) != 0 && IsCatalogRelationOid(relid)) { if (!concurrent_warning) @@ -2774,7 +2780,8 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind, continue; } - if (concurrent && get_rel_persistence(relid) != RELPERSISTENCE_TEMP) + if ((options & REINDEXOPT_CONCURRENTLY) != 0 && + get_rel_persistence(relid) != RELPERSISTENCE_TEMP) { (void) ReindexRelationConcurrently(relid, options | diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c index 89c409de66..0409a40b82 100644 --- a/src/backend/nodes/copyfuncs.c +++ b/src/backend/nodes/copyfuncs.c @@ -4450,7 +4450,6 @@ _copyReindexStmt(const ReindexStmt *from) COPY_NODE_FIELD(relation); COPY_STRING_FIELD(name); COPY_SCALAR_FIELD(options); - COPY_SCALAR_FIELD(concurrent); return newnode; } diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c index e3f33c40be..e2d1b987bf 100644 --- a/src/backend/nodes/equalfuncs.c +++ b/src/backend/nodes/equalfuncs.c @@ -2135,7 +2135,6 @@ _equalReindexStmt(const ReindexStmt *a, const ReindexStmt *b) COMPARE_NODE_FIELD(relation); COMPARE_STRING_FIELD(name); COMPARE_SCALAR_FIELD(options); - COMPARE_SCALAR_FIELD(concurrent); return true; } diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index dbb47d4982..c5154b818c 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -8177,40 +8177,44 @@ ReindexStmt: { ReindexStmt *n = makeNode(ReindexStmt); n->kind = $2; - n->concurrent = $3; n->relation = $4; n->name = NULL; n->options = 0; + if ($3) + n->options |= REINDEXOPT_CONCURRENTLY; $$ = (Node *)n; } | REINDEX reindex_target_multitable opt_concurrently name { ReindexStmt *n = makeNode(ReindexStmt); n->kind = $2; - n->concurrent = $3; n->name = $4; n->relation = NULL; n->options = 0; + if ($3) + n->options |= REINDEXOPT_CONCURRENTLY; $$ = (Node *)n; } | REINDEX '(' reindex_option_list ')' reindex_target_type opt_concurrently qualified_name { ReindexStmt *n = makeNode(ReindexStmt); n->kind = $5; - n->concurrent = $6; n->relation = $7; n->name = NULL; n->options = $3; + if ($6) + n->options |= REINDEXOPT_CONCURRENTLY; $$ = (Node *)n; } | REINDEX '(' reindex_option_list ')' reindex_target_multitable opt_concurrently name { ReindexStmt *n = makeNode(ReindexStmt); n->kind = $5; - n->concurrent = $6; n->name = $7; n->relation = NULL; n->options = $3; + if ($6) + n->options |= REINDEXOPT_CONCURRENTLY; $$ = (Node *)n; } ; diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c index 6154d2c8c6..b4cde5565e 100644 --- a/src/backend/tcop/utility.c +++ b/src/backend/tcop/utility.c @@ -919,17 +919,17 @@ standard_ProcessUtility(PlannedStmt *pstmt, { ReindexStmt *stmt = (ReindexStmt *) parsetree; - if (stmt->concurrent) + if ((stmt->options & REINDEXOPT_CONCURRENTLY) != 0) PreventInTransactionBlock(isTopLevel, "REINDEX CONCURRENTLY"); switch (stmt->kind) { case REINDEX_OBJECT_INDEX: - ReindexIndex(stmt->relation, stmt->options, stmt->concurrent); + ReindexIndex(stmt->relation, stmt->options); break; case REINDEX_OBJECT_TABLE: - ReindexTable(stmt->relation, stmt->options, stmt->concurrent); + ReindexTable(stmt->relation, stmt->options); break; case REINDEX_OBJECT_SCHEMA: case REINDEX_OBJECT_SYSTEM: @@ -945,7 +945,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, stmt->options, stmt->concurrent); + ReindexMultipleTables(stmt->name, stmt->kind, stmt->options); break; default: elog(ERROR, "unrecognized object type: %d", diff --git a/src/include/commands/defrem.h b/src/include/commands/defrem.h index c26a102b17..3129b684f6 100644 --- a/src/include/commands/defrem.h +++ b/src/include/commands/defrem.h @@ -34,10 +34,10 @@ extern ObjectAddress DefineIndex(Oid relationId, bool check_not_in_use, bool skip_build, bool quiet); -extern void ReindexIndex(RangeVar *indexRelation, int options, bool concurrent); -extern Oid ReindexTable(RangeVar *relation, int options, bool concurrent); +extern void ReindexIndex(RangeVar *indexRelation, int options); +extern Oid ReindexTable(RangeVar *relation, int options); extern void ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind, - int options, bool concurrent); + int 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/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index d52c563305..e83329fd6d 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -3353,6 +3353,7 @@ typedef struct ConstraintsSetStmt #define REINDEXOPT_VERBOSE (1 << 0) /* print progress info */ #define REINDEXOPT_REPORT_PROGRESS (1 << 1) /* report pgstat progress */ #define REINDEXOPT_MISSING_OK (1 << 2) /* skip missing relations */ +#define REINDEXOPT_CONCURRENTLY (1 << 3) /* concurrent mode */ typedef enum ReindexObjectType { @@ -3371,7 +3372,6 @@ typedef struct ReindexStmt RangeVar *relation; /* Table or index to reindex */ const char *name; /* name of database to reindex */ int options; /* Reindex options flags */ - bool concurrent; /* reindex concurrently? */ } ReindexStmt; /* ----------------------