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

diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index b3a92381f95f1684e877db42c167692e24d097f6..430e88b4c9fa569e8793080ab84dea8efb6e5786 100644 (file)
--- 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 89c409de6640a90ab6794336c6c191ac4bd10c0f..0409a40b82a839b4245c256718c483d7e699c7fc 100644 (file)
--- 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 e3f33c40be53c2a6b00bed109da793a8c3a43d33..e2d1b987bf490b5399bc043a4f52feb65e511a89 100644 (file)
--- 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 dbb47d498290b394b7f73dad44cbb1a17868c112..c5154b818cfb1badd1481ff7771a130703b3b67b 100644 (file)
--- 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 6154d2c8c63b8a60734b6fb23a9d9913c55ebc9e..b4cde5565ec17dcc7d0e2d428067c19529641493 100644 (file)
--- 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 c26a102b175b0e157002558daeb07ad6c1cd51b9..3129b684f633ea6725b7f4d38059c1a760a7ecb1 100644 (file)
--- 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 d52c563305ae963bdbe23bb4baba2acc14a9cc86..e83329fd6d10aa537a81b5521a7c4db0da999e2c 100644 (file)
--- 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;
 
 /* ----------------------