public inbox for [email protected]help / color / mirror / Atom feed
[PATCH v23 1/5] Change REINDEX/CLUSTER to accept an option list.. 2+ messages / 2 participants [nested] [flat]
* [PATCH v23 1/5] Change REINDEX/CLUSTER to accept an option list.. @ 2020-03-27 22:50 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 2+ messages in thread From: Justin Pryzby @ 2020-03-27 22:50 UTC (permalink / raw) ..like EXPLAIN (..), VACUUM (..), and ANALYZE (..). Change docs in the style of VACUUM. See also: 52dcfda48778d16683c64ca4372299a099a15b96 --- doc/src/sgml/ref/cluster.sgml | 28 ++++++++++++++++++--- doc/src/sgml/ref/reindex.sgml | 43 +++++++++++++++++++++----------- src/backend/commands/cluster.c | 23 ++++++++++++++++- src/backend/commands/indexcmds.c | 41 ++++++++++++++++-------------- src/backend/nodes/copyfuncs.c | 2 ++ src/backend/nodes/equalfuncs.c | 2 ++ src/backend/parser/gram.y | 35 +++++++++++++++----------- src/backend/tcop/utility.c | 36 +++++++++++++++++++++++--- src/bin/psql/tab-complete.c | 23 +++++++++++++---- src/include/commands/cluster.h | 3 ++- src/include/commands/defrem.h | 7 +++--- src/include/nodes/parsenodes.h | 2 ++ 12 files changed, 179 insertions(+), 66 deletions(-) diff --git a/doc/src/sgml/ref/cluster.sgml b/doc/src/sgml/ref/cluster.sgml index 4da60d8d56..110fb3083e 100644 --- a/doc/src/sgml/ref/cluster.sgml +++ b/doc/src/sgml/ref/cluster.sgml @@ -21,8 +21,13 @@ PostgreSQL documentation <refsynopsisdiv> <synopsis> -CLUSTER [VERBOSE] <replaceable class="parameter">table_name</replaceable> [ USING <replaceable class="parameter">index_name</replaceable> ] -CLUSTER [VERBOSE] +CLUSTER [VERBOSE] [ ( <replaceable class="parameter">option</replaceable> [, ...] ) ] <replaceable class="parameter">table_name</replaceable> [ USING <replaceable class="parameter">index_name</replaceable> ] +CLUSTER [VERBOSE] [ ( <replaceable class="parameter">option</replaceable> [, ...] ) ] + +<phrase>where <replaceable class="parameter">option</replaceable> can be one of:</phrase> + + VERBOSE [ <replaceable class="parameter">boolean</replaceable> ] + </synopsis> </refsynopsisdiv> @@ -81,6 +86,16 @@ CLUSTER [VERBOSE] <title>Parameters</title> <variablelist> + <varlistentry> + <term><literal>VERBOSE</literal></term> + <listitem> + <para> + Prints a progress report as each table is clustered. +<!-- When specified within parenthesis, <literal>VERBOSE</literal> may be followed by a boolean ...--> + </para> + </listitem> + </varlistentry> + <varlistentry> <term><replaceable class="parameter">table_name</replaceable></term> <listitem> @@ -100,10 +115,15 @@ CLUSTER [VERBOSE] </varlistentry> <varlistentry> - <term><literal>VERBOSE</literal></term> + <term><replaceable class="parameter">boolean</replaceable></term> <listitem> <para> - Prints a progress report as each table is clustered. + Specifies whether the selected option should be turned on or off. + You can write <literal>TRUE</literal>, <literal>ON</literal>, or + <literal>1</literal> to enable the option, and <literal>FALSE</literal>, + <literal>OFF</literal>, or <literal>0</literal> to disable it. The + <replaceable class="parameter">boolean</replaceable> value can also + be omitted, in which case <literal>TRUE</literal> is assumed. </para> </listitem> </varlistentry> 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 [ ( <replaceable class="parameter">option</replaceable> [, ...] ) ] { IN <phrase>where <replaceable class="parameter">option</replaceable> can be one of:</phrase> - VERBOSE + VERBOSE [ <replaceable class="parameter">boolean</replaceable> ] </synopsis> </refsynopsisdiv> @@ -141,19 +141,6 @@ REINDEX [ ( <replaceable class="parameter">option</replaceable> [, ...] ) ] { IN </listitem> </varlistentry> - <varlistentry> - <term><replaceable class="parameter">name</replaceable></term> - <listitem> - <para> - The name of the specific index, table, or database to be - reindexed. Index and table names can be schema-qualified. - Presently, <command>REINDEX DATABASE</command> and <command>REINDEX SYSTEM</command> - can only reindex the current database, so their parameter must match - the current database's name. - </para> - </listitem> - </varlistentry> - <varlistentry> <term><literal>CONCURRENTLY</literal></term> <listitem> @@ -181,6 +168,34 @@ REINDEX [ ( <replaceable class="parameter">option</replaceable> [, ...] ) ] { IN </para> </listitem> </varlistentry> + + <varlistentry> + <term><replaceable class="parameter">name</replaceable></term> + <listitem> + <para> + The name of the specific index, table, or database to be + reindexed. Index and table names can be schema-qualified. + Presently, <command>REINDEX DATABASE</command> and <command>REINDEX SYSTEM</command> + can only reindex the current database, so their parameter must match + the current database's name. + </para> + </listitem> + </varlistentry> + + <varlistentry> + <term><replaceable class="parameter">boolean</replaceable></term> + <listitem> + <para> + Specifies whether the selected option should be turned on or off. + You can write <literal>TRUE</literal>, <literal>ON</literal>, or + <literal>1</literal> to enable the option, and <literal>FALSE</literal>, + <literal>OFF</literal>, or <literal>0</literal> to disable it. The + <replaceable class="parameter">boolean</replaceable> value can also + be omitted, in which case <literal>TRUE</literal> is assumed. + </para> + </listitem> + </varlistentry> + </variablelist> </refsect1> diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c index 0d647e912c..a42dfe98f4 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,28 @@ 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 parameters not handled by the parser */ + foreach(lc, stmt->params) + { + DefElem *opt = (DefElem *) lfirst(lc); + + if (strcmp(opt->defname, "verbose") == 0) + if (defGetBoolean(opt)) + stmt->options |= CLUOPT_VERBOSE; + else + stmt->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. */ diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index b3a92381f9..e8383feea7 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -2420,8 +2420,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; @@ -2437,10 +2438,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); @@ -2460,11 +2461,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); } /* @@ -2542,8 +2543,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; @@ -2556,13 +2558,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, @@ -2574,7 +2576,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", @@ -2593,9 +2595,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; @@ -2613,7 +2616,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"))); @@ -2724,7 +2727,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) @@ -2774,10 +2777,10 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind, continue; } - if (concurrent && get_rel_persistence(relid) != RELPERSISTENCE_TEMP) + if (stmt->concurrent && get_rel_persistence(relid) != RELPERSISTENCE_TEMP) { (void) ReindexRelationConcurrently(relid, - options | + stmt->options | REINDEXOPT_MISSING_OK); /* ReindexRelationConcurrently() does the verbose output */ } @@ -2788,11 +2791,11 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind, result = reindex_relation(relid, REINDEX_REL_PROCESS_TOAST | REINDEX_REL_CHECK_CONSTRAINTS, - options | + stmt->options | REINDEXOPT_REPORT_PROGRESS | REINDEXOPT_MISSING_OK); - 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 89c409de66..71548acc0c 100644 --- a/src/backend/nodes/copyfuncs.c +++ b/src/backend/nodes/copyfuncs.c @@ -3358,6 +3358,7 @@ _copyClusterStmt(const ClusterStmt *from) COPY_NODE_FIELD(relation); COPY_STRING_FIELD(indexname); COPY_SCALAR_FIELD(options); + COPY_NODE_FIELD(params); return newnode; } @@ -4450,6 +4451,7 @@ _copyReindexStmt(const ReindexStmt *from) COPY_NODE_FIELD(relation); 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 e3f33c40be..de48a42cdd 100644 --- a/src/backend/nodes/equalfuncs.c +++ b/src/backend/nodes/equalfuncs.c @@ -1217,6 +1217,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; } @@ -2135,6 +2136,7 @@ _equalReindexStmt(const ReindexStmt *a, const ReindexStmt *b) COMPARE_NODE_FIELD(relation); 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 dbb47d4982..b909b161a6 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 <list> explain_option_list %type <ival> reindex_target_type reindex_target_multitable -%type <ival> reindex_option_list reindex_option_elem %type <node> copy_generic_opt_arg copy_generic_opt_arg_list_item %type <defelt> copy_generic_opt_elem @@ -8180,7 +8179,7 @@ ReindexStmt: n->concurrent = $3; n->relation = $4; n->name = NULL; - n->options = 0; + n->params = NIL; $$ = (Node *)n; } | REINDEX reindex_target_multitable opt_concurrently name @@ -8190,27 +8189,27 @@ ReindexStmt: n->concurrent = $3; n->name = $4; n->relation = NULL; - n->options = 0; + n->params = NIL; $$ = (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->concurrent = $6; n->relation = $7; n->name = NULL; - n->options = $3; + n->params = $3; $$ = (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->concurrent = $6; n->name = $7; n->relation = NULL; - n->options = $3; + n->params = $3; $$ = (Node *)n; } ; @@ -8223,13 +8222,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; } - ; /***************************************************************************** * @@ -10380,6 +10372,7 @@ CreateConversionStmt: * * QUERY: * CLUSTER [VERBOSE] <qualified_name> [ USING <index_name> ] + * CLUSTER [VERBOSE] [(options)] <qualified_name> [ USING <index_name> ] * CLUSTER [VERBOSE] * CLUSTER [VERBOSE] <index_name> ON <qualified_name> (for pre-8.3) * @@ -10394,6 +10387,18 @@ ClusterStmt: n->options = 0; if ($2) n->options |= CLUOPT_VERBOSE; + n->params = NIL; + $$ = (Node*)n; + } + + | CLUSTER opt_verbose '(' vac_analyze_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 @@ -10404,6 +10409,7 @@ ClusterStmt: n->options = 0; if ($2) n->options |= CLUOPT_VERBOSE; + n->params = NIL; $$ = (Node*)n; } /* kept for pre-8.3 compatibility */ @@ -10415,6 +10421,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 6154d2c8c6..0e074d7745 100644 --- a/src/backend/tcop/utility.c +++ b/src/backend/tcop/utility.c @@ -526,6 +526,33 @@ ProcessUtility(PlannedStmt *pstmt, dest, qc); } +/* Parse params not parsed by the grammar */ +static +void parse_reindex_params(ParseState *pstate, ReindexStmt *stmt) +{ + ListCell *lc; + foreach(lc, stmt->params) + { + DefElem *opt = (DefElem *) lfirst(lc); + + if (strcmp(opt->defname, "verbose") == 0) + { + if (defGetBoolean(opt)) + stmt->options |= REINDEXOPT_VERBOSE; + else + stmt->options &= ~REINDEXOPT_VERBOSE; + } + 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 @@ -818,7 +845,7 @@ standard_ProcessUtility(PlannedStmt *pstmt, break; case T_ClusterStmt: - cluster((ClusterStmt *) parsetree, isTopLevel); + cluster(pstate, (ClusterStmt *) parsetree, isTopLevel); break; case T_VacuumStmt: @@ -923,13 +950,14 @@ standard_ProcessUtility(PlannedStmt *pstmt, PreventInTransactionBlock(isTopLevel, "REINDEX CONCURRENTLY"); + parse_reindex_params(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: @@ -945,7 +973,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/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c index f41785f11c..de85772ff9 100644 --- a/src/bin/psql/tab-complete.c +++ b/src/bin/psql/tab-complete.c @@ -2259,21 +2259,34 @@ 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", "VERBOSE", "(*)") || + Matches("CLUSTER", "(*)")) COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_clusterables, NULL); /* If we have CLUSTER <sth>, 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 <sth>, then add "USING" */ - else if (Matches("CLUSTER", "VERBOSE", MatchAny)) + else if (Matches("CLUSTER", "VERBOSE|(*)", MatchAny)) COMPLETE_WITH("USING"); /* If we have CLUSTER <sth> 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 +3483,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 c26a102b17..a97c00a02e 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 23840bb8e6..6fb599c286 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -3211,6 +3211,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; /* ---------------------- @@ -3371,6 +3372,7 @@ typedef struct ReindexStmt RangeVar *relation; /* Table or index to reindex */ 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 --mP3DRpeJDSE+ciuQ Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v23-0002-Deprecate-ReindexStmt-concurrent-and-options.patch" ^ permalink raw reply [nested|flat] 2+ messages in thread
* Re: Wrong rows estimations with joins of CTEs slows queries by more than factor 500 @ 2023-02-08 21:27 Tomas Vondra <[email protected]> 0 siblings, 0 replies; 2+ messages in thread From: Tomas Vondra @ 2023-02-08 21:27 UTC (permalink / raw) To: Hans Buschmann <[email protected]>; [email protected] <[email protected]> On 2/8/23 14:55, Hans Buschmann wrote: > During data refactoring of our Application I encountered $subject when > joining 4 CTEs with left join or inner join. > > > 1. Background > > PG 15.1 on Windows x64 (OS seems no to have no meening here) > > > I try to collect data from 4 (analyzed) tables (up,li,in,ou) by grouping > certain data (4 CTEs qup,qli,qin,qou) > > The grouping of the data in the CTEs gives estimated row counts of about > 1000 (1 tenth of the real value) This is OK for estimation. > > > These 4 CTEs are then used to combine the data by joining them. > > > 2. Problem > > The 4 CTEs are joined by left joins as shown below: > ... > > This case really brought me to detect the problem! > > The original query and data are not shown here, but the principle should > be clear from the execution plans. > > I think the planner shouldn't change the row estimations on further > steps after left joins at all, and be a bit more conservative on inner > joins. But the code should alredy do exactly that, see: https://github.com/postgres/postgres/blob/dbe8a1726cfd5a09cf1ef99e76f5f89e2efada71/src/backend/optim... And in fact, the second part of the plains shows it's doing the trick: -> Merge Left Join (cost=1293.25..1388.21 rows=5733 width=104) (actual time=2.321..2.556 rows=1415 loops=1) Merge Cond: ((qup_1.curr_season = qli_1.curr_season) AND ((qup_1.curr_code)::text = (qli_1.curr_code)::text)) -> Sort (cost=641.68..656.02 rows=5733 width=72) -> Sort (cost=651.57..666.11 rows=5816 width=72) But notice the first join (with rows=33) doesn't say "Left". And I see there's Append on top, so presumably the query is much more complex, and there's a regular join of these CTEs in some other part. We'll need to se the whole query, not just one chunk of it. FWIW it seems you're using materialized CTEs - that's likely pretty bad for the estimates, because we don't propagate statistics from the CTE. So a join on CTEs can't see statistics from the underlying tables, and that can easily produce really bad estimates. I'm assuming you're not using AS MATERIALIZED explicitly, so I'd bet this happens because the "cardinality" function is marked as volatile. Perhaps it can be redefined as stable/immutable. > This may be related to the fact that this case has 2 join-conditions > (xx_season an xx_code). That shouldn't affect outer join estimates this way (but as I explained above, the join does not seem to be "left" per the explain). Multi-column joins can cause issues, no doubt about it - but CTEs make it worse because we can't e.g. see foreign keys. regards -- Tomas Vondra EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company ^ permalink raw reply [nested|flat] 2+ messages in thread
end of thread, other threads:[~2023-02-08 21:27 UTC | newest] Thread overview: 2+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2020-03-27 22:50 [PATCH v23 1/5] Change REINDEX/CLUSTER to accept an option list.. Justin Pryzby <[email protected]> 2023-02-08 21:27 Re: Wrong rows estimations with joins of CTEs slows queries by more than factor 500 Tomas Vondra <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox