From f68b164e849d107e21660eff956145147a385d1a Mon Sep 17 00:00:00 2001 From: Justin Pryzby Date: Tue, 11 Aug 2020 01:41:09 -0500 Subject: [PATCH v22 2/5] Deprecate ReindexStmt->concurrent.. ..since parenthesized syntax is the modern syntax, remove the special field for handling nonparenthesized option. --- src/backend/commands/indexcmds.c | 20 +++++++++++--------- src/backend/nodes/copyfuncs.c | 1 - src/backend/nodes/equalfuncs.c | 1 - src/backend/parser/gram.y | 16 ++++++++++++---- src/backend/tcop/utility.c | 6 +++--- src/include/nodes/parsenodes.h | 2 +- 6 files changed, 27 insertions(+), 19 deletions(-) diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index a22b32fc74..097d8720a4 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 */ + bool concurrent; /* flag from statement XXX */ Oid locked_table_oid; /* tracks previously locked table */ }; @@ -2438,10 +2438,10 @@ ReindexIndex(ReindexStmt *stmt) * upgrade the lock, but that's OK, because other sessions can't hold * locks on our temporary table. */ - state.concurrent = stmt->concurrent; + state.concurrent = (stmt->options & REINDEXOPT_CONCURRENT); state.locked_table_oid = InvalidOid; indOid = RangeVarGetRelidExtended(indexRelation, - stmt->concurrent ? ShareUpdateExclusiveLock : AccessExclusiveLock, + state.concurrent ? ShareUpdateExclusiveLock : AccessExclusiveLock, 0, RangeVarCallbackForReindexIndex, &state); @@ -2461,7 +2461,7 @@ ReindexIndex(ReindexStmt *stmt) persistence = irel->rd_rel->relpersistence; index_close(irel, NoLock); - if (stmt->concurrent && persistence != RELPERSISTENCE_TEMP) + if (state.concurrent && persistence != RELPERSISTENCE_TEMP) ReindexRelationConcurrently(indOid, stmt->options); else reindex_index(indOid, false, persistence, @@ -2548,6 +2548,7 @@ ReindexTable(ReindexStmt *stmt) RangeVar *relation = stmt->relation; Oid heapOid; bool result; + bool concurrent = (stmt->options & REINDEXOPT_CONCURRENT); /* * The lock level used here should match reindex_relation(). @@ -2558,11 +2559,11 @@ ReindexTable(ReindexStmt *stmt) * locks on our temporary table. */ heapOid = RangeVarGetRelidExtended(relation, - stmt->concurrent ? ShareUpdateExclusiveLock : ShareLock, + concurrent ? ShareUpdateExclusiveLock : ShareLock, 0, RangeVarCallbackOwnsTable, NULL); - if (stmt->concurrent && get_rel_persistence(heapOid) != RELPERSISTENCE_TEMP) + if (concurrent && get_rel_persistence(heapOid) != RELPERSISTENCE_TEMP) { result = ReindexRelationConcurrently(heapOid, stmt->options); @@ -2610,13 +2611,14 @@ ReindexMultipleTables(ReindexStmt *stmt) ListCell *l; int num_keys; bool concurrent_warning = false; + bool concurrent = (stmt->options & REINDEXOPT_CONCURRENT); AssertArg(objectName); Assert(objectKind == REINDEX_OBJECT_SCHEMA || objectKind == REINDEX_OBJECT_SYSTEM || objectKind == REINDEX_OBJECT_DATABASE); - if (objectKind == REINDEX_OBJECT_SYSTEM && stmt->concurrent) + if (objectKind == REINDEX_OBJECT_SYSTEM && concurrent) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("cannot reindex system catalogs concurrently"))); @@ -2727,7 +2729,7 @@ ReindexMultipleTables(ReindexStmt *stmt) * Skip system tables, since index_create() would reject indexing them * concurrently (and it would likely fail if we tried). */ - if (stmt->concurrent && + if (concurrent && IsCatalogRelationOid(relid)) { if (!concurrent_warning) @@ -2769,7 +2771,7 @@ ReindexMultipleTables(ReindexStmt *stmt) /* functions in indexes may want a snapshot set */ PushActiveSnapshot(GetTransactionSnapshot()); - if (stmt->concurrent && get_rel_persistence(relid) != RELPERSISTENCE_TEMP) + if (concurrent && get_rel_persistence(relid) != RELPERSISTENCE_TEMP) { (void) ReindexRelationConcurrently(relid, stmt->options); /* ReindexRelationConcurrently() does the verbose output */ diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c index 71548acc0c..a67e637c37 100644 --- a/src/backend/nodes/copyfuncs.c +++ b/src/backend/nodes/copyfuncs.c @@ -4452,7 +4452,6 @@ _copyReindexStmt(const ReindexStmt *from) COPY_STRING_FIELD(name); COPY_SCALAR_FIELD(options); COPY_NODE_FIELD(params); - COPY_SCALAR_FIELD(concurrent); return newnode; } diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c index de48a42cdd..efcb23c12e 100644 --- a/src/backend/nodes/equalfuncs.c +++ b/src/backend/nodes/equalfuncs.c @@ -2137,7 +2137,6 @@ _equalReindexStmt(const ReindexStmt *a, const ReindexStmt *b) COMPARE_STRING_FIELD(name); COMPARE_SCALAR_FIELD(options); COMPARE_NODE_FIELD(params); - COMPARE_SCALAR_FIELD(concurrent); return true; } diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index b909b161a6..ac489c03c3 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -8176,40 +8176,48 @@ ReindexStmt: { ReindexStmt *n = makeNode(ReindexStmt); n->kind = $2; - n->concurrent = $3; n->relation = $4; n->name = NULL; n->params = NIL; + if ($3) + n->params = lappend(n->params, + makeDefElem("concurrently", (Node *)makeString("concurrently"), @3)); $$ = (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->params = NIL; + if ($3) + n->params = lappend(n->params, + makeDefElem("concurrently", (Node *)makeString("concurrently"), @3)); $$ = (Node *)n; } | REINDEX '(' vac_analyze_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->params = $3; + if ($6) + n->params = lappend(n->params, + makeDefElem("concurrently", (Node *)makeString("concurrently"), @6)); $$ = (Node *)n; } | REINDEX '(' vac_analyze_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->params = $3; + if ($6) + n->params = lappend(n->params, + makeDefElem("concurrently", (Node *)makeString("concurrently"), @6)); $$ = (Node *)n; } ; diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c index c48983a5de..639b6fce74 100644 --- a/src/backend/tcop/utility.c +++ b/src/backend/tcop/utility.c @@ -543,7 +543,7 @@ void parse_reindex_params(ParseState *pstate, ReindexStmt *stmt) stmt->options &= ~REINDEXOPT_VERBOSE; } else if (strcmp(opt->defname, "concurrently") == 0) - stmt->concurrent = true; + stmt->options |= REINDEXOPT_CONCURRENT; else ereport(ERROR, (errcode(ERRCODE_SYNTAX_ERROR), @@ -946,11 +946,11 @@ standard_ProcessUtility(PlannedStmt *pstmt, { ReindexStmt *stmt = (ReindexStmt *) parsetree; - if (stmt->concurrent) + parse_reindex_params(pstate, stmt); + if (stmt->options & REINDEXOPT_CONCURRENT) PreventInTransactionBlock(isTopLevel, "REINDEX CONCURRENTLY"); - parse_reindex_params(pstate, stmt); switch (stmt->kind) { case REINDEX_OBJECT_INDEX: diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index 295c2802a4..ba685139ef 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -3352,6 +3352,7 @@ typedef struct ConstraintsSetStmt /* Reindex options */ #define REINDEXOPT_VERBOSE (1 << 0) /* print progress info */ #define REINDEXOPT_REPORT_PROGRESS (1 << 1) /* report pgstat progress */ +#define REINDEXOPT_CONCURRENT (1 << 2) /* reindex CONCURRENTLY */ typedef enum ReindexObjectType { @@ -3371,7 +3372,6 @@ typedef struct ReindexStmt const char *name; /* name of database to reindex */ int options; /* Reindex options flags */ List *params; /* Params not further parsed by the grammer */ - bool concurrent; /* reindex concurrently? */ } ReindexStmt; /* ---------------------- -- 2.17.0