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
This commit is contained in:
Michael Paquier 2020-09-04 10:36:35 +09:00
parent 67a472d71c
commit 844c05abc3
7 changed files with 36 additions and 27 deletions

View File

@ -97,7 +97,7 @@ static bool CompareOpclassOptions(Datum *opts1, Datum *opts2, int natts);
*/ */
struct ReindexIndexCallbackState struct ReindexIndexCallbackState
{ {
bool concurrent; /* flag from statement */ int options; /* options from statement */
Oid locked_table_oid; /* tracks previously locked table */ Oid locked_table_oid; /* tracks previously locked table */
}; };
@ -2420,7 +2420,7 @@ ChooseIndexColumnNames(List *indexElems)
* Recreate a specific index. * Recreate a specific index.
*/ */
void void
ReindexIndex(RangeVar *indexRelation, int options, bool concurrent) ReindexIndex(RangeVar *indexRelation, int options)
{ {
struct ReindexIndexCallbackState state; struct ReindexIndexCallbackState state;
Oid indOid; 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 * upgrade the lock, but that's OK, because other sessions can't hold
* locks on our temporary table. * locks on our temporary table.
*/ */
state.concurrent = concurrent; state.options = options;
state.locked_table_oid = InvalidOid; state.locked_table_oid = InvalidOid;
indOid = RangeVarGetRelidExtended(indexRelation, indOid = RangeVarGetRelidExtended(indexRelation,
concurrent ? ShareUpdateExclusiveLock : AccessExclusiveLock, (options & REINDEXOPT_CONCURRENTLY) != 0 ?
ShareUpdateExclusiveLock : AccessExclusiveLock,
0, 0,
RangeVarCallbackForReindexIndex, RangeVarCallbackForReindexIndex,
&state); &state);
@ -2460,7 +2461,8 @@ ReindexIndex(RangeVar *indexRelation, int options, bool concurrent)
persistence = irel->rd_rel->relpersistence; persistence = irel->rd_rel->relpersistence;
index_close(irel, NoLock); index_close(irel, NoLock);
if (concurrent && persistence != RELPERSISTENCE_TEMP) if ((options & REINDEXOPT_CONCURRENTLY) != 0 &&
persistence != RELPERSISTENCE_TEMP)
ReindexRelationConcurrently(indOid, options); ReindexRelationConcurrently(indOid, options);
else else
reindex_index(indOid, false, persistence, reindex_index(indOid, false, persistence,
@ -2485,7 +2487,8 @@ RangeVarCallbackForReindexIndex(const RangeVar *relation,
* non-concurrent case and table locks used by index_concurrently_*() for * non-concurrent case and table locks used by index_concurrently_*() for
* concurrent case. * 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 * 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) * Recreate all indexes of a table (and of its toast table, if any)
*/ */
Oid Oid
ReindexTable(RangeVar *relation, int options, bool concurrent) ReindexTable(RangeVar *relation, int options)
{ {
Oid heapOid; Oid heapOid;
bool result; bool result;
@ -2556,11 +2559,13 @@ ReindexTable(RangeVar *relation, int options, bool concurrent)
* locks on our temporary table. * locks on our temporary table.
*/ */
heapOid = RangeVarGetRelidExtended(relation, heapOid = RangeVarGetRelidExtended(relation,
concurrent ? ShareUpdateExclusiveLock : ShareLock, (options & REINDEXOPT_CONCURRENTLY) != 0 ?
ShareUpdateExclusiveLock : ShareLock,
0, 0,
RangeVarCallbackOwnsTable, NULL); 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); result = ReindexRelationConcurrently(heapOid, options);
@ -2594,7 +2599,7 @@ ReindexTable(RangeVar *relation, int options, bool concurrent)
*/ */
void void
ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind, ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
int options, bool concurrent) int options)
{ {
Oid objectOid; Oid objectOid;
Relation relationRelation; Relation relationRelation;
@ -2613,7 +2618,8 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
objectKind == REINDEX_OBJECT_SYSTEM || objectKind == REINDEX_OBJECT_SYSTEM ||
objectKind == REINDEX_OBJECT_DATABASE); objectKind == REINDEX_OBJECT_DATABASE);
if (objectKind == REINDEX_OBJECT_SYSTEM && concurrent) if (objectKind == REINDEX_OBJECT_SYSTEM &&
(options & REINDEXOPT_CONCURRENTLY) != 0)
ereport(ERROR, ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED), (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("cannot reindex system catalogs concurrently"))); 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 * Skip system tables, since index_create() would reject indexing them
* concurrently (and it would likely fail if we tried). * concurrently (and it would likely fail if we tried).
*/ */
if (concurrent && if ((options & REINDEXOPT_CONCURRENTLY) != 0 &&
IsCatalogRelationOid(relid)) IsCatalogRelationOid(relid))
{ {
if (!concurrent_warning) if (!concurrent_warning)
@ -2774,7 +2780,8 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
continue; continue;
} }
if (concurrent && get_rel_persistence(relid) != RELPERSISTENCE_TEMP) if ((options & REINDEXOPT_CONCURRENTLY) != 0 &&
get_rel_persistence(relid) != RELPERSISTENCE_TEMP)
{ {
(void) ReindexRelationConcurrently(relid, (void) ReindexRelationConcurrently(relid,
options | options |

View File

@ -4450,7 +4450,6 @@ _copyReindexStmt(const ReindexStmt *from)
COPY_NODE_FIELD(relation); COPY_NODE_FIELD(relation);
COPY_STRING_FIELD(name); COPY_STRING_FIELD(name);
COPY_SCALAR_FIELD(options); COPY_SCALAR_FIELD(options);
COPY_SCALAR_FIELD(concurrent);
return newnode; return newnode;
} }

View File

@ -2135,7 +2135,6 @@ _equalReindexStmt(const ReindexStmt *a, const ReindexStmt *b)
COMPARE_NODE_FIELD(relation); COMPARE_NODE_FIELD(relation);
COMPARE_STRING_FIELD(name); COMPARE_STRING_FIELD(name);
COMPARE_SCALAR_FIELD(options); COMPARE_SCALAR_FIELD(options);
COMPARE_SCALAR_FIELD(concurrent);
return true; return true;
} }

View File

@ -8177,40 +8177,44 @@ ReindexStmt:
{ {
ReindexStmt *n = makeNode(ReindexStmt); ReindexStmt *n = makeNode(ReindexStmt);
n->kind = $2; n->kind = $2;
n->concurrent = $3;
n->relation = $4; n->relation = $4;
n->name = NULL; n->name = NULL;
n->options = 0; n->options = 0;
if ($3)
n->options |= REINDEXOPT_CONCURRENTLY;
$$ = (Node *)n; $$ = (Node *)n;
} }
| REINDEX reindex_target_multitable opt_concurrently name | REINDEX reindex_target_multitable opt_concurrently name
{ {
ReindexStmt *n = makeNode(ReindexStmt); ReindexStmt *n = makeNode(ReindexStmt);
n->kind = $2; n->kind = $2;
n->concurrent = $3;
n->name = $4; n->name = $4;
n->relation = NULL; n->relation = NULL;
n->options = 0; n->options = 0;
if ($3)
n->options |= REINDEXOPT_CONCURRENTLY;
$$ = (Node *)n; $$ = (Node *)n;
} }
| REINDEX '(' reindex_option_list ')' reindex_target_type opt_concurrently qualified_name | REINDEX '(' reindex_option_list ')' reindex_target_type opt_concurrently qualified_name
{ {
ReindexStmt *n = makeNode(ReindexStmt); ReindexStmt *n = makeNode(ReindexStmt);
n->kind = $5; n->kind = $5;
n->concurrent = $6;
n->relation = $7; n->relation = $7;
n->name = NULL; n->name = NULL;
n->options = $3; n->options = $3;
if ($6)
n->options |= REINDEXOPT_CONCURRENTLY;
$$ = (Node *)n; $$ = (Node *)n;
} }
| REINDEX '(' reindex_option_list ')' reindex_target_multitable opt_concurrently name | REINDEX '(' reindex_option_list ')' reindex_target_multitable opt_concurrently name
{ {
ReindexStmt *n = makeNode(ReindexStmt); ReindexStmt *n = makeNode(ReindexStmt);
n->kind = $5; n->kind = $5;
n->concurrent = $6;
n->name = $7; n->name = $7;
n->relation = NULL; n->relation = NULL;
n->options = $3; n->options = $3;
if ($6)
n->options |= REINDEXOPT_CONCURRENTLY;
$$ = (Node *)n; $$ = (Node *)n;
} }
; ;

View File

@ -919,17 +919,17 @@ standard_ProcessUtility(PlannedStmt *pstmt,
{ {
ReindexStmt *stmt = (ReindexStmt *) parsetree; ReindexStmt *stmt = (ReindexStmt *) parsetree;
if (stmt->concurrent) if ((stmt->options & REINDEXOPT_CONCURRENTLY) != 0)
PreventInTransactionBlock(isTopLevel, PreventInTransactionBlock(isTopLevel,
"REINDEX CONCURRENTLY"); "REINDEX CONCURRENTLY");
switch (stmt->kind) switch (stmt->kind)
{ {
case REINDEX_OBJECT_INDEX: case REINDEX_OBJECT_INDEX:
ReindexIndex(stmt->relation, stmt->options, stmt->concurrent); ReindexIndex(stmt->relation, stmt->options);
break; break;
case REINDEX_OBJECT_TABLE: case REINDEX_OBJECT_TABLE:
ReindexTable(stmt->relation, stmt->options, stmt->concurrent); ReindexTable(stmt->relation, stmt->options);
break; break;
case REINDEX_OBJECT_SCHEMA: case REINDEX_OBJECT_SCHEMA:
case REINDEX_OBJECT_SYSTEM: case REINDEX_OBJECT_SYSTEM:
@ -945,7 +945,7 @@ standard_ProcessUtility(PlannedStmt *pstmt,
(stmt->kind == REINDEX_OBJECT_SCHEMA) ? "REINDEX SCHEMA" : (stmt->kind == REINDEX_OBJECT_SCHEMA) ? "REINDEX SCHEMA" :
(stmt->kind == REINDEX_OBJECT_SYSTEM) ? "REINDEX SYSTEM" : (stmt->kind == REINDEX_OBJECT_SYSTEM) ? "REINDEX SYSTEM" :
"REINDEX DATABASE"); "REINDEX DATABASE");
ReindexMultipleTables(stmt->name, stmt->kind, stmt->options, stmt->concurrent); ReindexMultipleTables(stmt->name, stmt->kind, stmt->options);
break; break;
default: default:
elog(ERROR, "unrecognized object type: %d", elog(ERROR, "unrecognized object type: %d",

View File

@ -34,10 +34,10 @@ extern ObjectAddress DefineIndex(Oid relationId,
bool check_not_in_use, bool check_not_in_use,
bool skip_build, bool skip_build,
bool quiet); bool quiet);
extern void ReindexIndex(RangeVar *indexRelation, int options, bool concurrent); extern void ReindexIndex(RangeVar *indexRelation, int options);
extern Oid ReindexTable(RangeVar *relation, int options, bool concurrent); extern Oid ReindexTable(RangeVar *relation, int options);
extern void ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind, extern void ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
int options, bool concurrent); int options);
extern char *makeObjectName(const char *name1, const char *name2, extern char *makeObjectName(const char *name1, const char *name2,
const char *label); const char *label);
extern char *ChooseRelationName(const char *name1, const char *name2, extern char *ChooseRelationName(const char *name1, const char *name2,

View File

@ -3353,6 +3353,7 @@ typedef struct ConstraintsSetStmt
#define REINDEXOPT_VERBOSE (1 << 0) /* print progress info */ #define REINDEXOPT_VERBOSE (1 << 0) /* print progress info */
#define REINDEXOPT_REPORT_PROGRESS (1 << 1) /* report pgstat progress */ #define REINDEXOPT_REPORT_PROGRESS (1 << 1) /* report pgstat progress */
#define REINDEXOPT_MISSING_OK (1 << 2) /* skip missing relations */ #define REINDEXOPT_MISSING_OK (1 << 2) /* skip missing relations */
#define REINDEXOPT_CONCURRENTLY (1 << 3) /* concurrent mode */
typedef enum ReindexObjectType typedef enum ReindexObjectType
{ {
@ -3371,7 +3372,6 @@ typedef struct ReindexStmt
RangeVar *relation; /* Table or index to reindex */ RangeVar *relation; /* Table or index to reindex */
const char *name; /* name of database to reindex */ const char *name; /* name of database to reindex */
int options; /* Reindex options flags */ int options; /* Reindex options flags */
bool concurrent; /* reindex concurrently? */
} ReindexStmt; } ReindexStmt;
/* ---------------------- /* ----------------------