From 7e04caad0d010b5fd3eeca8d9bd436e89b657e4a Mon Sep 17 00:00:00 2001 From: Justin Pryzby Date: Fri, 27 Mar 2020 17:50:46 -0500 Subject: [PATCH v26 1/5] Change REINDEX/CLUSTER to accept an option list.. ..like EXPLAIN (..), VACUUM (..), and ANALYZE (..). Change docs in the style of VACUUM. See also: 52dcfda48778d16683c64ca4372299a099a15b96 --- doc/src/sgml/ref/cluster.sgml | 27 ++++++++++++++--- doc/src/sgml/ref/reindex.sgml | 43 ++++++++++++++++++--------- src/backend/commands/cluster.c | 28 ++++++++++++++++-- src/backend/nodes/copyfuncs.c | 4 +-- src/backend/nodes/equalfuncs.c | 4 +-- src/backend/parser/gram.y | 54 +++++++++++++++++++--------------- src/backend/tcop/utility.c | 42 ++++++++++++++++++++++---- src/bin/psql/tab-complete.c | 22 ++++++++++---- src/include/commands/cluster.h | 3 +- src/include/commands/defrem.h | 2 +- src/include/nodes/parsenodes.h | 4 +-- 11 files changed, 170 insertions(+), 63 deletions(-) diff --git a/doc/src/sgml/ref/cluster.sgml b/doc/src/sgml/ref/cluster.sgml index 4da60d8d56..e6ebce27e6 100644 --- a/doc/src/sgml/ref/cluster.sgml +++ b/doc/src/sgml/ref/cluster.sgml @@ -21,8 +21,13 @@ PostgreSQL documentation -CLUSTER [VERBOSE] table_name [ USING index_name ] -CLUSTER [VERBOSE] +CLUSTER [VERBOSE] [ ( option [, ...] ) ] table_name [ USING index_name ] +CLUSTER [VERBOSE] [ ( option [, ...] ) ] + +where option can be one of: + + VERBOSE [ boolean ] + @@ -81,6 +86,15 @@ CLUSTER [VERBOSE] Parameters + + VERBOSE + + + Prints a progress report as each table is clustered. + + + + table_name @@ -100,10 +114,15 @@ CLUSTER [VERBOSE] - VERBOSE + boolean - Prints a progress report as each table is clustered. + Specifies whether the selected option should be turned on or off. + You can write TRUE, ON, or + 1 to enable the option, and FALSE, + OFF, or 0 to disable it. The + boolean value can also + be omitted, in which case TRUE is assumed. diff --git a/doc/src/sgml/ref/reindex.sgml b/doc/src/sgml/ref/reindex.sgml index c16f223e4e..a32e192a87 100644 --- a/doc/src/sgml/ref/reindex.sgml +++ b/doc/src/sgml/ref/reindex.sgml @@ -25,7 +25,7 @@ REINDEX [ ( option [, ...] ) ] { IN where option can be one of: - VERBOSE + VERBOSE [ boolean ] @@ -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 @@ -181,6 +168,34 @@ 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. + + + + + + boolean + + + Specifies whether the selected option should be turned on or off. + You can write TRUE, ON, or + 1 to enable the option, and FALSE, + OFF, or 0 to disable it. The + boolean value can also + be omitted, in which case TRUE is assumed. + + + + diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c index 0d647e912c..4a3678831d 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" @@ -102,8 +103,29 @@ static List *get_tables_to_cluster(MemoryContext cluster_context); *--------------------------------------------------------------------------- */ void -cluster(ClusterStmt *stmt, bool isTopLevel) +cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel) { + ListCell *lc; + int options = 0; + + /* Parse list of generic parameters not handled by the parser */ + foreach(lc, stmt->params) + { + DefElem *opt = (DefElem *) lfirst(lc); + + if (strcmp(opt->defname, "verbose") == 0) + if (defGetBoolean(opt)) + options |= CLUOPT_VERBOSE; + else + options &= ~CLUOPT_VERBOSE; + 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. */ @@ -173,7 +195,7 @@ cluster(ClusterStmt *stmt, bool isTopLevel) table_close(rel, NoLock); /* Do the job. */ - cluster_rel(tableOid, indexOid, stmt->options, isTopLevel); + cluster_rel(tableOid, indexOid, options, isTopLevel); } else { @@ -222,7 +244,7 @@ cluster(ClusterStmt *stmt, bool isTopLevel) PushActiveSnapshot(GetTransactionSnapshot()); /* Do the job. */ cluster_rel(rvtc->tableOid, rvtc->indexOid, - stmt->options | CLUOPT_RECHECK, + options | CLUOPT_RECHECK, isTopLevel); PopActiveSnapshot(); CommitTransactionCommand(); diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c index 0409a40b82..2a73f1cb6d 100644 --- a/src/backend/nodes/copyfuncs.c +++ b/src/backend/nodes/copyfuncs.c @@ -3357,7 +3357,7 @@ _copyClusterStmt(const ClusterStmt *from) COPY_NODE_FIELD(relation); COPY_STRING_FIELD(indexname); - COPY_SCALAR_FIELD(options); + COPY_NODE_FIELD(params); return newnode; } @@ -4449,7 +4449,7 @@ _copyReindexStmt(const ReindexStmt *from) COPY_SCALAR_FIELD(kind); COPY_NODE_FIELD(relation); COPY_STRING_FIELD(name); - COPY_SCALAR_FIELD(options); + COPY_NODE_FIELD(params); return newnode; } diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c index e2d1b987bf..3c6c40a50d 100644 --- a/src/backend/nodes/equalfuncs.c +++ b/src/backend/nodes/equalfuncs.c @@ -1216,7 +1216,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; } @@ -2134,7 +2134,7 @@ _equalReindexStmt(const ReindexStmt *a, const ReindexStmt *b) COMPARE_SCALAR_FIELD(kind); COMPARE_NODE_FIELD(relation); COMPARE_STRING_FIELD(name); - COMPARE_SCALAR_FIELD(options); + COMPARE_NODE_FIELD(params); return true; } diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index c5154b818c..0256da5530 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -519,7 +519,6 @@ 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 copy_generic_opt_arg copy_generic_opt_arg_list_item %type copy_generic_opt_elem @@ -8179,9 +8178,10 @@ ReindexStmt: n->kind = $2; n->relation = $4; n->name = NULL; - n->options = 0; + n->params = NIL; if ($3) - n->options |= REINDEXOPT_CONCURRENTLY; + n->params = lappend(n->params, + makeDefElem("concurrently", NULL, @3)); $$ = (Node *)n; } | REINDEX reindex_target_multitable opt_concurrently name @@ -8190,31 +8190,34 @@ ReindexStmt: n->kind = $2; n->name = $4; n->relation = NULL; - n->options = 0; + n->params = NIL; if ($3) - n->options |= REINDEXOPT_CONCURRENTLY; + n->params = lappend(n->params, + makeDefElem("concurrently", NULL, @3)); $$ = (Node *)n; } - | REINDEX '(' reindex_option_list ')' reindex_target_type opt_concurrently qualified_name + | REINDEX '(' vac_analyze_option_list ')' reindex_target_type opt_concurrently qualified_name { ReindexStmt *n = makeNode(ReindexStmt); n->kind = $5; n->relation = $7; n->name = NULL; - n->options = $3; + n->params = $3; if ($6) - n->options |= REINDEXOPT_CONCURRENTLY; + n->params = lappend(n->params, + makeDefElem("concurrently", NULL, @6)); $$ = (Node *)n; } - | REINDEX '(' reindex_option_list ')' reindex_target_multitable opt_concurrently name + | REINDEX '(' vac_analyze_option_list ')' reindex_target_multitable opt_concurrently name { ReindexStmt *n = makeNode(ReindexStmt); n->kind = $5; n->name = $7; n->relation = NULL; - n->options = $3; + n->params = $3; if ($6) - n->options |= REINDEXOPT_CONCURRENTLY; + n->params = lappend(n->params, + makeDefElem("concurrently", NULL, @6)); $$ = (Node *)n; } ; @@ -8227,13 +8230,6 @@ reindex_target_multitable: | SYSTEM_P { $$ = REINDEX_OBJECT_SYSTEM; } | DATABASE { $$ = REINDEX_OBJECT_DATABASE; } ; -reindex_option_list: - reindex_option_elem { $$ = $1; } - | reindex_option_list ',' reindex_option_elem { $$ = $1 | $3; } - ; -reindex_option_elem: - VERBOSE { $$ = REINDEXOPT_VERBOSE; } - ; /***************************************************************************** * @@ -10384,6 +10380,7 @@ CreateConversionStmt: * * QUERY: * CLUSTER [VERBOSE] [ USING ] + * CLUSTER [VERBOSE] [(options)] [ USING ] * CLUSTER [VERBOSE] * CLUSTER [VERBOSE] ON (for pre-8.3) * @@ -10395,9 +10392,18 @@ ClusterStmt: ClusterStmt *n = makeNode(ClusterStmt); n->relation = $3; n->indexname = $4; - n->options = 0; + n->params = NIL; if ($2) - n->options |= CLUOPT_VERBOSE; + n->params = lappend(n->params, makeDefElem("verbose", NULL, @2)); + $$ = (Node*)n; + } + + | CLUSTER '(' vac_analyze_option_list ')' qualified_name cluster_index_specification + { + ClusterStmt *n = makeNode(ClusterStmt); + n->relation = $5; + n->indexname = $6; + n->params = $3; $$ = (Node*)n; } | CLUSTER opt_verbose @@ -10405,9 +10411,9 @@ ClusterStmt: ClusterStmt *n = makeNode(ClusterStmt); n->relation = NULL; n->indexname = NULL; - n->options = 0; + n->params = NIL; if ($2) - n->options |= CLUOPT_VERBOSE; + n->params = lappend(n->params, makeDefElem("verbose", NULL, @2)); $$ = (Node*)n; } /* kept for pre-8.3 compatibility */ @@ -10416,9 +10422,9 @@ ClusterStmt: ClusterStmt *n = makeNode(ClusterStmt); n->relation = $5; n->indexname = $3; - n->options = 0; + n->params = NIL; if ($2) - n->options |= CLUOPT_VERBOSE; + n->params = lappend(n->params, makeDefElem("verbose", NULL, @2)); $$ = (Node*)n; } ; diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c index b4cde5565e..3e67effec3 100644 --- a/src/backend/tcop/utility.c +++ b/src/backend/tcop/utility.c @@ -526,6 +526,36 @@ ProcessUtility(PlannedStmt *pstmt, dest, qc); } +/* Parse params not parsed by the grammar */ +static +void parse_reindex_params(ParseState *pstate, ReindexStmt *stmt, int *options) +{ + ListCell *lc; + foreach(lc, stmt->params) + { + DefElem *opt = (DefElem *) lfirst(lc); + + if (strcmp(opt->defname, "verbose") == 0) + { + if (defGetBoolean(opt)) + *options |= REINDEXOPT_VERBOSE; + else + *options &= ~REINDEXOPT_VERBOSE; + } + else if (strcmp(opt->defname, "concurrently") == 0) + if (defGetBoolean(opt)) + *options |= REINDEXOPT_CONCURRENTLY; + else + *options &= ~REINDEXOPT_CONCURRENTLY; + 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 @@ -818,7 +848,7 @@ standard_ProcessUtility(PlannedStmt *pstmt, break; case T_ClusterStmt: - cluster((ClusterStmt *) parsetree, isTopLevel); + cluster(pstate, (ClusterStmt *) parsetree, isTopLevel); break; case T_VacuumStmt: @@ -918,18 +948,20 @@ standard_ProcessUtility(PlannedStmt *pstmt, case T_ReindexStmt: { ReindexStmt *stmt = (ReindexStmt *) parsetree; + int options = 0; - if ((stmt->options & REINDEXOPT_CONCURRENTLY) != 0) + parse_reindex_params(pstate, stmt, &options); + if (options & REINDEXOPT_CONCURRENTLY) PreventInTransactionBlock(isTopLevel, "REINDEX CONCURRENTLY"); switch (stmt->kind) { case REINDEX_OBJECT_INDEX: - ReindexIndex(stmt->relation, stmt->options); + ReindexIndex(stmt->relation, options); break; case REINDEX_OBJECT_TABLE: - ReindexTable(stmt->relation, stmt->options); + ReindexTable(stmt->relation, options); break; case REINDEX_OBJECT_SCHEMA: case REINDEX_OBJECT_SYSTEM: @@ -945,7 +977,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); + ReindexMultipleTables(stmt->name, stmt->kind, options); break; default: elog(ERROR, "unrecognized object type: %d", diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c index f41785f11c..245919aead 100644 --- a/src/bin/psql/tab-complete.c +++ b/src/bin/psql/tab-complete.c @@ -2259,21 +2259,33 @@ psql_completion(const char *text, int start, int end) /* CLUSTER */ else if (Matches("CLUSTER")) COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_clusterables, "UNION SELECT 'VERBOSE'"); - else if (Matches("CLUSTER", "VERBOSE")) + else if (Matches("CLUSTER", "VERBOSE") || + Matches("CLUSTER", "(*)")) COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_clusterables, NULL); /* If we have CLUSTER , then add "USING" */ - else if (Matches("CLUSTER", MatchAnyExcept("VERBOSE|ON"))) + else if (Matches("CLUSTER", MatchAnyExcept("VERBOSE|ON|(|(*)"))) COMPLETE_WITH("USING"); /* If we have CLUSTER VERBOSE , then add "USING" */ - else if (Matches("CLUSTER", "VERBOSE", MatchAny)) + else if (Matches("CLUSTER", "VERBOSE|(*)", MatchAny)) COMPLETE_WITH("USING"); /* If we have CLUSTER USING, then add the index as well */ else if (Matches("CLUSTER", MatchAny, "USING") || - Matches("CLUSTER", "VERBOSE", MatchAny, "USING")) + Matches("CLUSTER", "VERBOSE|(*)", MatchAny, "USING")) { completion_info_charp = prev2_wd; COMPLETE_WITH_QUERY(Query_for_index_of_table); } + else if (HeadMatches("CLUSTER", "(*") && + !HeadMatches("CLUSTER", "(*)")) + { + /* + * This fires if we're in an unfinished parenthesized option list. + * get_previous_words treats a completed parenthesized option list as + * one word, so the above test is correct. + */ + if (ends_with(prev_wd, '(') || ends_with(prev_wd, ',')) + COMPLETE_WITH("VERBOSE"); + } /* COMMENT */ else if (Matches("COMMENT")) @@ -3470,7 +3482,7 @@ psql_completion(const char *text, int start, int end) * one word, so the above test is correct. */ if (ends_with(prev_wd, '(') || ends_with(prev_wd, ',')) - COMPLETE_WITH("VERBOSE"); + COMPLETE_WITH("CONCURRENTLY", "VERBOSE"); } /* SECURITY LABEL */ diff --git a/src/include/commands/cluster.h b/src/include/commands/cluster.h index 1eb144204b..c761b9575a 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, bool isTopLevel); extern void check_index_is_clusterable(Relation OldHeap, Oid indexOid, diff --git a/src/include/commands/defrem.h b/src/include/commands/defrem.h index 3129b684f6..7d9f3f4687 100644 --- a/src/include/commands/defrem.h +++ b/src/include/commands/defrem.h @@ -37,7 +37,7 @@ extern ObjectAddress DefineIndex(Oid relationId, extern void ReindexIndex(RangeVar *indexRelation, int options); extern Oid ReindexTable(RangeVar *relation, int options); extern void ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind, - int options); + 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 e83329fd6d..dd6976b46d 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -3210,7 +3210,7 @@ typedef struct ClusterStmt NodeTag type; RangeVar *relation; /* relation being indexed, or NULL if all */ char *indexname; /* original index defined */ - int options; /* OR of ClusterOption flags */ + List *params; /* list of DefElem nodes */ } ClusterStmt; /* ---------------------- @@ -3371,7 +3371,7 @@ 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 *params; /* list of DefElem nodes */ } ReindexStmt; /* ---------------------- -- 2.17.0