On Sat, Apr 11, 2020 at 08:33:52PM -0500, Justin Pryzby wrote: > > That's the last known issue with the patch. I doubt anyone will elect to > > pick > > it up in the next 8 hours, but I think it's in very good shape for v14 :) > > I tweaked some comments and docs and plan to mark this RfC.
Rebased onto d12bdba77b0fce9df818bc84ad8b1d8e7a96614b Restored two tests from Alexey's original patch which exposed issue with REINDEX DATABASE when allow_system_table_mods=off. -- Justin
>From b29f3b29287188bf4fd3ff2289cb1dfbb09ff99f Mon Sep 17 00:00:00 2001 From: Justin Pryzby <pryz...@telsasoft.com> Date: Thu, 2 Apr 2020 14:20:11 -0500 Subject: [PATCH v21 1/5] tab completion for reindex(verbose).. ..which was added at ecd222e77 for v9.5. This handles "verbose" itself as well as the following word. Separate commit as this could be backpatched to v12 (but backpatching further is less trivial, due to improvements added at 121213d9d). --- src/bin/psql/tab-complete.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c index f6fd623c98..8178e69575 100644 --- a/src/bin/psql/tab-complete.c +++ b/src/bin/psql/tab-complete.c @@ -3418,7 +3418,7 @@ psql_completion(const char *text, int start, int end) COMPLETE_WITH("DATA"); /* REINDEX */ - else if (Matches("REINDEX")) + else if (Matches("REINDEX") || Matches("REINDEX", "(*)")) COMPLETE_WITH("TABLE", "INDEX", "SYSTEM", "SCHEMA", "DATABASE"); else if (Matches("REINDEX", "TABLE")) COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_indexables, @@ -3440,6 +3440,17 @@ psql_completion(const char *text, int start, int end) COMPLETE_WITH_QUERY(Query_for_list_of_schemas); else if (Matches("REINDEX", "SYSTEM|DATABASE", "CONCURRENTLY")) COMPLETE_WITH_QUERY(Query_for_list_of_databases); + else if (HeadMatches("REINDEX", "(*") && + !HeadMatches("REINDEX", "(*)")) + { + /* + * 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"); + } /* SECURITY LABEL */ else if (Matches("SECURITY")) -- 2.17.0
>From eff5654089dc199d596535fcc22b8f4a2b7e27a6 Mon Sep 17 00:00:00 2001 From: Justin Pryzby <pryz...@telsasoft.com> Date: Fri, 27 Mar 2020 17:50:46 -0500 Subject: [PATCH v21 2/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 | 29 ++++++++++++++++++--- 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, 180 insertions(+), 66 deletions(-) diff --git a/doc/src/sgml/ref/cluster.sgml b/doc/src/sgml/ref/cluster.sgml index 4da60d8d56..a6df8a3d81 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,13 +115,19 @@ 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> + </variablelist> </refsect1> diff --git a/doc/src/sgml/ref/reindex.sgml b/doc/src/sgml/ref/reindex.sgml index c54a7c420d..71941e52e3 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> @@ -182,6 +169,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 04d12a7ece..1817b8829a 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,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 2baca12c5f..f5eac5dc8e 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) @@ -2766,9 +2769,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 @@ -2778,9 +2781,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 491452ae2d..ecf16f1190 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 8408c28ec6..142c3c7d3a 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 3c78f2d1b5..6fb4d814cc 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -521,7 +521,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 @@ -8421,7 +8420,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 @@ -8431,27 +8430,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; } ; @@ -8464,13 +8463,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; } - ; /***************************************************************************** * @@ -10621,6 +10613,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) * @@ -10635,6 +10628,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 @@ -10645,6 +10650,7 @@ ClusterStmt: n->options = 0; if ($2) n->options |= CLUOPT_VERBOSE; + n->params = NIL; $$ = (Node*)n; } /* kept for pre-8.3 compatibility */ @@ -10656,6 +10662,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..23f4831ace 100644 --- a/src/backend/tcop/utility.c +++ b/src/backend/tcop/utility.c @@ -524,6 +524,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 @@ -816,7 +843,7 @@ standard_ProcessUtility(PlannedStmt *pstmt, break; case T_ClusterStmt: - cluster((ClusterStmt *) parsetree, isTopLevel); + cluster(pstate, (ClusterStmt *) parsetree, isTopLevel); break; case T_VacuumStmt: @@ -921,13 +948,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: @@ -943,7 +971,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 8178e69575..b2be3aba5c 100644 --- a/src/bin/psql/tab-complete.c +++ b/src/bin/psql/tab-complete.c @@ -2253,21 +2253,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")) @@ -3449,7 +3462,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 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 5e1ffafb91..490b12890f 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -3210,6 +3210,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; /* ---------------------- @@ -3369,6 +3370,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
>From 38c1494a43a6f9f0b5b13634490ea1ea8f098e0a Mon Sep 17 00:00:00 2001 From: Alexey Kondratov <kondratov.alek...@gmail.com> Date: Mon, 23 Mar 2020 21:10:29 +0300 Subject: [PATCH v21 3/5] Allow REINDEX to change tablespace REINDEX already does full relation rewrite, this patch adds a possibility to specify a new tablespace where new relfilenode will be created. --- doc/src/sgml/ref/reindex.sgml | 22 +++++ src/backend/catalog/index.c | 101 ++++++++++++++++++++-- src/backend/commands/cluster.c | 2 +- src/backend/commands/indexcmds.c | 72 +++++++++++++-- src/backend/commands/tablecmds.c | 2 +- src/backend/nodes/copyfuncs.c | 1 + src/backend/nodes/equalfuncs.c | 1 + src/backend/tcop/utility.c | 2 + src/bin/psql/tab-complete.c | 4 +- src/include/catalog/index.h | 5 +- src/include/nodes/parsenodes.h | 1 + src/test/regress/input/tablespace.source | 45 ++++++++++ src/test/regress/output/tablespace.source | 61 +++++++++++++ 13 files changed, 301 insertions(+), 18 deletions(-) diff --git a/doc/src/sgml/ref/reindex.sgml b/doc/src/sgml/ref/reindex.sgml index 71941e52e3..23b4644f30 100644 --- a/doc/src/sgml/ref/reindex.sgml +++ b/doc/src/sgml/ref/reindex.sgml @@ -26,6 +26,7 @@ REINDEX [ ( <replaceable class="parameter">option</replaceable> [, ...] ) ] { IN <phrase>where <replaceable class="parameter">option</replaceable> can be one of:</phrase> VERBOSE [ <replaceable class="parameter">boolean</replaceable> ] + TABLESPACE <replaceable class="parameter">new_tablespace</replaceable> </synopsis> </refsynopsisdiv> @@ -161,6 +162,19 @@ REINDEX [ ( <replaceable class="parameter">option</replaceable> [, ...] ) ] { IN </listitem> </varlistentry> + <varlistentry> + <term><literal>TABLESPACE</literal></term> + <listitem> + <para> + This specifies that indexes will be rebuilt on a new tablespace. + Cannot be used with "mapped" relations. If <literal>SCHEMA</literal>, + <literal>DATABASE</literal> or <literal>SYSTEM</literal> is specified, then + all unsuitable relations will be skipped and a single <literal>WARNING</literal> + will be generated. + </para> + </listitem> + </varlistentry> + <varlistentry> <term><literal>VERBOSE</literal></term> <listitem> @@ -197,6 +211,14 @@ REINDEX [ ( <replaceable class="parameter">option</replaceable> [, ...] ) ] { IN </listitem> </varlistentry> + <varlistentry> + <term><replaceable class="parameter">new_tablespace</replaceable></term> + <listitem> + <para> + The tablespace where indexes will be rebuilt. + </para> + </listitem> + </varlistentry> </variablelist> </refsect1> diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c index 7cfbdd57db..1c3b142f84 100644 --- a/src/backend/catalog/index.c +++ b/src/backend/catalog/index.c @@ -56,6 +56,7 @@ #include "commands/event_trigger.h" #include "commands/progress.h" #include "commands/tablecmds.h" +#include "commands/tablespace.h" #include "commands/trigger.h" #include "executor/executor.h" #include "miscadmin.h" @@ -1246,9 +1247,13 @@ index_create(Relation heapRelation, * Create concurrently an index based on the definition of the one provided by * caller. The index is inserted into catalogs and needs to be built later * on. This is called during concurrent reindex processing. + * + * "tablespaceOid" is the new tablespace to use for this index. If + * InvalidOid, use the tablespace in-use instead. */ Oid -index_concurrently_create_copy(Relation heapRelation, Oid oldIndexId, const char *newName) +index_concurrently_create_copy(Relation heapRelation, Oid oldIndexId, + Oid tablespaceOid, const char *newName) { Relation indexRelation; IndexInfo *oldInfo, @@ -1378,7 +1383,8 @@ index_concurrently_create_copy(Relation heapRelation, Oid oldIndexId, const char newInfo, indexColNames, indexRelation->rd_rel->relam, - indexRelation->rd_rel->reltablespace, + OidIsValid(tablespaceOid) ? + tablespaceOid : indexRelation->rd_rel->reltablespace, indexRelation->rd_indcollation, indclass->values, indcoloptions->values, @@ -3428,10 +3434,12 @@ IndexGetRelation(Oid indexId, bool missing_ok) /* * reindex_index - This routine is used to recreate a single index + * + * See comments of reindex_relation() for details about "tablespaceOid". */ void -reindex_index(Oid indexId, bool skip_constraint_checks, char persistence, - int options) +reindex_index(Oid indexId, Oid tablespaceOid, bool skip_constraint_checks, + char persistence, int options) { Relation iRel, heapRelation; @@ -3440,6 +3448,7 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence, volatile bool skipped_constraint = false; PGRUsage ru0; bool progress = (options & REINDEXOPT_REPORT_PROGRESS) != 0; + bool set_tablespace = OidIsValid(tablespaceOid); pg_rusage_init(&ru0); @@ -3478,6 +3487,35 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence, elog(ERROR, "unsupported relation kind for index \"%s\"", RelationGetRelationName(iRel)); + /* + * We don't support moving system relations into different tablespaces, + * unless allow_system_table_mods=1. + */ + if (set_tablespace && + !allowSystemTableMods && IsSystemRelation(iRel)) + ereport(ERROR, + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), + errmsg("permission denied: \"%s\" is a system catalog", + RelationGetRelationName(iRel)))); + + /* + * We cannot support moving mapped relations into different tablespaces. + * (In particular this eliminates all shared catalogs.) + */ + if (set_tablespace && RelationIsMapped(iRel)) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot change tablespace of mapped relation \"%s\"", + RelationGetRelationName(iRel)))); + + /* It's not a shared catalog, so refuse to move it to shared tablespace */ + if (tablespaceOid == GLOBALTABLESPACE_OID) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot move non-shared relation to tablespace \"%s\"", + get_tablespace_name(tablespaceOid)))); + + /* * Don't allow reindex on temp tables of other backends ... their local * buffer manager is not going to cope. @@ -3504,6 +3542,47 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence, */ CheckTableNotInUse(iRel, "REINDEX INDEX"); + if (tablespaceOid == MyDatabaseTableSpace) + tablespaceOid = InvalidOid; + + /* + * Set the new tablespace for the relation. Do that only in the + * case where the reindex caller wishes to enforce a new tablespace. + */ + if (set_tablespace && + tablespaceOid != iRel->rd_rel->reltablespace) + { + Relation pg_class; + Form_pg_class rd_rel; + HeapTuple tuple; + + /* First get a modifiable copy of the relation's pg_class row */ + pg_class = table_open(RelationRelationId, RowExclusiveLock); + + tuple = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(indexId)); + if (!HeapTupleIsValid(tuple)) + elog(ERROR, "cache lookup failed for relation %u", indexId); + rd_rel = (Form_pg_class) GETSTRUCT(tuple); + + /* + * Mark the relation as ready to be dropped at transaction commit, + * before making visible the new tablespace change so as this won't + * miss things. + */ + RelationDropStorage(iRel); + + /* Update the pg_class row */ + rd_rel->reltablespace = tablespaceOid; + CatalogTupleUpdate(pg_class, &tuple->t_self, tuple); + + heap_freetuple(tuple); + + table_close(pg_class, RowExclusiveLock); + + /* Make sure the reltablespace change is visible */ + CommandCounterIncrement(); + } + /* * All predicate locks on the index are about to be made invalid. Promote * them to relation locks on the heap. @@ -3635,6 +3714,9 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence, * reindex_relation - This routine is used to recreate all indexes * of a relation (and optionally its toast relation too, if any). * + * "tablespaceOid" is the tablespace where the relation's indexes will be + * rebuilt, or InvalidOid to keep each index on its current tablespace. + * * "flags" is a bitmask that can include any combination of these bits: * * REINDEX_REL_PROCESS_TOAST: if true, process the toast table too (if any). @@ -3667,7 +3749,7 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence, * index rebuild. */ bool -reindex_relation(Oid relid, int flags, int options) +reindex_relation(Oid relid, Oid tablespaceOid, int flags, int options) { Relation rel; Oid toast_relid; @@ -3755,7 +3837,8 @@ reindex_relation(Oid relid, int flags, int options) continue; } - reindex_index(indexOid, !(flags & REINDEX_REL_CHECK_CONSTRAINTS), + reindex_index(indexOid, tablespaceOid, + !(flags & REINDEX_REL_CHECK_CONSTRAINTS), persistence, options); CommandCounterIncrement(); @@ -3781,7 +3864,11 @@ reindex_relation(Oid relid, int flags, int options) * still hold the lock on the master table. */ if ((flags & REINDEX_REL_PROCESS_TOAST) && OidIsValid(toast_relid)) - result |= reindex_relation(toast_relid, flags, options); + { + /* Even if table was moved to new tablespace, normally toast cannot move */ + Oid toasttablespaceOid = allowSystemTableMods ? tablespaceOid : InvalidOid; + result |= reindex_relation(toast_relid, toasttablespaceOid, flags, options); + } return result; } diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c index 1817b8829a..90a7dacbe7 100644 --- a/src/backend/commands/cluster.c +++ b/src/backend/commands/cluster.c @@ -1411,7 +1411,7 @@ finish_heap_swap(Oid OIDOldHeap, Oid OIDNewHeap, pgstat_progress_update_param(PROGRESS_CLUSTER_PHASE, PROGRESS_CLUSTER_PHASE_REBUILD_INDEX); - reindex_relation(OIDOldHeap, reindex_flags, 0); + reindex_relation(OIDOldHeap, InvalidOid, reindex_flags, 0); /* Report that we are now doing clean up */ pgstat_progress_update_param(PROGRESS_CLUSTER_PHASE, diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index f5eac5dc8e..b6deb7b805 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -87,7 +87,7 @@ static char *ChooseIndexNameAddition(List *colnames); static List *ChooseIndexColumnNames(List *indexElems); static void RangeVarCallbackForReindexIndex(const RangeVar *relation, Oid relId, Oid oldRelId, void *arg); -static bool ReindexRelationConcurrently(Oid relationOid, int options); +static bool ReindexRelationConcurrently(Oid relationOid, Oid tablespaceOid, int options); static void ReindexPartitionedIndex(Relation parentIdx); static void update_relispartition(Oid relationId, bool newval); static bool CompareOpclassOptions(Datum *opts1, Datum *opts2, int natts); @@ -2425,6 +2425,7 @@ ReindexIndex(ReindexStmt *stmt) RangeVar *indexRelation = stmt->relation; struct ReindexIndexCallbackState state; Oid indOid; + Oid tablespaceOid = InvalidOid; Relation irel; char persistence; @@ -2459,12 +2460,17 @@ ReindexIndex(ReindexStmt *stmt) } persistence = irel->rd_rel->relpersistence; + + /* Define new tablespaceOid if requested */ + if (stmt->tablespacename) + tablespaceOid = get_tablespace_oid(stmt->tablespacename, false); + index_close(irel, NoLock); if (stmt->concurrent && persistence != RELPERSISTENCE_TEMP) - ReindexRelationConcurrently(indOid, stmt->options); + ReindexRelationConcurrently(indOid, tablespaceOid, stmt->options); else - reindex_index(indOid, false, persistence, + reindex_index(indOid, tablespaceOid, false, persistence, stmt->options | REINDEXOPT_REPORT_PROGRESS); } @@ -2548,6 +2554,7 @@ ReindexTable(ReindexStmt *stmt) RangeVar *relation = stmt->relation; Oid heapOid; bool result; + Oid tablespaceOid = InvalidOid; /* * The lock level used here should match reindex_relation(). @@ -2562,9 +2569,13 @@ ReindexTable(ReindexStmt *stmt) 0, RangeVarCallbackOwnsTable, NULL); + /* Define new tablespaceOid if requested */ + if (stmt->tablespacename) + tablespaceOid = get_tablespace_oid(stmt->tablespacename, false); + if (stmt->concurrent && get_rel_persistence(heapOid) != RELPERSISTENCE_TEMP) { - result = ReindexRelationConcurrently(heapOid, stmt->options); + result = ReindexRelationConcurrently(heapOid, tablespaceOid, stmt->options); if (!result) ereport(NOTICE, @@ -2574,6 +2585,7 @@ ReindexTable(ReindexStmt *stmt) else { result = reindex_relation(heapOid, + tablespaceOid, REINDEX_REL_PROCESS_TOAST | REINDEX_REL_CHECK_CONSTRAINTS, stmt->options | REINDEXOPT_REPORT_PROGRESS); @@ -2600,6 +2612,7 @@ ReindexMultipleTables(ReindexStmt *stmt) const char *objectName = stmt->name; ReindexObjectType objectKind = stmt->kind; Oid objectOid; + Oid tablespaceOid = InvalidOid; Relation relationRelation; TableScanDesc scan; ScanKeyData scan_keys[1]; @@ -2610,6 +2623,8 @@ ReindexMultipleTables(ReindexStmt *stmt) ListCell *l; int num_keys; bool concurrent_warning = false; + bool tablespace_warning = false; + bool mapped_warning = false; AssertArg(objectName); Assert(objectKind == REINDEX_OBJECT_SCHEMA || @@ -2648,6 +2663,10 @@ ReindexMultipleTables(ReindexStmt *stmt) objectName); } + /* Define new tablespaceOid if requested */ + if (stmt->tablespacename) + tablespaceOid = get_tablespace_oid(stmt->tablespacename, false); + /* * Create a memory context that will survive forced transaction commits we * do below. Since it is a child of PortalContext, it will go away @@ -2738,6 +2757,35 @@ ReindexMultipleTables(ReindexStmt *stmt) continue; } + if (OidIsValid(tablespaceOid) && + IsSystemClass(relid, classtuple)) + { + if (!allowSystemTableMods) + { + /* Skip all system relations, if not allowSystemTableMods */ + if (!tablespace_warning) + ereport(WARNING, + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), + errmsg("cannot change tablespace of indexes on system relations, skipping all"))); + tablespace_warning = true; + continue; + } + else if (!OidIsValid(classtuple->relfilenode)) + { + /* + * Skip all mapped relations if TABLESPACE is specified. + * OidIsValid(relfilenode) checks that, similar to + * RelationIsMapped(). + */ + if (!mapped_warning) + ereport(WARNING, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot change tablespace of indexes on mapped relations, skipping all"))); + mapped_warning = true; + continue; + } + } + /* Save the list of relation OIDs in private context */ old = MemoryContextSwitchTo(private_context); @@ -2771,7 +2819,7 @@ ReindexMultipleTables(ReindexStmt *stmt) if (stmt->concurrent && get_rel_persistence(relid) != RELPERSISTENCE_TEMP) { - (void) ReindexRelationConcurrently(relid, stmt->options); + (void) ReindexRelationConcurrently(relid, tablespaceOid, stmt->options); /* ReindexRelationConcurrently() does the verbose output */ } else @@ -2779,6 +2827,7 @@ ReindexMultipleTables(ReindexStmt *stmt) bool result; result = reindex_relation(relid, + tablespaceOid, REINDEX_REL_PROCESS_TOAST | REINDEX_REL_CHECK_CONSTRAINTS, stmt->options | REINDEXOPT_REPORT_PROGRESS); @@ -2811,6 +2860,9 @@ ReindexMultipleTables(ReindexStmt *stmt) * itself will be rebuilt. If 'relationOid' belongs to a partitioned table * then we issue a warning to mention these are not yet supported. * + * 'tablespaceOid' is the tablespace where the relation's indexes will be + * rebuilt. + * * The locks taken on parent tables and involved indexes are kept until the * transaction is committed, at which point a session lock is taken on each * relation. Both of these protect against concurrent schema changes. @@ -2825,7 +2877,7 @@ ReindexMultipleTables(ReindexStmt *stmt) * anyway, and a non-concurrent reindex is more efficient. */ static bool -ReindexRelationConcurrently(Oid relationOid, int options) +ReindexRelationConcurrently(Oid relationOid, Oid tablespaceOid, int options) { List *heapRelationIds = NIL; List *indexIds = NIL; @@ -3028,6 +3080,13 @@ ReindexRelationConcurrently(Oid relationOid, int options) break; } + /* It's not a shared catalog, so refuse to move it to shared tablespace */ + if (tablespaceOid == GLOBALTABLESPACE_OID) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot move non-shared relation to tablespace \"%s\"", + get_tablespace_name(tablespaceOid)))); + /* Definitely no indexes, so leave */ if (indexIds == NIL) { @@ -3099,6 +3158,7 @@ ReindexRelationConcurrently(Oid relationOid, int options) /* Create new index definition based on given index */ newIndexId = index_concurrently_create_copy(heapRel, indexId, + tablespaceOid, concurrentName); /* diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 5745cd648a..4bdefcb4b7 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -1873,7 +1873,7 @@ ExecuteTruncateGuts(List *explicit_rels, List *relids, List *relids_logged, /* * Reconstruct the indexes to match, and we're done. */ - reindex_relation(heap_relid, REINDEX_REL_PROCESS_TOAST, 0); + reindex_relation(heap_relid, InvalidOid, REINDEX_REL_PROCESS_TOAST, 0); } pgstat_count_truncate(rel); diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c index ecf16f1190..d4aed7c848 100644 --- a/src/backend/nodes/copyfuncs.c +++ b/src/backend/nodes/copyfuncs.c @@ -4453,6 +4453,7 @@ _copyReindexStmt(const ReindexStmt *from) COPY_SCALAR_FIELD(options); COPY_NODE_FIELD(params); COPY_SCALAR_FIELD(concurrent); + COPY_STRING_FIELD(tablespacename); return newnode; } diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c index 142c3c7d3a..0ff6535611 100644 --- a/src/backend/nodes/equalfuncs.c +++ b/src/backend/nodes/equalfuncs.c @@ -2138,6 +2138,7 @@ _equalReindexStmt(const ReindexStmt *a, const ReindexStmt *b) COMPARE_SCALAR_FIELD(options); COMPARE_NODE_FIELD(params); COMPARE_SCALAR_FIELD(concurrent); + COMPARE_STRING_FIELD(tablespacename); return true; } diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c index 23f4831ace..c9722d390c 100644 --- a/src/backend/tcop/utility.c +++ b/src/backend/tcop/utility.c @@ -542,6 +542,8 @@ void parse_reindex_params(ParseState *pstate, ReindexStmt *stmt) } else if (strcmp(opt->defname, "concurrently") == 0) stmt->concurrent = true; + else if (strcmp(opt->defname, "tablespace") == 0) + stmt->tablespacename = defGetString(opt); else ereport(ERROR, (errcode(ERRCODE_SYNTAX_ERROR), diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c index b2be3aba5c..630fae52df 100644 --- a/src/bin/psql/tab-complete.c +++ b/src/bin/psql/tab-complete.c @@ -3462,7 +3462,9 @@ 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("CONCURRENTLY", "VERBOSE"); + COMPLETE_WITH("CONCURRENTLY", "TABLESPACE", "VERBOSE"); + else if (TailMatches("TABLESPACE")) + COMPLETE_WITH_QUERY(Query_for_list_of_tablespaces); } /* SECURITY LABEL */ diff --git a/src/include/catalog/index.h b/src/include/catalog/index.h index f58e8675f3..d5af4ea89d 100644 --- a/src/include/catalog/index.h +++ b/src/include/catalog/index.h @@ -80,6 +80,7 @@ extern Oid index_create(Relation heapRelation, extern Oid index_concurrently_create_copy(Relation heapRelation, Oid oldIndexId, + Oid tablespaceOid, const char *newName); extern void index_concurrently_build(Oid heapRelationId, @@ -133,7 +134,7 @@ extern void index_set_state_flags(Oid indexId, IndexStateFlagsAction action); extern Oid IndexGetRelation(Oid indexId, bool missing_ok); -extern void reindex_index(Oid indexId, bool skip_constraint_checks, +extern void reindex_index(Oid indexId, Oid tablespaceOid, bool skip_constraint_checks, char relpersistence, int options); /* Flag bits for reindex_relation(): */ @@ -143,7 +144,7 @@ extern void reindex_index(Oid indexId, bool skip_constraint_checks, #define REINDEX_REL_FORCE_INDEXES_UNLOGGED 0x08 #define REINDEX_REL_FORCE_INDEXES_PERMANENT 0x10 -extern bool reindex_relation(Oid relid, int flags, int options); +extern bool reindex_relation(Oid relid, Oid tablespaceOid, int flags, int options); extern bool ReindexIsProcessingHeap(Oid heapOid); extern bool ReindexIsProcessingIndex(Oid indexOid); diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index 490b12890f..6f6134b512 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -3372,6 +3372,7 @@ typedef struct ReindexStmt int options; /* Reindex options flags */ List *params; /* Params not further parsed by the grammer */ bool concurrent; /* reindex concurrently? */ + char *tablespacename; /* name of tablespace where index is to be rebuilt */ } ReindexStmt; /* ---------------------- diff --git a/src/test/regress/input/tablespace.source b/src/test/regress/input/tablespace.source index a5f61a35dc..230cf46833 100644 --- a/src/test/regress/input/tablespace.source +++ b/src/test/regress/input/tablespace.source @@ -17,6 +17,48 @@ ALTER TABLESPACE regress_tblspace SET (some_nonexistent_parameter = true); -- f ALTER TABLESPACE regress_tblspace RESET (random_page_cost = 2.0); -- fail ALTER TABLESPACE regress_tblspace RESET (random_page_cost, effective_io_concurrency); -- ok +-- create table to test REINDEX with TABLESPACE change +CREATE TABLE regress_tblspace_test_tbl (num1 bigint, num2 double precision, num3 double precision); +INSERT INTO regress_tblspace_test_tbl (num1, num2, num3) + SELECT round(random()*100), random(), random()*42 + FROM generate_series(1, 20000) s(i); +CREATE INDEX regress_tblspace_test_tbl_idx ON regress_tblspace_test_tbl (num1); + +-- check that REINDEX with TABLESPACE change is transactional +BEGIN; +REINDEX (TABLESPACE regress_tblspace) INDEX regress_tblspace_test_tbl_idx; +REINDEX (TABLESPACE regress_tblspace) TABLE regress_tblspace_test_tbl; +ROLLBACK; +SELECT relname FROM pg_class +WHERE reltablespace=(SELECT oid FROM pg_tablespace WHERE spcname='regress_tblspace'); + +-- first, let us reindex and move the entire database, after that return everything back +REINDEX (TABLESPACE regress_tblspace) DATABASE regression; -- ok with warning +REINDEX (TABLESPACE pg_default) DATABASE regression; -- ok with warning +SELECT relname FROM pg_class +WHERE reltablespace=(SELECT oid FROM pg_tablespace WHERE spcname='regress_tblspace'); + +-- check REINDEX with TABLESPACE change +REINDEX (TABLESPACE regress_tblspace) INDEX regress_tblspace_test_tbl_idx; -- ok +REINDEX (TABLESPACE regress_tblspace) TABLE regress_tblspace_test_tbl; -- ok +REINDEX (TABLESPACE regress_tblspace) TABLE pg_authid; -- fail +REINDEX (TABLESPACE regress_tblspace) SYSTEM CONCURRENTLY postgres; -- fail +REINDEX (TABLESPACE regress_tblspace) TABLE CONCURRENTLY pg_am; -- fail +REINDEX (TABLESPACE pg_global) INDEX regress_tblspace_test_tbl_idx; -- fail +REINDEX (TABLESPACE regress_tblspace) TABLE pg_am; -- fail + +-- check that all relations moved to new tablespace +SELECT relname FROM pg_class +WHERE reltablespace=(SELECT oid FROM pg_tablespace WHERE spcname='regress_tblspace') +ORDER BY relname; + +-- move indexes back to pg_default tablespace +REINDEX (TABLESPACE pg_default) TABLE CONCURRENTLY regress_tblspace_test_tbl; -- ok + +-- check that all relations moved back to pg_default +SELECT relname FROM pg_class +WHERE reltablespace=(SELECT oid FROM pg_tablespace WHERE spcname='regress_tblspace'); + -- create a schema we can use CREATE SCHEMA testschema; @@ -279,6 +321,9 @@ ALTER TABLE ALL IN TABLESPACE regress_tblspace_renamed SET TABLESPACE pg_default -- Should succeed DROP TABLESPACE regress_tblspace_renamed; +DROP INDEX regress_tblspace_test_tbl_idx; +DROP TABLE regress_tblspace_test_tbl; + DROP SCHEMA testschema CASCADE; DROP ROLE regress_tablespace_user1; diff --git a/src/test/regress/output/tablespace.source b/src/test/regress/output/tablespace.source index 162b591b31..ec5742df98 100644 --- a/src/test/regress/output/tablespace.source +++ b/src/test/regress/output/tablespace.source @@ -20,6 +20,65 @@ ERROR: unrecognized parameter "some_nonexistent_parameter" ALTER TABLESPACE regress_tblspace RESET (random_page_cost = 2.0); -- fail ERROR: RESET must not include values for parameters ALTER TABLESPACE regress_tblspace RESET (random_page_cost, effective_io_concurrency); -- ok +-- create table to test REINDEX with TABLESPACE change +CREATE TABLE regress_tblspace_test_tbl (num1 bigint, num2 double precision, num3 double precision); +INSERT INTO regress_tblspace_test_tbl (num1, num2, num3) + SELECT round(random()*100), random(), random()*42 + FROM generate_series(1, 20000) s(i); +CREATE INDEX regress_tblspace_test_tbl_idx ON regress_tblspace_test_tbl (num1); +-- check that REINDEX with TABLESPACE change is transactional +BEGIN; +REINDEX (TABLESPACE regress_tblspace) INDEX regress_tblspace_test_tbl_idx; +REINDEX (TABLESPACE regress_tblspace) TABLE regress_tblspace_test_tbl; +ROLLBACK; +SELECT relname FROM pg_class +WHERE reltablespace=(SELECT oid FROM pg_tablespace WHERE spcname='regress_tblspace'); + relname +--------- +(0 rows) + +-- first, let us reindex and move the entire database, after that return everything back +REINDEX (TABLESPACE regress_tblspace) DATABASE regression; -- ok with warning +WARNING: cannot change tablespace of indexes on system relations, skipping all +REINDEX (TABLESPACE pg_default) DATABASE regression; -- ok with warning +WARNING: cannot change tablespace of indexes on system relations, skipping all +SELECT relname FROM pg_class +WHERE reltablespace=(SELECT oid FROM pg_tablespace WHERE spcname='regress_tblspace'); + relname +--------- +(0 rows) + +-- check REINDEX with TABLESPACE change +REINDEX (TABLESPACE regress_tblspace) INDEX regress_tblspace_test_tbl_idx; -- ok +REINDEX (TABLESPACE regress_tblspace) TABLE regress_tblspace_test_tbl; -- ok +REINDEX (TABLESPACE regress_tblspace) TABLE pg_authid; -- fail +ERROR: permission denied: "pg_authid_rolname_index" is a system catalog +REINDEX (TABLESPACE regress_tblspace) SYSTEM CONCURRENTLY postgres; -- fail +ERROR: cannot reindex system catalogs concurrently +REINDEX (TABLESPACE regress_tblspace) TABLE CONCURRENTLY pg_am; -- fail +ERROR: cannot reindex system catalogs concurrently +REINDEX (TABLESPACE pg_global) INDEX regress_tblspace_test_tbl_idx; -- fail +ERROR: cannot move non-shared relation to tablespace "pg_global" +REINDEX (TABLESPACE regress_tblspace) TABLE pg_am; -- fail +ERROR: permission denied: "pg_am_name_index" is a system catalog +-- check that all relations moved to new tablespace +SELECT relname FROM pg_class +WHERE reltablespace=(SELECT oid FROM pg_tablespace WHERE spcname='regress_tblspace') +ORDER BY relname; + relname +------------------------------- + regress_tblspace_test_tbl_idx +(1 row) + +-- move indexes back to pg_default tablespace +REINDEX (TABLESPACE pg_default) TABLE CONCURRENTLY regress_tblspace_test_tbl; -- ok +-- check that all relations moved back to pg_default +SELECT relname FROM pg_class +WHERE reltablespace=(SELECT oid FROM pg_tablespace WHERE spcname='regress_tblspace'); + relname +--------- +(0 rows) + -- create a schema we can use CREATE SCHEMA testschema; -- try a table @@ -736,6 +795,8 @@ ALTER TABLE ALL IN TABLESPACE regress_tblspace_renamed SET TABLESPACE pg_default NOTICE: no matching relations in tablespace "regress_tblspace_renamed" found -- Should succeed DROP TABLESPACE regress_tblspace_renamed; +DROP INDEX regress_tblspace_test_tbl_idx; +DROP TABLE regress_tblspace_test_tbl; DROP SCHEMA testschema CASCADE; NOTICE: drop cascades to 6 other objects DETAIL: drop cascades to table testschema.foo -- 2.17.0
>From 12640a8502a6167b04cd9cd83e08519a778e0bf6 Mon Sep 17 00:00:00 2001 From: Alexey Kondratov <kondratov.alek...@gmail.com> Date: Tue, 24 Mar 2020 18:16:06 +0300 Subject: [PATCH v21 4/5] Allow CLUSTER and VACUUM FULL to change tablespace --- doc/src/sgml/ref/cluster.sgml | 19 +++++++ doc/src/sgml/ref/vacuum.sgml | 20 +++++++ src/backend/commands/cluster.c | 63 ++++++++++++++++++++--- src/backend/commands/tablecmds.c | 5 +- src/backend/commands/vacuum.c | 54 +++++++++++++++++-- src/backend/parser/gram.y | 5 +- src/backend/postmaster/autovacuum.c | 1 + src/bin/psql/tab-complete.c | 9 +++- src/include/commands/cluster.h | 2 +- src/include/commands/vacuum.h | 2 + src/test/regress/input/tablespace.source | 23 ++++++++- src/test/regress/output/tablespace.source | 37 ++++++++++++- 12 files changed, 222 insertions(+), 18 deletions(-) diff --git a/doc/src/sgml/ref/cluster.sgml b/doc/src/sgml/ref/cluster.sgml index a6df8a3d81..a824694e68 100644 --- a/doc/src/sgml/ref/cluster.sgml +++ b/doc/src/sgml/ref/cluster.sgml @@ -27,6 +27,7 @@ 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> ] + TABLESPACE <replaceable class="parameter">new_tablespace</replaceable> </synopsis> </refsynopsisdiv> @@ -96,6 +97,15 @@ CLUSTER [VERBOSE] [ ( <replaceable class="parameter">option</replaceable> [, ... </listitem> </varlistentry> + <varlistentry> + <term><literal>TABLESPACE</literal></term> + <listitem> + <para> + Specifies that the table will be rebuilt on a new tablespace. + </para> + </listitem> + </varlistentry> + <varlistentry> <term><replaceable class="parameter">table_name</replaceable></term> <listitem> @@ -128,6 +138,15 @@ CLUSTER [VERBOSE] [ ( <replaceable class="parameter">option</replaceable> [, ... </listitem> </varlistentry> + <varlistentry> + <term><replaceable class="parameter">new_tablespace</replaceable></term> + <listitem> + <para> + The tablespace where the table will be rebuilt. + </para> + </listitem> + </varlistentry> + </variablelist> </refsect1> diff --git a/doc/src/sgml/ref/vacuum.sgml b/doc/src/sgml/ref/vacuum.sgml index a48f75ad7b..2408b59bb2 100644 --- a/doc/src/sgml/ref/vacuum.sgml +++ b/doc/src/sgml/ref/vacuum.sgml @@ -35,6 +35,7 @@ VACUUM [ FULL ] [ FREEZE ] [ VERBOSE ] [ ANALYZE ] [ <replaceable class="paramet INDEX_CLEANUP [ <replaceable class="parameter">boolean</replaceable> ] TRUNCATE [ <replaceable class="parameter">boolean</replaceable> ] PARALLEL <replaceable class="parameter">integer</replaceable> + TABLESPACE <replaceable class="parameter">new_tablespace</replaceable> <phrase>and <replaceable class="parameter">table_and_columns</replaceable> is:</phrase> @@ -255,6 +256,15 @@ VACUUM [ FULL ] [ FREEZE ] [ VERBOSE ] [ ANALYZE ] [ <replaceable class="paramet </listitem> </varlistentry> + <varlistentry> + <term><literal>TABLESPACE</literal></term> + <listitem> + <para> + Specifies that the relation will be rebuilt on a new tablespace. + </para> + </listitem> + </varlistentry> + <varlistentry> <term><replaceable class="parameter">boolean</replaceable></term> <listitem> @@ -299,6 +309,16 @@ VACUUM [ FULL ] [ FREEZE ] [ VERBOSE ] [ ANALYZE ] [ <replaceable class="paramet </para> </listitem> </varlistentry> + + <varlistentry> + <term><replaceable class="parameter">new_tablespace</replaceable></term> + <listitem> + <para> + The tablespace where the relation will be rebuilt. + </para> + </listitem> + </varlistentry> + </variablelist> </refsect1> diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c index 90a7dacbe7..dcb8f83d95 100644 --- a/src/backend/commands/cluster.c +++ b/src/backend/commands/cluster.c @@ -33,11 +33,13 @@ #include "catalog/namespace.h" #include "catalog/objectaccess.h" #include "catalog/pg_am.h" +#include "catalog/pg_tablespace.h" #include "catalog/toasting.h" #include "commands/cluster.h" #include "commands/defrem.h" #include "commands/progress.h" #include "commands/tablecmds.h" +#include "commands/tablespace.h" #include "commands/vacuum.h" #include "miscadmin.h" #include "optimizer/optimizer.h" @@ -68,7 +70,7 @@ typedef struct } RelToCluster; -static void rebuild_relation(Relation OldHeap, Oid indexOid, bool verbose); +static void rebuild_relation(Relation OldHeap, Oid indexOid, Oid NewTableSpaceOid, bool verbose); static void copy_table_data(Oid OIDNewHeap, Oid OIDOldHeap, Oid OIDOldIndex, bool verbose, bool *pSwapToastByContent, TransactionId *pFreezeXid, MultiXactId *pCutoffMulti); @@ -103,6 +105,9 @@ void cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel) { ListCell *lc; + /* Name and Oid of tablespace to use for clustered relation. */ + char *tablespaceName = NULL; + Oid tablespaceOid = InvalidOid; /* Parse list of generic parameters not handled by the parser */ foreach(lc, stmt->params) @@ -114,6 +119,8 @@ cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel) stmt->options |= CLUOPT_VERBOSE; else stmt->options &= ~CLUOPT_VERBOSE; + else if (strcmp(opt->defname, "tablespace") == 0) + tablespaceName = defGetString(opt); else ereport(ERROR, (errcode(ERRCODE_SYNTAX_ERROR), @@ -122,6 +129,19 @@ cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel) parser_errposition(pstate, opt->location))); } + /* Select tablespace Oid to use. */ + if (tablespaceName) + { + tablespaceOid = get_tablespace_oid(tablespaceName, false); + + /* Can't move a non-shared relation into pg_global */ + if (tablespaceOid == GLOBALTABLESPACE_OID) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot move non-shared relation to tablespace \"%s\"", + tablespaceName))); + } + if (stmt->relation != NULL) { /* This is the single-relation case. */ @@ -191,7 +211,7 @@ cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel) table_close(rel, NoLock); /* Do the job. */ - cluster_rel(tableOid, indexOid, stmt->options); + cluster_rel(tableOid, indexOid, tablespaceOid, stmt->options); } else { @@ -239,7 +259,7 @@ cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel) /* functions in indexes may want a snapshot set */ PushActiveSnapshot(GetTransactionSnapshot()); /* Do the job. */ - cluster_rel(rvtc->tableOid, rvtc->indexOid, + cluster_rel(rvtc->tableOid, rvtc->indexOid, tablespaceOid, stmt->options | CLUOPT_RECHECK); PopActiveSnapshot(); CommitTransactionCommand(); @@ -269,9 +289,12 @@ cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel) * If indexOid is InvalidOid, the table will be rewritten in physical order * instead of index order. This is the new implementation of VACUUM FULL, * and error messages should refer to the operation as VACUUM not CLUSTER. + * + * "tablespaceOid" is the tablespace where the relation will be rebuilt, or + * InvalidOid to use its current tablespace. */ void -cluster_rel(Oid tableOid, Oid indexOid, int options) +cluster_rel(Oid tableOid, Oid indexOid, Oid tablespaceOid, int options) { Relation OldHeap; bool verbose = ((options & CLUOPT_VERBOSE) != 0); @@ -371,6 +394,23 @@ cluster_rel(Oid tableOid, Oid indexOid, int options) (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("cannot cluster a shared catalog"))); + if (OidIsValid(tablespaceOid) && + !allowSystemTableMods && IsSystemRelation(OldHeap)) + ereport(ERROR, + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), + errmsg("permission denied: \"%s\" is a system catalog", + RelationGetRelationName(OldHeap)))); + + /* + * We cannot support moving mapped relations into different tablespaces. + * (In particular this eliminates all shared catalogs.) + */ + if (OidIsValid(tablespaceOid) && RelationIsMapped(OldHeap)) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot change tablespace of mapped relation \"%s\"", + RelationGetRelationName(OldHeap)))); + /* * Don't process temp tables of other backends ... their local buffer * manager is not going to cope. @@ -421,7 +461,7 @@ cluster_rel(Oid tableOid, Oid indexOid, int options) TransferPredicateLocksToHeapRelation(OldHeap); /* rebuild_relation does all the dirty work */ - rebuild_relation(OldHeap, indexOid, verbose); + rebuild_relation(OldHeap, indexOid, tablespaceOid, verbose); /* NB: rebuild_relation does table_close() on OldHeap */ @@ -570,7 +610,7 @@ mark_index_clustered(Relation rel, Oid indexOid, bool is_internal) * NB: this routine closes OldHeap at the right time; caller should not. */ static void -rebuild_relation(Relation OldHeap, Oid indexOid, bool verbose) +rebuild_relation(Relation OldHeap, Oid indexOid, Oid NewTablespaceOid, bool verbose) { Oid tableOid = RelationGetRelid(OldHeap); Oid tableSpace = OldHeap->rd_rel->reltablespace; @@ -581,6 +621,10 @@ rebuild_relation(Relation OldHeap, Oid indexOid, bool verbose) TransactionId frozenXid; MultiXactId cutoffMulti; + /* Use new tablespace if passed. */ + if (OidIsValid(NewTablespaceOid)) + tableSpace = NewTablespaceOid; + /* Mark the correct index as clustered */ if (OidIsValid(indexOid)) mark_index_clustered(OldHeap, indexOid, true); @@ -1025,6 +1069,13 @@ swap_relation_files(Oid r1, Oid r2, bool target_is_pg_class, */ Assert(!target_is_pg_class); + if (!allowSystemTableMods && IsSystemClass(r1, relform1) && + relform1->reltablespace != relform2->reltablespace) + ereport(ERROR, + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), + errmsg("permission denied: \"%s\" is a system catalog", + get_rel_name(r1)))); + swaptemp = relform1->relfilenode; relform1->relfilenode = relform2->relfilenode; relform2->relfilenode = swaptemp; diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 4bdefcb4b7..b01e1aa8bf 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -12963,8 +12963,9 @@ ATExecSetTableSpace(Oid tableOid, Oid newTableSpace, LOCKMODE lockmode) if (RelationIsMapped(rel)) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("cannot move system relation \"%s\"", - RelationGetRelationName(rel)))); + errmsg("cannot change tablespace of mapped relation \"%s\"", + RelationGetRelationName(rel)))); + /* Can't move a non-shared relation into pg_global */ if (newTableSpace == GLOBALTABLESPACE_OID) diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c index 5a110edb07..e9373e5dae 100644 --- a/src/backend/commands/vacuum.c +++ b/src/backend/commands/vacuum.c @@ -31,13 +31,16 @@ #include "access/tableam.h" #include "access/transam.h" #include "access/xact.h" +#include "catalog/catalog.h" #include "catalog/namespace.h" #include "catalog/pg_database.h" #include "catalog/pg_inherits.h" #include "catalog/pg_namespace.h" +#include "catalog/pg_tablespace.h" #include "commands/cluster.h" #include "commands/defrem.h" #include "commands/vacuum.h" +#include "commands/tablespace.h" #include "miscadmin.h" #include "nodes/makefuncs.h" #include "pgstat.h" @@ -106,6 +109,10 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel) bool disable_page_skipping = false; ListCell *lc; + /* Name and Oid of tablespace to use for relations after VACUUM FULL. */ + char *tablespacename = NULL; + Oid tablespaceOid = InvalidOid; + /* Set default value */ params.index_cleanup = VACOPT_TERNARY_DEFAULT; params.truncate = VACOPT_TERNARY_DEFAULT; @@ -142,6 +149,8 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel) params.index_cleanup = get_vacopt_ternary_value(opt); else if (strcmp(opt->defname, "truncate") == 0) params.truncate = get_vacopt_ternary_value(opt); + else if (strcmp(opt->defname, "tablespace") == 0) + tablespacename = defGetString(opt); else if (strcmp(opt->defname, "parallel") == 0) { if (opt->arg == NULL) @@ -202,6 +211,27 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel) (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("VACUUM FULL cannot be performed in parallel"))); + /* Get tablespace Oid to use. */ + if (tablespacename) + { + if ((params.options & VACOPT_FULL) == 0) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("incompatible TABLESPACE option"), + errdetail("You can only use TABLESPACE with VACUUM FULL."))); + + tablespaceOid = get_tablespace_oid(tablespacename, false); + + /* Can't move a non-shared relation into pg_global */ + if (tablespaceOid == GLOBALTABLESPACE_OID) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot move non-shared relation to tablespace \"%s\"", + tablespacename))); + + } + params.tablespace_oid = tablespaceOid; + /* * Make sure VACOPT_ANALYZE is specified if any column lists are present. */ @@ -1670,8 +1700,9 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params) LOCKMODE lmode; Relation onerel; LockRelId onerelid; - Oid toast_relid; - Oid save_userid; + Oid toast_relid, + save_userid, + tablespaceOid = InvalidOid; int save_sec_context; int save_nestlevel; @@ -1805,6 +1836,23 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params) return true; } + /* + * We cannot support moving system relations into different tablespaces, + * unless allow_system_table_mods=1. + */ + if (params->options & VACOPT_FULL && + OidIsValid(params->tablespace_oid) && + IsSystemRelation(onerel) && !allowSystemTableMods) + { + ereport(WARNING, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("skipping tablespace change of \"%s\"", + RelationGetRelationName(onerel)), + errdetail("Cannot move system relation, only VACUUM is performed."))); + } + else + tablespaceOid = params->tablespace_oid; + /* * Get a session-level lock too. This will protect our access to the * relation across multiple transactions, so that we can vacuum the @@ -1874,7 +1922,7 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params) cluster_options |= CLUOPT_VERBOSE; /* VACUUM FULL is now a variant of CLUSTER; see cluster.c */ - cluster_rel(relid, InvalidOid, cluster_options); + cluster_rel(relid, InvalidOid, tablespaceOid, cluster_options); } else table_relation_vacuum(onerel, params, vac_strategy); diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index 6fb4d814cc..93b35378d0 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -10676,8 +10676,9 @@ cluster_index_specification: /***************************************************************************** * * QUERY: - * VACUUM - * ANALYZE + * VACUUM [FULL] [FREEZE] [VERBOSE] [ANALYZE] [ <table_and_columns> [, ...] ] + * VACUUM [(options)] [ <table_and_columns> [, ...] ] + * ANALYZE [VERBOSE] [ <table_and_columns> [, ...] ] * *****************************************************************************/ diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c index 7e97ffab27..8acc321faa 100644 --- a/src/backend/postmaster/autovacuum.c +++ b/src/backend/postmaster/autovacuum.c @@ -2897,6 +2897,7 @@ table_recheck_autovac(Oid relid, HTAB *table_toast_map, tab->at_params.multixact_freeze_table_age = multixact_freeze_table_age; tab->at_params.is_wraparound = wraparound; tab->at_params.log_min_duration = log_min_duration; + tab->at_params.tablespace_oid = InvalidOid; tab->at_vacuum_cost_limit = vac_cost_limit; tab->at_vacuum_cost_delay = vac_cost_delay; tab->at_relname = NULL; diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c index 630fae52df..48018507dc 100644 --- a/src/bin/psql/tab-complete.c +++ b/src/bin/psql/tab-complete.c @@ -2279,7 +2279,9 @@ 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("TABLESPACE|VERBOSE"); + else if (TailMatches("TABLESPACE")) + COMPLETE_WITH_QUERY(Query_for_list_of_tablespaces); } /* COMMENT */ @@ -3691,9 +3693,12 @@ psql_completion(const char *text, int start, int end) if (ends_with(prev_wd, '(') || ends_with(prev_wd, ',')) COMPLETE_WITH("FULL", "FREEZE", "ANALYZE", "VERBOSE", "DISABLE_PAGE_SKIPPING", "SKIP_LOCKED", - "INDEX_CLEANUP", "TRUNCATE", "PARALLEL"); + "INDEX_CLEANUP", "TRUNCATE", "PARALLEL", + "TABLESPACE"); else if (TailMatches("FULL|FREEZE|ANALYZE|VERBOSE|DISABLE_PAGE_SKIPPING|SKIP_LOCKED|INDEX_CLEANUP|TRUNCATE")) COMPLETE_WITH("ON", "OFF"); + else if (TailMatches("TABLESPACE")) + COMPLETE_WITH_QUERY(Query_for_list_of_tablespaces); } else if (HeadMatches("VACUUM") && TailMatches("(")) /* "VACUUM (" should be caught above, so assume we want columns */ diff --git a/src/include/commands/cluster.h b/src/include/commands/cluster.h index 674cdcd0cd..bc9f881d8c 100644 --- a/src/include/commands/cluster.h +++ b/src/include/commands/cluster.h @@ -20,7 +20,7 @@ extern void cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel); -extern void cluster_rel(Oid tableOid, Oid indexOid, int options); +extern void cluster_rel(Oid tableOid, Oid indexOid, Oid tablespaceOid, int options); extern void check_index_is_clusterable(Relation OldHeap, Oid indexOid, bool recheck, LOCKMODE lockmode); extern void mark_index_clustered(Relation rel, Oid indexOid, bool is_internal); diff --git a/src/include/commands/vacuum.h b/src/include/commands/vacuum.h index a4cd721400..4b5ac7145d 100644 --- a/src/include/commands/vacuum.h +++ b/src/include/commands/vacuum.h @@ -229,6 +229,8 @@ typedef struct VacuumParams * disabled. */ int nworkers; + Oid tablespace_oid; /* tablespace to use for relations + * rebuilt by VACUUM FULL */ } VacuumParams; /* GUC parameters */ diff --git a/src/test/regress/input/tablespace.source b/src/test/regress/input/tablespace.source index 230cf46833..eb9dfcc0f4 100644 --- a/src/test/regress/input/tablespace.source +++ b/src/test/regress/input/tablespace.source @@ -17,7 +17,7 @@ ALTER TABLESPACE regress_tblspace SET (some_nonexistent_parameter = true); -- f ALTER TABLESPACE regress_tblspace RESET (random_page_cost = 2.0); -- fail ALTER TABLESPACE regress_tblspace RESET (random_page_cost, effective_io_concurrency); -- ok --- create table to test REINDEX with TABLESPACE change +-- create table to test REINDEX, CLUSTER and VACUUM FULL with TABLESPACE change CREATE TABLE regress_tblspace_test_tbl (num1 bigint, num2 double precision, num3 double precision); INSERT INTO regress_tblspace_test_tbl (num1, num2, num3) SELECT round(random()*100), random(), random()*42 @@ -47,11 +47,32 @@ REINDEX (TABLESPACE regress_tblspace) TABLE CONCURRENTLY pg_am; -- fail REINDEX (TABLESPACE pg_global) INDEX regress_tblspace_test_tbl_idx; -- fail REINDEX (TABLESPACE regress_tblspace) TABLE pg_am; -- fail +-- check that all indexes moved to new tablespace +SELECT relname FROM pg_class +WHERE reltablespace=(SELECT oid FROM pg_tablespace WHERE spcname='regress_tblspace') +ORDER BY relname; + +-- check CLUSTER with TABLESPACE change +CLUSTER (TABLESPACE regress_tblspace) regress_tblspace_test_tbl USING regress_tblspace_test_tbl_idx; -- ok +CLUSTER (TABLESPACE regress_tblspace) pg_authid USING pg_authid_rolname_index; -- fail +CLUSTER (TABLESPACE pg_global) regress_tblspace_test_tbl USING regress_tblspace_test_tbl_idx; -- fail + -- check that all relations moved to new tablespace SELECT relname FROM pg_class WHERE reltablespace=(SELECT oid FROM pg_tablespace WHERE spcname='regress_tblspace') ORDER BY relname; +-- check VACUUM with TABLESPACE change +VACUUM (FULL, ANALYSE, FREEZE, TABLESPACE pg_default) regress_tblspace_test_tbl; -- ok +VACUUM (FULL, TABLESPACE pg_default) pg_authid; -- skip with warning +VACUUM (ANALYSE, TABLESPACE pg_default); -- fail +VACUUM (FULL, TABLESPACE pg_global) regress_tblspace_test_tbl; -- fail + +-- check that all tables moved back to pg_default +SELECT relname FROM pg_class +WHERE reltablespace=(SELECT oid FROM pg_tablespace WHERE spcname='regress_tblspace') +ORDER BY relname; + -- move indexes back to pg_default tablespace REINDEX (TABLESPACE pg_default) TABLE CONCURRENTLY regress_tblspace_test_tbl; -- ok diff --git a/src/test/regress/output/tablespace.source b/src/test/regress/output/tablespace.source index ec5742df98..789a0e56cc 100644 --- a/src/test/regress/output/tablespace.source +++ b/src/test/regress/output/tablespace.source @@ -20,7 +20,7 @@ ERROR: unrecognized parameter "some_nonexistent_parameter" ALTER TABLESPACE regress_tblspace RESET (random_page_cost = 2.0); -- fail ERROR: RESET must not include values for parameters ALTER TABLESPACE regress_tblspace RESET (random_page_cost, effective_io_concurrency); -- ok --- create table to test REINDEX with TABLESPACE change +-- create table to test REINDEX, CLUSTER and VACUUM FULL with TABLESPACE change CREATE TABLE regress_tblspace_test_tbl (num1 bigint, num2 double precision, num3 double precision); INSERT INTO regress_tblspace_test_tbl (num1, num2, num3) SELECT round(random()*100), random(), random()*42 @@ -61,9 +61,44 @@ REINDEX (TABLESPACE pg_global) INDEX regress_tblspace_test_tbl_idx; -- fail ERROR: cannot move non-shared relation to tablespace "pg_global" REINDEX (TABLESPACE regress_tblspace) TABLE pg_am; -- fail ERROR: permission denied: "pg_am_name_index" is a system catalog +-- check that all indexes moved to new tablespace +SELECT relname FROM pg_class +WHERE reltablespace=(SELECT oid FROM pg_tablespace WHERE spcname='regress_tblspace') +ORDER BY relname; + relname +------------------------------- + regress_tblspace_test_tbl_idx +(1 row) + +-- check CLUSTER with TABLESPACE change +CLUSTER (TABLESPACE regress_tblspace) regress_tblspace_test_tbl USING regress_tblspace_test_tbl_idx; -- ok +CLUSTER (TABLESPACE regress_tblspace) pg_authid USING pg_authid_rolname_index; -- fail +ERROR: cannot cluster a shared catalog +CLUSTER (TABLESPACE pg_global) regress_tblspace_test_tbl USING regress_tblspace_test_tbl_idx; -- fail +ERROR: cannot move non-shared relation to tablespace "pg_global" -- check that all relations moved to new tablespace SELECT relname FROM pg_class WHERE reltablespace=(SELECT oid FROM pg_tablespace WHERE spcname='regress_tblspace') +ORDER BY relname; + relname +------------------------------- + regress_tblspace_test_tbl + regress_tblspace_test_tbl_idx +(2 rows) + +-- check VACUUM with TABLESPACE change +VACUUM (FULL, ANALYSE, FREEZE, TABLESPACE pg_default) regress_tblspace_test_tbl; -- ok +VACUUM (FULL, TABLESPACE pg_default) pg_authid; -- skip with warning +WARNING: skipping tablespace change of "pg_authid" +DETAIL: Cannot move system relation, only VACUUM is performed. +VACUUM (ANALYSE, TABLESPACE pg_default); -- fail +ERROR: incompatible TABLESPACE option +DETAIL: You can only use TABLESPACE with VACUUM FULL. +VACUUM (FULL, TABLESPACE pg_global) regress_tblspace_test_tbl; -- fail +ERROR: cannot move non-shared relation to tablespace "pg_global" +-- check that all tables moved back to pg_default +SELECT relname FROM pg_class +WHERE reltablespace=(SELECT oid FROM pg_tablespace WHERE spcname='regress_tblspace') ORDER BY relname; relname ------------------------------- -- 2.17.0
>From ec4fea351f2d7c06b8b43e5d3f33990cb066a798 Mon Sep 17 00:00:00 2001 From: Justin Pryzby <pryz...@telsasoft.com> Date: Tue, 31 Mar 2020 20:35:41 -0500 Subject: [PATCH v21 5/5] Implement vacuum full/cluster (INDEX_TABLESPACE <tablespace>) --- doc/src/sgml/ref/cluster.sgml | 12 ++++- doc/src/sgml/ref/vacuum.sgml | 12 ++++- src/backend/commands/cluster.c | 64 ++++++++++++++--------- src/backend/commands/matview.c | 3 +- src/backend/commands/tablecmds.c | 2 +- src/backend/commands/vacuum.c | 47 +++++++---------- src/backend/postmaster/autovacuum.c | 1 + src/include/commands/cluster.h | 5 +- src/include/commands/vacuum.h | 5 +- src/test/regress/input/tablespace.source | 13 +++++ src/test/regress/output/tablespace.source | 20 +++++++ 11 files changed, 123 insertions(+), 61 deletions(-) diff --git a/doc/src/sgml/ref/cluster.sgml b/doc/src/sgml/ref/cluster.sgml index a824694e68..ac20db012f 100644 --- a/doc/src/sgml/ref/cluster.sgml +++ b/doc/src/sgml/ref/cluster.sgml @@ -28,6 +28,7 @@ CLUSTER [VERBOSE] [ ( <replaceable class="parameter">option</replaceable> [, ... VERBOSE [ <replaceable class="parameter">boolean</replaceable> ] TABLESPACE <replaceable class="parameter">new_tablespace</replaceable> + INDEX_TABLESPACE <replaceable class="parameter">new_tablespace</replaceable> </synopsis> </refsynopsisdiv> @@ -106,6 +107,15 @@ CLUSTER [VERBOSE] [ ( <replaceable class="parameter">option</replaceable> [, ... </listitem> </varlistentry> + <varlistentry> + <term><literal>INDEX_TABLESPACE</literal></term> + <listitem> + <para> + Specifies that the table's indexes will be rebuilt on a new tablespace. + </para> + </listitem> + </varlistentry> + <varlistentry> <term><replaceable class="parameter">table_name</replaceable></term> <listitem> @@ -142,7 +152,7 @@ CLUSTER [VERBOSE] [ ( <replaceable class="parameter">option</replaceable> [, ... <term><replaceable class="parameter">new_tablespace</replaceable></term> <listitem> <para> - The tablespace where the table will be rebuilt. + The tablespace where the table or its indexes will be rebuilt. </para> </listitem> </varlistentry> diff --git a/doc/src/sgml/ref/vacuum.sgml b/doc/src/sgml/ref/vacuum.sgml index 2408b59bb2..6461746968 100644 --- a/doc/src/sgml/ref/vacuum.sgml +++ b/doc/src/sgml/ref/vacuum.sgml @@ -36,6 +36,7 @@ VACUUM [ FULL ] [ FREEZE ] [ VERBOSE ] [ ANALYZE ] [ <replaceable class="paramet TRUNCATE [ <replaceable class="parameter">boolean</replaceable> ] PARALLEL <replaceable class="parameter">integer</replaceable> TABLESPACE <replaceable class="parameter">new_tablespace</replaceable> + INDEX_TABLESPACE <replaceable class="parameter">new_tablespace</replaceable> <phrase>and <replaceable class="parameter">table_and_columns</replaceable> is:</phrase> @@ -265,6 +266,15 @@ VACUUM [ FULL ] [ FREEZE ] [ VERBOSE ] [ ANALYZE ] [ <replaceable class="paramet </listitem> </varlistentry> + <varlistentry> + <term><literal>INDEX_TABLESPACE</literal></term> + <listitem> + <para> + Specifies that the relation's indexes will be rebuilt on a new tablespace. + </para> + </listitem> + </varlistentry> + <varlistentry> <term><replaceable class="parameter">boolean</replaceable></term> <listitem> @@ -314,7 +324,7 @@ VACUUM [ FULL ] [ FREEZE ] [ VERBOSE ] [ ANALYZE ] [ <replaceable class="paramet <term><replaceable class="parameter">new_tablespace</replaceable></term> <listitem> <para> - The tablespace where the relation will be rebuilt. + The tablespace where the relation or its indexes will be rebuilt. </para> </listitem> </varlistentry> diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c index dcb8f83d95..e9781f3ab9 100644 --- a/src/backend/commands/cluster.c +++ b/src/backend/commands/cluster.c @@ -70,7 +70,7 @@ typedef struct } RelToCluster; -static void rebuild_relation(Relation OldHeap, Oid indexOid, Oid NewTableSpaceOid, bool verbose); +static void rebuild_relation(Relation OldHeap, Oid indexOid, Oid NewTableSpaceOid, Oid NewIdxTableSpaceOid, bool verbose); static void copy_table_data(Oid OIDNewHeap, Oid OIDOldHeap, Oid OIDOldIndex, bool verbose, bool *pSwapToastByContent, TransactionId *pFreezeXid, MultiXactId *pCutoffMulti); @@ -105,9 +105,11 @@ void cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel) { ListCell *lc; - /* Name and Oid of tablespace to use for clustered relation. */ - char *tablespaceName = NULL; - Oid tablespaceOid = InvalidOid; + /* Name and Oid of tablespaces to use for clustered relations. */ + char *tablespaceName = NULL, + *idxtablespaceName = NULL; + Oid tablespaceOid; + Oid idxtablespaceOid; /* Parse list of generic parameters not handled by the parser */ foreach(lc, stmt->params) @@ -121,6 +123,8 @@ cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel) stmt->options &= ~CLUOPT_VERBOSE; else if (strcmp(opt->defname, "tablespace") == 0) tablespaceName = defGetString(opt); + else if (strcmp(opt->defname, "index_tablespace") == 0) + idxtablespaceName = defGetString(opt); else ereport(ERROR, (errcode(ERRCODE_SYNTAX_ERROR), @@ -129,18 +133,11 @@ cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel) parser_errposition(pstate, opt->location))); } - /* Select tablespace Oid to use. */ - if (tablespaceName) - { - tablespaceOid = get_tablespace_oid(tablespaceName, false); - - /* Can't move a non-shared relation into pg_global */ - if (tablespaceOid == GLOBALTABLESPACE_OID) - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("cannot move non-shared relation to tablespace \"%s\"", - tablespaceName))); - } + /* Get tablespaces to use. */ + tablespaceOid = tablespaceName ? + get_tablespace_oid(tablespaceName, false) : InvalidOid; + idxtablespaceOid = idxtablespaceName ? + get_tablespace_oid(idxtablespaceName, false) : InvalidOid; if (stmt->relation != NULL) { @@ -211,7 +208,7 @@ cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel) table_close(rel, NoLock); /* Do the job. */ - cluster_rel(tableOid, indexOid, tablespaceOid, stmt->options); + cluster_rel(tableOid, indexOid, tablespaceOid, idxtablespaceOid, stmt->options); } else { @@ -260,7 +257,7 @@ cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel) PushActiveSnapshot(GetTransactionSnapshot()); /* Do the job. */ cluster_rel(rvtc->tableOid, rvtc->indexOid, tablespaceOid, - stmt->options | CLUOPT_RECHECK); + idxtablespaceOid, stmt->options | CLUOPT_RECHECK); PopActiveSnapshot(); CommitTransactionCommand(); } @@ -290,11 +287,12 @@ cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel) * instead of index order. This is the new implementation of VACUUM FULL, * and error messages should refer to the operation as VACUUM not CLUSTER. * - * "tablespaceOid" is the tablespace where the relation will be rebuilt, or - * InvalidOid to use its current tablespace. + * "tablespaceOid" and "idxtablespaceOid" are the tablespaces where the relation + * and its indexes will be rebuilt, or InvalidOid to use their current + * tablespaces. */ void -cluster_rel(Oid tableOid, Oid indexOid, Oid tablespaceOid, int options) +cluster_rel(Oid tableOid, Oid indexOid, Oid tablespaceOid, Oid idxtablespaceOid, int options) { Relation OldHeap; bool verbose = ((options & CLUOPT_VERBOSE) != 0); @@ -461,7 +459,7 @@ cluster_rel(Oid tableOid, Oid indexOid, Oid tablespaceOid, int options) TransferPredicateLocksToHeapRelation(OldHeap); /* rebuild_relation does all the dirty work */ - rebuild_relation(OldHeap, indexOid, tablespaceOid, verbose); + rebuild_relation(OldHeap, indexOid, tablespaceOid, idxtablespaceOid, verbose); /* NB: rebuild_relation does table_close() on OldHeap */ @@ -610,10 +608,11 @@ mark_index_clustered(Relation rel, Oid indexOid, bool is_internal) * NB: this routine closes OldHeap at the right time; caller should not. */ static void -rebuild_relation(Relation OldHeap, Oid indexOid, Oid NewTablespaceOid, bool verbose) +rebuild_relation(Relation OldHeap, Oid indexOid, Oid NewTablespaceOid, Oid NewIdxTablespaceOid, bool verbose) { Oid tableOid = RelationGetRelid(OldHeap); Oid tableSpace = OldHeap->rd_rel->reltablespace; + Oid idxtableSpace; Oid OIDNewHeap; char relpersistence; bool is_system_catalog; @@ -623,7 +622,20 @@ rebuild_relation(Relation OldHeap, Oid indexOid, Oid NewTablespaceOid, bool verb /* Use new tablespace if passed. */ if (OidIsValid(NewTablespaceOid)) + { tableSpace = NewTablespaceOid; + /* It's not a shared catalog, so refuse to move it to shared tablespace */ + if (tableSpace == GLOBALTABLESPACE_OID) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot move non-shared relation to tablespace \"%s\"", + get_tablespace_name(tableSpace)))); + } + + if (OidIsValid(NewIdxTablespaceOid)) + idxtableSpace = NewIdxTablespaceOid; + else + idxtableSpace = get_rel_tablespace(indexOid); /* Mark the correct index as clustered */ if (OidIsValid(indexOid)) @@ -652,7 +664,7 @@ rebuild_relation(Relation OldHeap, Oid indexOid, Oid NewTablespaceOid, bool verb finish_heap_swap(tableOid, OIDNewHeap, is_system_catalog, swap_toast_by_content, false, true, frozenXid, cutoffMulti, - relpersistence); + relpersistence, idxtableSpace); } @@ -1400,7 +1412,7 @@ finish_heap_swap(Oid OIDOldHeap, Oid OIDNewHeap, bool is_internal, TransactionId frozenXid, MultiXactId cutoffMulti, - char newrelpersistence) + char newrelpersistence, Oid idxtableSpace) { ObjectAddress object; Oid mapped_tables[4]; @@ -1462,7 +1474,7 @@ finish_heap_swap(Oid OIDOldHeap, Oid OIDNewHeap, pgstat_progress_update_param(PROGRESS_CLUSTER_PHASE, PROGRESS_CLUSTER_PHASE_REBUILD_INDEX); - reindex_relation(OIDOldHeap, InvalidOid, reindex_flags, 0); + reindex_relation(OIDOldHeap, idxtableSpace, reindex_flags, 0); /* Report that we are now doing clean up */ pgstat_progress_update_param(PROGRESS_CLUSTER_PHASE, diff --git a/src/backend/commands/matview.c b/src/backend/commands/matview.c index f80a9e96a9..68ea07cae5 100644 --- a/src/backend/commands/matview.c +++ b/src/backend/commands/matview.c @@ -844,7 +844,8 @@ static void refresh_by_heap_swap(Oid matviewOid, Oid OIDNewHeap, char relpersistence) { finish_heap_swap(matviewOid, OIDNewHeap, false, false, true, true, - RecentXmin, ReadNextMultiXactId(), relpersistence); + RecentXmin, ReadNextMultiXactId(), relpersistence, + InvalidOid); } /* diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index b01e1aa8bf..aef8cbea41 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -4918,7 +4918,7 @@ ATRewriteTables(AlterTableStmt *parsetree, List **wqueue, LOCKMODE lockmode, !OidIsValid(tab->newTableSpace), RecentXmin, ReadNextMultiXactId(), - persistence); + persistence, InvalidOid); } else { diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c index e9373e5dae..e191f19b0b 100644 --- a/src/backend/commands/vacuum.c +++ b/src/backend/commands/vacuum.c @@ -109,9 +109,9 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel) bool disable_page_skipping = false; ListCell *lc; - /* Name and Oid of tablespace to use for relations after VACUUM FULL. */ - char *tablespacename = NULL; - Oid tablespaceOid = InvalidOid; + /* Tablespace to use for relations after VACUUM FULL. */ + char *tablespacename = NULL, + *idxtablespacename = NULL; /* Set default value */ params.index_cleanup = VACOPT_TERNARY_DEFAULT; @@ -151,6 +151,8 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel) params.truncate = get_vacopt_ternary_value(opt); else if (strcmp(opt->defname, "tablespace") == 0) tablespacename = defGetString(opt); + else if (strcmp(opt->defname, "index_tablespace") == 0) + idxtablespacename = defGetString(opt); else if (strcmp(opt->defname, "parallel") == 0) { if (opt->arg == NULL) @@ -211,26 +213,18 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel) (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("VACUUM FULL cannot be performed in parallel"))); - /* Get tablespace Oid to use. */ - if (tablespacename) - { - if ((params.options & VACOPT_FULL) == 0) - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("incompatible TABLESPACE option"), - errdetail("You can only use TABLESPACE with VACUUM FULL."))); - - tablespaceOid = get_tablespace_oid(tablespacename, false); - - /* Can't move a non-shared relation into pg_global */ - if (tablespaceOid == GLOBALTABLESPACE_OID) - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("cannot move non-shared relation to tablespace \"%s\"", - tablespacename))); + if ((params.options & VACOPT_FULL) == 0 && + (tablespacename || idxtablespacename)) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("incompatible TABLESPACE option"), + errdetail("You can only use TABLESPACE with VACUUM FULL."))); - } - params.tablespace_oid = tablespaceOid; + /* Get tablespace Oids to use. */ + params.tablespace_oid = tablespacename ? + get_tablespace_oid(tablespacename, false) : InvalidOid; + params.idxtablespace_oid = idxtablespacename ? + get_tablespace_oid(idxtablespacename, false) : InvalidOid; /* * Make sure VACOPT_ANALYZE is specified if any column lists are present. @@ -1701,8 +1695,7 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params) Relation onerel; LockRelId onerelid; Oid toast_relid, - save_userid, - tablespaceOid = InvalidOid; + save_userid; int save_sec_context; int save_nestlevel; @@ -1844,14 +1837,13 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params) OidIsValid(params->tablespace_oid) && IsSystemRelation(onerel) && !allowSystemTableMods) { + params->tablespace_oid = InvalidOid; ereport(WARNING, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("skipping tablespace change of \"%s\"", RelationGetRelationName(onerel)), errdetail("Cannot move system relation, only VACUUM is performed."))); } - else - tablespaceOid = params->tablespace_oid; /* * Get a session-level lock too. This will protect our access to the @@ -1922,7 +1914,8 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params) cluster_options |= CLUOPT_VERBOSE; /* VACUUM FULL is now a variant of CLUSTER; see cluster.c */ - cluster_rel(relid, InvalidOid, tablespaceOid, cluster_options); + cluster_rel(relid, InvalidOid, params->tablespace_oid, + params->idxtablespace_oid, cluster_options); } else table_relation_vacuum(onerel, params, vac_strategy); diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c index 8acc321faa..7de5797f9c 100644 --- a/src/backend/postmaster/autovacuum.c +++ b/src/backend/postmaster/autovacuum.c @@ -2898,6 +2898,7 @@ table_recheck_autovac(Oid relid, HTAB *table_toast_map, tab->at_params.is_wraparound = wraparound; tab->at_params.log_min_duration = log_min_duration; tab->at_params.tablespace_oid = InvalidOid; + tab->at_params.idxtablespace_oid = InvalidOid; tab->at_vacuum_cost_limit = vac_cost_limit; tab->at_vacuum_cost_delay = vac_cost_delay; tab->at_relname = NULL; diff --git a/src/include/commands/cluster.h b/src/include/commands/cluster.h index bc9f881d8c..515e810505 100644 --- a/src/include/commands/cluster.h +++ b/src/include/commands/cluster.h @@ -20,7 +20,7 @@ extern void cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel); -extern void cluster_rel(Oid tableOid, Oid indexOid, Oid tablespaceOid, int options); +extern void cluster_rel(Oid tableOid, Oid indexOid, Oid tablespaceOid, Oid indextablespaceOid, int options); extern void check_index_is_clusterable(Relation OldHeap, Oid indexOid, bool recheck, LOCKMODE lockmode); extern void mark_index_clustered(Relation rel, Oid indexOid, bool is_internal); @@ -34,6 +34,7 @@ extern void finish_heap_swap(Oid OIDOldHeap, Oid OIDNewHeap, bool is_internal, TransactionId frozenXid, MultiXactId minMulti, - char newrelpersistence); + char newrelpersistence, + Oid idxtablespaceOid); #endif /* CLUSTER_H */ diff --git a/src/include/commands/vacuum.h b/src/include/commands/vacuum.h index 4b5ac7145d..488d7540c7 100644 --- a/src/include/commands/vacuum.h +++ b/src/include/commands/vacuum.h @@ -229,8 +229,9 @@ typedef struct VacuumParams * disabled. */ int nworkers; - Oid tablespace_oid; /* tablespace to use for relations - * rebuilt by VACUUM FULL */ + /* tablespaces to use for relations rebuilt by VACUUM FULL */ + Oid tablespace_oid; + Oid idxtablespace_oid; } VacuumParams; /* GUC parameters */ diff --git a/src/test/regress/input/tablespace.source b/src/test/regress/input/tablespace.source index eb9dfcc0f4..4cf79ab4bf 100644 --- a/src/test/regress/input/tablespace.source +++ b/src/test/regress/input/tablespace.source @@ -80,6 +80,19 @@ REINDEX (TABLESPACE pg_default) TABLE CONCURRENTLY regress_tblspace_test_tbl; -- SELECT relname FROM pg_class WHERE reltablespace=(SELECT oid FROM pg_tablespace WHERE spcname='regress_tblspace'); +-- check CLUSTER with INDEX_TABLESPACE change to non-default location +CLUSTER (INDEX_TABLESPACE regress_tblspace) regress_tblspace_test_tbl USING regress_tblspace_test_tbl_idx; -- ok +-- check relations moved to new tablespace +SELECT relname FROM pg_class +WHERE reltablespace=(SELECT oid FROM pg_tablespace WHERE spcname='regress_tblspace') +ORDER BY relname; +-- check VACUUM with INDEX_TABLESPACE change +VACUUM (FULL, ANALYSE, FREEZE, INDEX_TABLESPACE pg_default) regress_tblspace_test_tbl; -- ok +-- check relations moved back to pg_default +SELECT relname FROM pg_class +WHERE reltablespace=(SELECT oid FROM pg_tablespace WHERE spcname='regress_tblspace'); + + -- create a schema we can use CREATE SCHEMA testschema; diff --git a/src/test/regress/output/tablespace.source b/src/test/regress/output/tablespace.source index 789a0e56cc..bc72406b8e 100644 --- a/src/test/regress/output/tablespace.source +++ b/src/test/regress/output/tablespace.source @@ -114,6 +114,26 @@ WHERE reltablespace=(SELECT oid FROM pg_tablespace WHERE spcname='regress_tblspa --------- (0 rows) +-- check CLUSTER with INDEX_TABLESPACE change to non-default location +CLUSTER (INDEX_TABLESPACE regress_tblspace) regress_tblspace_test_tbl USING regress_tblspace_test_tbl_idx; -- ok +-- check relations moved to new tablespace +SELECT relname FROM pg_class +WHERE reltablespace=(SELECT oid FROM pg_tablespace WHERE spcname='regress_tblspace') +ORDER BY relname; + relname +------------------------------- + regress_tblspace_test_tbl_idx +(1 row) + +-- check VACUUM with INDEX_TABLESPACE change +VACUUM (FULL, ANALYSE, FREEZE, INDEX_TABLESPACE pg_default) regress_tblspace_test_tbl; -- ok +-- check relations moved back to pg_default +SELECT relname FROM pg_class +WHERE reltablespace=(SELECT oid FROM pg_tablespace WHERE spcname='regress_tblspace'); + relname +--------- +(0 rows) + -- create a schema we can use CREATE SCHEMA testschema; -- try a table -- 2.17.0