From cbb8e856e0913f1a94d28d7164adabad1362d58c Mon Sep 17 00:00:00 2001 From: Justin Pryzby Date: Fri, 27 Mar 2020 17:50:46 -0500 Subject: [PATCH v16 1/7] Change REINDEX/CLUSTER to accept an option list.. ..like reindex (CONCURRENTLY) Change docs in the style of VACUUM. See also: 52dcfda48778d16683c64ca4372299a099a15b96 --- doc/src/sgml/ref/cluster.sgml | 13 ++++---- doc/src/sgml/ref/reindex.sgml | 26 +++++++-------- src/backend/commands/cluster.c | 21 ++++++++++++- src/backend/commands/indexcmds.c | 41 +++++++++++++----------- src/backend/nodes/copyfuncs.c | 2 ++ src/backend/nodes/equalfuncs.c | 2 ++ src/backend/parser/gram.y | 54 +++++++++++++++++++++++++++----- src/backend/tcop/utility.c | 33 ++++++++++++++++--- src/include/commands/cluster.h | 3 +- src/include/commands/defrem.h | 7 ++--- src/include/nodes/parsenodes.h | 4 ++- 11 files changed, 149 insertions(+), 57 deletions(-) diff --git a/doc/src/sgml/ref/cluster.sgml b/doc/src/sgml/ref/cluster.sgml index 4da60d8d56..bd0682ddfd 100644 --- a/doc/src/sgml/ref/cluster.sgml +++ b/doc/src/sgml/ref/cluster.sgml @@ -82,31 +82,32 @@ CLUSTER [VERBOSE] - table_name + VERBOSE - The name (possibly schema-qualified) of a table. + Prints a progress report as each table is clustered. - index_name + table_name - The name of an index. + The name (possibly schema-qualified) of a table. - VERBOSE + index_name - Prints a progress report as each table is clustered. + The name of an index. + diff --git a/doc/src/sgml/ref/reindex.sgml b/doc/src/sgml/ref/reindex.sgml index c54a7c420d..200526b6f4 100644 --- a/doc/src/sgml/ref/reindex.sgml +++ b/doc/src/sgml/ref/reindex.sgml @@ -141,19 +141,6 @@ REINDEX [ ( option [, ...] ) ] { IN - - name - - - The name of the specific index, table, or database to be - reindexed. Index and table names can be schema-qualified. - Presently, REINDEX DATABASE and REINDEX SYSTEM - can only reindex the current database, so their parameter must match - the current database's name. - - - - CONCURRENTLY @@ -182,6 +169,19 @@ REINDEX [ ( option [, ...] ) ] { IN + + + name + + + The name of the specific index, table, or database to be + reindexed. Index and table names can be schema-qualified. + Presently, REINDEX DATABASE and REINDEX SYSTEM + can only reindex the current database, so their parameter must match + the current database's name. + + + diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c index fc1cea0236..6c3fdd3874 100644 --- a/src/backend/commands/cluster.c +++ b/src/backend/commands/cluster.c @@ -35,6 +35,7 @@ #include "catalog/pg_am.h" #include "catalog/toasting.h" #include "commands/cluster.h" +#include "commands/defrem.h" #include "commands/progress.h" #include "commands/tablecmds.h" #include "commands/vacuum.h" @@ -99,8 +100,26 @@ static List *get_tables_to_cluster(MemoryContext cluster_context); *--------------------------------------------------------------------------- */ void -cluster(ClusterStmt *stmt, bool isTopLevel) +cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel) { + ListCell *lc; + + /* Parse list of generic parameter not handled by the parser */ + foreach(lc, stmt->params) + { + DefElem *opt = (DefElem *) lfirst(lc); + + if (strcmp(opt->defname, "verbose") == 0) + stmt->options |= CLUOPT_VERBOSE; + // XXX: handle boolean opt: VERBOSE off + else + ereport(ERROR, + (errcode(ERRCODE_SYNTAX_ERROR), + errmsg("unrecognized CLUSTER option \"%s\"", + opt->defname), + parser_errposition(pstate, opt->location))); + } + if (stmt->relation != NULL) { /* This is the single-relation case. */ diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index 2e5997b5c3..e0f87f2dbf 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -2366,8 +2366,9 @@ ChooseIndexColumnNames(List *indexElems) * Recreate a specific index. */ void -ReindexIndex(RangeVar *indexRelation, int options, bool concurrent) +ReindexIndex(ReindexStmt *stmt) { + RangeVar *indexRelation = stmt->relation; struct ReindexIndexCallbackState state; Oid indOid; Relation irel; @@ -2383,10 +2384,10 @@ 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.concurrent = stmt->concurrent; state.locked_table_oid = InvalidOid; indOid = RangeVarGetRelidExtended(indexRelation, - concurrent ? ShareUpdateExclusiveLock : AccessExclusiveLock, + stmt->concurrent ? ShareUpdateExclusiveLock : AccessExclusiveLock, 0, RangeVarCallbackForReindexIndex, &state); @@ -2406,11 +2407,11 @@ ReindexIndex(RangeVar *indexRelation, int options, bool concurrent) persistence = irel->rd_rel->relpersistence; index_close(irel, NoLock); - if (concurrent && persistence != RELPERSISTENCE_TEMP) - ReindexRelationConcurrently(indOid, options); + if (stmt->concurrent && persistence != RELPERSISTENCE_TEMP) + ReindexRelationConcurrently(indOid, stmt->options); else reindex_index(indOid, false, persistence, - options | REINDEXOPT_REPORT_PROGRESS); + stmt->options | REINDEXOPT_REPORT_PROGRESS); } /* @@ -2488,8 +2489,9 @@ 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(ReindexStmt *stmt) { + RangeVar *relation = stmt->relation; Oid heapOid; bool result; @@ -2502,13 +2504,13 @@ ReindexTable(RangeVar *relation, int options, bool concurrent) * locks on our temporary table. */ heapOid = RangeVarGetRelidExtended(relation, - concurrent ? ShareUpdateExclusiveLock : ShareLock, + stmt->concurrent ? ShareUpdateExclusiveLock : ShareLock, 0, RangeVarCallbackOwnsTable, NULL); - if (concurrent && get_rel_persistence(heapOid) != RELPERSISTENCE_TEMP) + if (stmt->concurrent && get_rel_persistence(heapOid) != RELPERSISTENCE_TEMP) { - result = ReindexRelationConcurrently(heapOid, options); + result = ReindexRelationConcurrently(heapOid, stmt->options); if (!result) ereport(NOTICE, @@ -2520,7 +2522,7 @@ ReindexTable(RangeVar *relation, int options, bool concurrent) result = reindex_relation(heapOid, REINDEX_REL_PROCESS_TOAST | REINDEX_REL_CHECK_CONSTRAINTS, - options | REINDEXOPT_REPORT_PROGRESS); + stmt->options | REINDEXOPT_REPORT_PROGRESS); if (!result) ereport(NOTICE, (errmsg("table \"%s\" has no indexes to reindex", @@ -2539,9 +2541,10 @@ ReindexTable(RangeVar *relation, int options, bool concurrent) * That means this must not be called within a user transaction block! */ void -ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind, - int options, bool concurrent) +ReindexMultipleTables(ReindexStmt *stmt) { + const char *objectName = stmt->name; + ReindexObjectType objectKind = stmt->kind; Oid objectOid; Relation relationRelation; TableScanDesc scan; @@ -2559,7 +2562,7 @@ 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 && stmt->concurrent) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("cannot reindex system catalogs concurrently"))); @@ -2670,7 +2673,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 (stmt->concurrent && IsCatalogRelationOid(relid)) { if (!concurrent_warning) @@ -2712,9 +2715,9 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind, /* functions in indexes may want a snapshot set */ PushActiveSnapshot(GetTransactionSnapshot()); - if (concurrent && get_rel_persistence(relid) != RELPERSISTENCE_TEMP) + if (stmt->concurrent && get_rel_persistence(relid) != RELPERSISTENCE_TEMP) { - (void) ReindexRelationConcurrently(relid, options); + (void) ReindexRelationConcurrently(relid, stmt->options); /* ReindexRelationConcurrently() does the verbose output */ } else @@ -2724,9 +2727,9 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind, result = reindex_relation(relid, REINDEX_REL_PROCESS_TOAST | REINDEX_REL_CHECK_CONSTRAINTS, - options | REINDEXOPT_REPORT_PROGRESS); + stmt->options | REINDEXOPT_REPORT_PROGRESS); - if (result && (options & REINDEXOPT_VERBOSE)) + if (result && (stmt->options & REINDEXOPT_VERBOSE)) ereport(INFO, (errmsg("table \"%s.%s\" was reindexed", get_namespace_name(get_rel_namespace(relid)), diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c index c9a90d1191..a4672f1bb8 100644 --- a/src/backend/nodes/copyfuncs.c +++ b/src/backend/nodes/copyfuncs.c @@ -3317,6 +3317,7 @@ _copyClusterStmt(const ClusterStmt *from) COPY_NODE_FIELD(relation); COPY_STRING_FIELD(indexname); COPY_SCALAR_FIELD(options); + COPY_NODE_FIELD(params); return newnode; } @@ -4405,6 +4406,7 @@ _copyReindexStmt(const ReindexStmt *from) COPY_SCALAR_FIELD(kind); COPY_NODE_FIELD(relation); COPY_STRING_FIELD(name); + COPY_NODE_FIELD(rawoptions); COPY_SCALAR_FIELD(options); COPY_SCALAR_FIELD(concurrent); diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c index d05ca26fcf..cb22426dbd 100644 --- a/src/backend/nodes/equalfuncs.c +++ b/src/backend/nodes/equalfuncs.c @@ -1215,6 +1215,7 @@ _equalClusterStmt(const ClusterStmt *a, const ClusterStmt *b) COMPARE_NODE_FIELD(relation); COMPARE_STRING_FIELD(indexname); COMPARE_SCALAR_FIELD(options); + COMPARE_NODE_FIELD(params); return true; } @@ -2129,6 +2130,7 @@ _equalReindexStmt(const ReindexStmt *a, const ReindexStmt *b) COMPARE_SCALAR_FIELD(kind); COMPARE_NODE_FIELD(relation); COMPARE_STRING_FIELD(name); + COMPARE_NODE_FIELD(rawoptions); COMPARE_SCALAR_FIELD(options); COMPARE_SCALAR_FIELD(concurrent); diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index eb0bf12cd8..f1ec2b4951 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -511,7 +511,10 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query); %type explain_option_list %type reindex_target_type reindex_target_multitable -%type reindex_option_list reindex_option_elem +%type reindex_option_name +%type reindex_option_arg +%type reindex_option_list +%type reindex_option_elem %type copy_generic_opt_arg copy_generic_opt_arg_list_item %type copy_generic_opt_elem @@ -8406,7 +8409,7 @@ ReindexStmt: n->concurrent = $3; n->relation = $4; n->name = NULL; - n->options = 0; + n->rawoptions = NIL; $$ = (Node *)n; } | REINDEX reindex_target_multitable opt_concurrently name @@ -8416,7 +8419,7 @@ ReindexStmt: n->concurrent = $3; n->name = $4; n->relation = NULL; - n->options = 0; + n->rawoptions = NIL; $$ = (Node *)n; } | REINDEX '(' reindex_option_list ')' reindex_target_type opt_concurrently qualified_name @@ -8426,7 +8429,7 @@ ReindexStmt: n->concurrent = $6; n->relation = $7; n->name = NULL; - n->options = $3; + n->rawoptions = $3; $$ = (Node *)n; } | REINDEX '(' reindex_option_list ')' reindex_target_multitable opt_concurrently name @@ -8436,7 +8439,7 @@ ReindexStmt: n->concurrent = $6; n->name = $7; n->relation = NULL; - n->options = $3; + n->rawoptions = $3; $$ = (Node *)n; } ; @@ -8450,11 +8453,31 @@ reindex_target_multitable: | DATABASE { $$ = REINDEX_OBJECT_DATABASE; } ; reindex_option_list: - reindex_option_elem { $$ = $1; } - | reindex_option_list ',' reindex_option_elem { $$ = $1 | $3; } + reindex_option_elem + { + $$ = list_make1($1); + } + | reindex_option_list ',' reindex_option_elem + { + $$ = lappend($1, $3); + } ; + reindex_option_elem: - VERBOSE { $$ = REINDEXOPT_VERBOSE; } + reindex_option_name reindex_option_arg + { + $$ = makeDefElem($1, $2, @1); + } + ; + +reindex_option_name: + NonReservedWord { $$ = $1; } + ; + +reindex_option_arg: + opt_boolean_or_string { $$ = (Node *) makeString($1); } + | NumericOnly { $$ = (Node *) $1; } + | /* EMPTY */ { $$ = NULL; } ; /***************************************************************************** @@ -10596,6 +10619,7 @@ CreateConversionStmt: * * QUERY: * CLUSTER [VERBOSE] [ USING ] + * CLUSTER [VERBOSE] [(options)] [ USING ] * CLUSTER [VERBOSE] * CLUSTER [VERBOSE] ON (for pre-8.3) * @@ -10610,6 +10634,18 @@ ClusterStmt: n->options = 0; if ($2) n->options |= CLUOPT_VERBOSE; + n->params = NIL; + $$ = (Node*)n; + } +/* XXX: reusing reindex_option_list */ + | CLUSTER opt_verbose '(' reindex_option_list ')' qualified_name cluster_index_specification + { + ClusterStmt *n = makeNode(ClusterStmt); + n->relation = $6; + n->indexname = $7; + if ($2) + n->options |= CLUOPT_VERBOSE; + n->params = $4; $$ = (Node*)n; } | CLUSTER opt_verbose @@ -10620,6 +10656,7 @@ ClusterStmt: n->options = 0; if ($2) n->options |= CLUOPT_VERBOSE; + n->params = NIL; $$ = (Node*)n; } /* kept for pre-8.3 compatibility */ @@ -10631,6 +10668,7 @@ ClusterStmt: n->options = 0; if ($2) n->options |= CLUOPT_VERBOSE; + n->params = NIL; $$ = (Node*)n; } ; diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c index b1f7f6e2d0..d6674a9e38 100644 --- a/src/backend/tcop/utility.c +++ b/src/backend/tcop/utility.c @@ -524,6 +524,30 @@ ProcessUtility(PlannedStmt *pstmt, dest, qc); } +/* Parse rawoptions into options flags and tablespace */ +static +void parse_reindex_options(ParseState *pstate, ReindexStmt *stmt) +{ + ListCell *lc; + /* Parse options list. */ + foreach(lc, stmt->rawoptions) + { + DefElem *opt = (DefElem *) lfirst(lc); + + if (strcmp(opt->defname, "verbose") == 0) + stmt->options |= REINDEXOPT_VERBOSE; + // XXX: handle boolean opt: VERBOSE off + else if (strcmp(opt->defname, "concurrently") == 0) + stmt->concurrent = true; + else + ereport(ERROR, + (errcode(ERRCODE_SYNTAX_ERROR), + errmsg("unrecognized REINDEX option \"%s\"", + opt->defname), + parser_errposition(pstate, opt->location))); + } +} + /* * standard_ProcessUtility itself deals only with utility commands for * which we do not provide event trigger support. Commands that do have @@ -816,7 +840,7 @@ standard_ProcessUtility(PlannedStmt *pstmt, break; case T_ClusterStmt: - cluster((ClusterStmt *) parsetree, isTopLevel); + cluster(pstate, (ClusterStmt *) parsetree, isTopLevel); break; case T_VacuumStmt: @@ -921,13 +945,14 @@ standard_ProcessUtility(PlannedStmt *pstmt, PreventInTransactionBlock(isTopLevel, "REINDEX CONCURRENTLY"); + parse_reindex_options(pstate, stmt); switch (stmt->kind) { case REINDEX_OBJECT_INDEX: - ReindexIndex(stmt->relation, stmt->options, stmt->concurrent); + ReindexIndex(stmt); break; case REINDEX_OBJECT_TABLE: - ReindexTable(stmt->relation, stmt->options, stmt->concurrent); + ReindexTable(stmt); break; case REINDEX_OBJECT_SCHEMA: case REINDEX_OBJECT_SYSTEM: @@ -943,7 +968,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); break; default: elog(ERROR, "unrecognized object type: %d", diff --git a/src/include/commands/cluster.h b/src/include/commands/cluster.h index e05884781b..674cdcd0cd 100644 --- a/src/include/commands/cluster.h +++ b/src/include/commands/cluster.h @@ -14,11 +14,12 @@ #define CLUSTER_H #include "nodes/parsenodes.h" +#include "parser/parse_node.h" #include "storage/lock.h" #include "utils/relcache.h" -extern void cluster(ClusterStmt *stmt, bool isTopLevel); +extern void cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel); extern void cluster_rel(Oid tableOid, Oid indexOid, int options); extern void check_index_is_clusterable(Relation OldHeap, Oid indexOid, bool recheck, LOCKMODE lockmode); diff --git a/src/include/commands/defrem.h b/src/include/commands/defrem.h index c77c9a6ed5..7020edbdbb 100644 --- a/src/include/commands/defrem.h +++ b/src/include/commands/defrem.h @@ -34,10 +34,9 @@ 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 ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind, - int options, bool concurrent); +extern void ReindexIndex(ReindexStmt *stmt); +extern Oid ReindexTable(ReindexStmt *stmt); +extern void ReindexMultipleTables(ReindexStmt *stmt); 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 77943f0637..f72587a584 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -3204,6 +3204,7 @@ typedef struct ClusterStmt RangeVar *relation; /* relation being indexed, or NULL if all */ char *indexname; /* original index defined */ int options; /* OR of ClusterOption flags */ + List *params; /* Params not further parsed by the grammar */ } ClusterStmt; /* ---------------------- @@ -3362,7 +3363,8 @@ typedef struct ReindexStmt * etc. */ RangeVar *relation; /* Table or index to reindex */ const char *name; /* name of database to reindex */ - int options; /* Reindex options flags */ + List *rawoptions; /* Raw options */ + int options; /* Parsed options */ bool concurrent; /* reindex concurrently? */ } ReindexStmt; -- 2.17.0