On Thu, Mar 12, 2020 at 08:08:46PM +0300, Alexey Kondratov wrote: > On 09.03.2020 23:04, Justin Pryzby wrote: >> On Sat, Feb 29, 2020 at 08:53:04AM -0600, Justin Pryzby wrote: >>> On Sat, Feb 29, 2020 at 03:35:27PM +0300, Alexey Kondratov wrote: >>> tests for that. (I'm including your v8 untouched in hopes of not messing up >>> the cfbot). My fixes avoid an issue if you try to REINDEX onto pg_default, >>> I >>> think due to moving system toast indexes. >> I was able to avoid this issue by adding a call to GetNewRelFileNode, even >> though that's already called by RelationSetNewRelfilenode(). Not sure if >> there's a better way, or if it's worth Alexey's v3 patch which added a >> tablespace param to RelationSetNewRelfilenode. > > Do you have any understanding of what exactly causes this error? I have > tried to debug it a little bit, but still cannot figure out why we need this > extra GetNewRelFileNode() call and a mechanism how it helps.
The PANIC is from smgr hashtable, which couldn't find an entry it expected. My very tentative understanding is that smgr is prepared to handle a *relation* which is dropped/recreated multiple times in a transaction, but it's *not* prepared to deal with a given RelFileNode(Backend) being dropped/recreated, since that's used as a hash key. I revisited it and solved it in a somewhat nicer way. It's still not clear to me if there's an issue with your original way of adding a tablespace parameter to RelationSetNewRelfilenode(). > Probably you mean v4 patch. Yes, interestingly, if we do everything at once > inside RelationSetNewRelfilenode(), then there is no issue at all with: Yes, I meant to say "worth revisiting the v4 patch". > Many thanks for you review and fixups! There are some inconsistencies like > mentions of SET TABLESPACE in error messages and so on. I am going to > refactor and include your fixes 0003-0004 into 0001 and 0002, but keep 0005 > separated for now, since this part requires more understanding IMO (and > comparison with v4 implementation). I'd suggest to keep the CLUSTER/VACUUM FULL separate from REINDEX, in case Michael or someone else wants to progress one but cannot commit to both. But probably we should plan to finish this in July. -- Justin
>From 8b5dde0fe77ae46aba35d3bb9ce026ca217a6e3e Mon Sep 17 00:00:00 2001 From: Alexey Kondratov <a.kondra...@postgrespro.ru> Date: Sat, 29 Feb 2020 15:35:27 +0300 Subject: [PATCH v11 1/5] Allow REINDEX to change tablespace >From d2b7a5fa2e11601759b47af0c142a7824ef907a2 Mon Sep 17 00:00:00 2001 From: Alexey Kondratov <kondratov.alek...@gmail.com> Date: Mon, 30 Dec 2019 20:00:37 +0300 Subject: [PATCH v8] 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 | 24 +++++- src/backend/catalog/index.c | 75 ++++++++++++++++-- src/backend/commands/cluster.c | 2 +- src/backend/commands/indexcmds.c | 96 ++++++++++++++++++++--- src/backend/commands/tablecmds.c | 2 +- src/backend/nodes/copyfuncs.c | 1 + src/backend/nodes/equalfuncs.c | 1 + src/backend/parser/gram.y | 14 ++-- src/backend/tcop/utility.c | 6 +- src/bin/psql/tab-complete.c | 6 ++ src/include/catalog/index.h | 7 +- src/include/commands/defrem.h | 6 +- src/include/nodes/parsenodes.h | 1 + src/test/regress/input/tablespace.source | 49 ++++++++++++ src/test/regress/output/tablespace.source | 66 ++++++++++++++++ 15 files changed, 323 insertions(+), 33 deletions(-) diff --git a/doc/src/sgml/ref/reindex.sgml b/doc/src/sgml/ref/reindex.sgml index c54a7c420d..0628c94bb1 100644 --- a/doc/src/sgml/ref/reindex.sgml +++ b/doc/src/sgml/ref/reindex.sgml @@ -21,7 +21,7 @@ PostgreSQL documentation <refsynopsisdiv> <synopsis> -REINDEX [ ( <replaceable class="parameter">option</replaceable> [, ...] ) ] { INDEX | TABLE | SCHEMA | DATABASE | SYSTEM } [ CONCURRENTLY ] <replaceable class="parameter">name</replaceable> +REINDEX [ ( <replaceable class="parameter">option</replaceable> [, ...] ) ] { INDEX | TABLE | SCHEMA | DATABASE | SYSTEM } [ CONCURRENTLY ] <replaceable class="parameter">name</replaceable> [ TABLESPACE <replaceable class="parameter">new_tablespace</replaceable> ] <phrase>where <replaceable class="parameter">option</replaceable> can be one of:</phrase> @@ -174,6 +174,28 @@ REINDEX [ ( <replaceable class="parameter">option</replaceable> [, ...] ) ] { IN </listitem> </varlistentry> + <varlistentry> + <term><literal>TABLESPACE</literal></term> + <listitem> + <para> + This specifies a tablespace, where all rebuilt indexes will be created. + 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><replaceable class="parameter">new_tablespace</replaceable></term> + <listitem> + <para> + The name of the specific tablespace to store rebuilt indexes. + </para> + </listitem> + </varlistentry> + <varlistentry> <term><literal>VERBOSE</literal></term> <listitem> diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c index 2d81bc3cbc..e6c06e6a06 100644 --- a/src/backend/catalog/index.c +++ b/src/backend/catalog/index.c @@ -1235,9 +1235,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, @@ -1367,7 +1371,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, @@ -3415,10 +3420,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; @@ -3465,6 +3472,16 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence, elog(ERROR, "unsupported relation kind for index \"%s\"", RelationGetRelationName(iRel)); + /* + * We cannot support moving mapped relations into different tablespaces. + * (In particular this eliminates all shared catalogs.) + */ + if (OidIsValid(tablespaceOid) && RelationIsMapped(iRel)) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot move system relation \"%s\"", + RelationGetRelationName(iRel)))); + /* * Don't allow reindex on temp tables of other backends ... their local * buffer manager is not going to cope. @@ -3491,6 +3508,45 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence, */ CheckTableNotInUse(iRel, "REINDEX INDEX"); + /* + * Set the new tablespace for the relation. Do that only in the + * case where the reindex caller wishes to enforce a new tablespace. + */ + if (OidIsValid(tablespaceOid) && + 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 == MyDatabaseTableSpace) ? + InvalidOid : 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. @@ -3629,6 +3685,10 @@ 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" defines the new tablespace where the indexes of + * the relation will be rebuilt. If InvalidOid is used, the current + * tablespace of each index is used instead. + * * "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). @@ -3661,7 +3721,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; @@ -3752,7 +3812,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(); @@ -3785,7 +3846,7 @@ 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); + result |= reindex_relation(toast_relid, tablespaceOid, flags, options); return result; } diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c index fc1cea0236..afc752e9d6 100644 --- a/src/backend/commands/cluster.c +++ b/src/backend/commands/cluster.c @@ -1406,7 +1406,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 4e8263af4b..1d394d5248 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); @@ -2303,10 +2303,11 @@ ChooseIndexColumnNames(List *indexElems) * Recreate a specific index. */ void -ReindexIndex(RangeVar *indexRelation, int options, bool concurrent) +ReindexIndex(RangeVar *indexRelation, char *newTableSpaceName, int options, bool concurrent) { struct ReindexIndexCallbackState state; Oid indOid; + Oid tablespaceOid = InvalidOid; Relation irel; char persistence; @@ -2341,12 +2342,26 @@ ReindexIndex(RangeVar *indexRelation, int options, bool concurrent) } persistence = irel->rd_rel->relpersistence; + + /* Define new tablespaceOid if it is wanted by caller */ + if (newTableSpaceName) + { + tablespaceOid = get_tablespace_oid(newTableSpaceName, 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\"", + newTableSpaceName))); + } + index_close(irel, NoLock); if (concurrent && persistence != RELPERSISTENCE_TEMP) - ReindexRelationConcurrently(indOid, options); + ReindexRelationConcurrently(indOid, tablespaceOid, options); else - reindex_index(indOid, false, persistence, + reindex_index(indOid, tablespaceOid, false, persistence, options | REINDEXOPT_REPORT_PROGRESS); } @@ -2425,10 +2440,11 @@ 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(RangeVar *relation, char *newTableSpaceName, int options, bool concurrent) { Oid heapOid; bool result; + Oid tablespaceOid = InvalidOid; /* * The lock level used here should match reindex_relation(). @@ -2443,9 +2459,22 @@ ReindexTable(RangeVar *relation, int options, bool concurrent) 0, RangeVarCallbackOwnsTable, NULL); + /* Define new tablespaceOid if it is wanted by caller */ + if (newTableSpaceName) + { + tablespaceOid = get_tablespace_oid(newTableSpaceName, 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\"", + newTableSpaceName))); + } + if (concurrent && get_rel_persistence(heapOid) != RELPERSISTENCE_TEMP) { - result = ReindexRelationConcurrently(heapOid, options); + result = ReindexRelationConcurrently(heapOid, tablespaceOid, options); if (!result) ereport(NOTICE, @@ -2455,6 +2484,7 @@ ReindexTable(RangeVar *relation, int options, bool concurrent) else { result = reindex_relation(heapOid, + tablespaceOid, REINDEX_REL_PROCESS_TOAST | REINDEX_REL_CHECK_CONSTRAINTS, options | REINDEXOPT_REPORT_PROGRESS); @@ -2476,10 +2506,11 @@ 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, +ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind, char *newTableSpaceName, int options, bool concurrent) { Oid objectOid; + Oid tablespaceOid = InvalidOid; Relation relationRelation; TableScanDesc scan; ScanKeyData scan_keys[1]; @@ -2490,6 +2521,7 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind, ListCell *l; int num_keys; bool concurrent_warning = false; + bool mapped_warning = false; AssertArg(objectName); Assert(objectKind == REINDEX_OBJECT_SCHEMA || @@ -2528,6 +2560,19 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind, objectName); } + /* Define new tablespaceOid if it is wanted by caller */ + if (newTableSpaceName) + { + tablespaceOid = get_tablespace_oid(newTableSpaceName, 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\"", + newTableSpaceName))); + } + /* * Create a memory context that will survive forced transaction commits we * do below. Since it is a child of PortalContext, it will go away @@ -2618,6 +2663,22 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind, continue; } + /* + * Skip all mapped relations if TABLESPACE is specified. + * relfilenode == 0 checks after that, similarly to + * RelationIsMapped(). + */ + if (OidIsValid(tablespaceOid) && + !OidIsValid(classtuple->relfilenode)) + { + if (!mapped_warning) + ereport(WARNING, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot change tablespace of indexes for mapped relations, skipping all"))); + mapped_warning = true; + continue; + } + /* Save the list of relation OIDs in private context */ old = MemoryContextSwitchTo(private_context); @@ -2651,7 +2712,7 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind, if (concurrent && get_rel_persistence(relid) != RELPERSISTENCE_TEMP) { - (void) ReindexRelationConcurrently(relid, options); + (void) ReindexRelationConcurrently(relid, tablespaceOid, options); /* ReindexRelationConcurrently() does the verbose output */ } else @@ -2659,6 +2720,7 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind, bool result; result = reindex_relation(relid, + tablespaceOid, REINDEX_REL_PROCESS_TOAST | REINDEX_REL_CHECK_CONSTRAINTS, options | REINDEXOPT_REPORT_PROGRESS); @@ -2691,6 +2753,9 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind, * itself will be rebuilt. If 'relationOid' belongs to a partitioned table * then we issue a warning to mention these are not yet supported. * + * 'tablespaceOid' defines the new tablespace where the indexes of + * the relation 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. @@ -2705,7 +2770,7 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind, * 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; @@ -2778,6 +2843,12 @@ ReindexRelationConcurrently(Oid relationOid, int options) /* Open relation to get its indexes */ heapRelation = table_open(relationOid, ShareUpdateExclusiveLock); + if (OidIsValid(tablespaceOid) && RelationIsMapped(heapRelation)) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot change tablespace of indexes for mapped relation \"%s\"", + RelationGetRelationName(heapRelation)))); + /* Add all the valid indexes of relation to list */ foreach(lc, RelationGetIndexList(heapRelation)) { @@ -2960,6 +3031,12 @@ ReindexRelationConcurrently(Oid relationOid, int options) if (indexRel->rd_rel->relpersistence == RELPERSISTENCE_TEMP) elog(ERROR, "cannot reindex a temporary table concurrently"); + if (OidIsValid(tablespaceOid) && RelationIsMapped(heapRel)) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot change tablespace of mapped relation \"%s\"", + RelationGetRelationName(heapRel)))); + pgstat_progress_start_command(PROGRESS_COMMAND_CREATE_INDEX, RelationGetRelid(heapRel)); pgstat_progress_update_param(PROGRESS_CREATEIDX_COMMAND, @@ -2979,6 +3056,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 8e35c5bd1a..d1f954265c 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -1871,7 +1871,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 eaab97f753..7bf9aa7e75 100644 --- a/src/backend/nodes/copyfuncs.c +++ b/src/backend/nodes/copyfuncs.c @@ -4406,6 +4406,7 @@ _copyReindexStmt(const ReindexStmt *from) COPY_STRING_FIELD(name); COPY_SCALAR_FIELD(options); 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 88b912977e..048c59fd68 100644 --- a/src/backend/nodes/equalfuncs.c +++ b/src/backend/nodes/equalfuncs.c @@ -2131,6 +2131,7 @@ _equalReindexStmt(const ReindexStmt *a, const ReindexStmt *b) COMPARE_STRING_FIELD(name); COMPARE_SCALAR_FIELD(options); COMPARE_SCALAR_FIELD(concurrent); + COMPARE_STRING_FIELD(tablespacename); return true; } diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index 7e384f956c..df8f63f989 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -8385,31 +8385,33 @@ DropTransformStmt: DROP TRANSFORM opt_if_exists FOR Typename LANGUAGE name opt_d * * QUERY: * - * REINDEX [ (options) ] type [CONCURRENTLY] <name> + * REINDEX [ (options) ] type [CONCURRENTLY] <name> [ TABLESPACE <tablespace_name> ] *****************************************************************************/ ReindexStmt: - REINDEX reindex_target_type opt_concurrently qualified_name + REINDEX reindex_target_type opt_concurrently qualified_name OptTableSpace { ReindexStmt *n = makeNode(ReindexStmt); n->kind = $2; n->concurrent = $3; n->relation = $4; + n->tablespacename = $5; n->name = NULL; n->options = 0; $$ = (Node *)n; } - | REINDEX reindex_target_multitable opt_concurrently name + | REINDEX reindex_target_multitable opt_concurrently name OptTableSpace { ReindexStmt *n = makeNode(ReindexStmt); n->kind = $2; n->concurrent = $3; n->name = $4; + n->tablespacename = $5; n->relation = NULL; n->options = 0; $$ = (Node *)n; } - | REINDEX '(' reindex_option_list ')' reindex_target_type opt_concurrently qualified_name + | REINDEX '(' reindex_option_list ')' reindex_target_type opt_concurrently qualified_name OptTableSpace { ReindexStmt *n = makeNode(ReindexStmt); n->kind = $5; @@ -8417,9 +8419,10 @@ ReindexStmt: n->relation = $7; n->name = NULL; n->options = $3; + n->tablespacename = $8; $$ = (Node *)n; } - | REINDEX '(' reindex_option_list ')' reindex_target_multitable opt_concurrently name + | REINDEX '(' reindex_option_list ')' reindex_target_multitable opt_concurrently name OptTableSpace { ReindexStmt *n = makeNode(ReindexStmt); n->kind = $5; @@ -8427,6 +8430,7 @@ ReindexStmt: n->name = $7; n->relation = NULL; n->options = $3; + n->tablespacename = $8; $$ = (Node *)n; } ; diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c index b1f7f6e2d0..1d9e448a1c 100644 --- a/src/backend/tcop/utility.c +++ b/src/backend/tcop/utility.c @@ -924,10 +924,10 @@ standard_ProcessUtility(PlannedStmt *pstmt, switch (stmt->kind) { case REINDEX_OBJECT_INDEX: - ReindexIndex(stmt->relation, stmt->options, stmt->concurrent); + ReindexIndex(stmt->relation, stmt->tablespacename, stmt->options, stmt->concurrent); break; case REINDEX_OBJECT_TABLE: - ReindexTable(stmt->relation, stmt->options, stmt->concurrent); + ReindexTable(stmt->relation, stmt->tablespacename, stmt->options, stmt->concurrent); break; case REINDEX_OBJECT_SCHEMA: case REINDEX_OBJECT_SYSTEM: @@ -943,7 +943,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->name, stmt->kind, stmt->tablespacename, stmt->options, stmt->concurrent); 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 ae35fa4aa9..c2257582e0 100644 --- a/src/bin/psql/tab-complete.c +++ b/src/bin/psql/tab-complete.c @@ -3432,6 +3432,12 @@ 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 (Matches("REINDEX", MatchAny, "CONCURRENTLY", MatchAny)) + COMPLETE_WITH("TABLESPACE"); + else if (Matches("REINDEX", MatchAny, MatchAny)) + COMPLETE_WITH("TABLESPACE"); + else if (TailMatches("TABLESPACE")) + COMPLETE_WITH_QUERY(Query_for_list_of_tablespaces); /* SECURITY LABEL */ else if (Matches("SECURITY")) diff --git a/src/include/catalog/index.h b/src/include/catalog/index.h index a2890c1314..f38e978b45 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, @@ -131,8 +132,8 @@ extern void validate_index(Oid heapId, Oid indexId, Snapshot snapshot); extern void index_set_state_flags(Oid indexId, IndexStateFlagsAction action); -extern void reindex_index(Oid indexId, bool skip_constraint_checks, - char relpersistence, int options); +extern void reindex_index(Oid indexId, Oid tablespaceOid, bool skip_constraint_checks, + char relpersistence, int options); /* Flag bits for reindex_relation(): */ #define REINDEX_REL_PROCESS_TOAST 0x01 @@ -141,7 +142,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/commands/defrem.h b/src/include/commands/defrem.h index c77c9a6ed5..6c4f0996fa 100644 --- a/src/include/commands/defrem.h +++ b/src/include/commands/defrem.h @@ -34,10 +34,10 @@ 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 ReindexIndex(RangeVar *indexRelation, char *newTableSpaceName, int options, bool concurrent); +extern Oid ReindexTable(RangeVar *relation, char *newTableSpaceName, int options, bool concurrent); extern void ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind, - int options, bool concurrent); + char *newTableSpaceName, int options, bool concurrent); 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 2039b42449..5075a79bd3 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -3363,6 +3363,7 @@ typedef struct ReindexStmt const char *name; /* name of database to reindex */ int options; /* Reindex options flags */ bool concurrent; /* reindex concurrently? */ + char *tablespacename; /* name of tablespace to store index */ } ReindexStmt; /* ---------------------- diff --git a/src/test/regress/input/tablespace.source b/src/test/regress/input/tablespace.source index a5f61a35dc..7d698d4294 100644 --- a/src/test/regress/input/tablespace.source +++ b/src/test/regress/input/tablespace.source @@ -17,6 +17,52 @@ 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 INDEX regress_tblspace_test_tbl_idx TABLESPACE regress_tblspace; +REINDEX TABLE regress_tblspace_test_tbl TABLESPACE regress_tblspace; +ROLLBACK; +BEGIN; +REINDEX TABLE pg_am TABLESPACE regress_tblspace; +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 DATABASE regression TABLESPACE regress_tblspace; -- ok with warning +REINDEX DATABASE regression TABLESPACE pg_default; -- 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 INDEX regress_tblspace_test_tbl_idx TABLESPACE regress_tblspace; -- ok +REINDEX TABLE regress_tblspace_test_tbl TABLESPACE regress_tblspace; -- ok +REINDEX TABLE pg_authid TABLESPACE regress_tblspace; -- fail +REINDEX SYSTEM CONCURRENTLY postgres TABLESPACE regress_tblspace; -- fail +REINDEX TABLE CONCURRENTLY pg_am TABLESPACE regress_tblspace; -- fail +REINDEX INDEX regress_tblspace_test_tbl_idx TABLESPACE pg_global; -- fail +REINDEX TABLE pg_am TABLESPACE regress_tblspace; -- ok + +-- 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 back to pg_default tablespace +REINDEX TABLE CONCURRENTLY regress_tblspace_test_tbl TABLESPACE pg_default; -- ok +REINDEX TABLE pg_am TABLESPACE pg_default; -- 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 +325,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..bd17feaa73 100644 --- a/src/test/regress/output/tablespace.source +++ b/src/test/regress/output/tablespace.source @@ -20,6 +20,70 @@ 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 INDEX regress_tblspace_test_tbl_idx TABLESPACE regress_tblspace; +REINDEX TABLE regress_tblspace_test_tbl TABLESPACE regress_tblspace; +ROLLBACK; +BEGIN; +REINDEX TABLE pg_am TABLESPACE regress_tblspace; +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 DATABASE regression TABLESPACE regress_tblspace; -- ok with warning +WARNING: cannot change tablespace of indexes for mapped relations, skipping all +REINDEX DATABASE regression TABLESPACE pg_default; -- ok with warning +WARNING: cannot change tablespace of indexes for mapped 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 INDEX regress_tblspace_test_tbl_idx TABLESPACE regress_tblspace; -- ok +REINDEX TABLE regress_tblspace_test_tbl TABLESPACE regress_tblspace; -- ok +REINDEX TABLE pg_authid TABLESPACE regress_tblspace; -- fail +ERROR: cannot move system relation "pg_authid_rolname_index" +REINDEX SYSTEM CONCURRENTLY postgres TABLESPACE regress_tblspace; -- fail +ERROR: cannot reindex system catalogs concurrently +REINDEX TABLE CONCURRENTLY pg_am TABLESPACE regress_tblspace; -- fail +ERROR: cannot reindex system catalogs concurrently +REINDEX INDEX regress_tblspace_test_tbl_idx TABLESPACE pg_global; -- fail +ERROR: cannot move non-shared relation to tablespace "pg_global" +REINDEX TABLE pg_am TABLESPACE regress_tblspace; -- ok +-- 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 +------------------------------- + pg_am_name_index + pg_am_oid_index + regress_tblspace_test_tbl_idx +(3 rows) + +-- move back to pg_default tablespace +REINDEX TABLE CONCURRENTLY regress_tblspace_test_tbl TABLESPACE pg_default; -- ok +REINDEX TABLE pg_am TABLESPACE pg_default; -- 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 +800,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 d6cc6dedfc2a16df896768295b8399edff9196b7 Mon Sep 17 00:00:00 2001 From: Alexey Kondratov <alex.lu...@gmail.com> Date: Fri, 21 Dec 2018 14:54:10 +0300 Subject: [PATCH v11 2/5] Allow CLUSTER and VACUUM FULL to change tablespace. --- doc/src/sgml/ref/cluster.sgml | 11 ++++- doc/src/sgml/ref/vacuum.sgml | 13 +++++- src/backend/commands/cluster.c | 28 +++++++---- src/backend/commands/tablecmds.c | 57 +++++++++++++---------- src/backend/commands/vacuum.c | 39 ++++++++++++++-- src/backend/parser/gram.y | 42 +++++++++++++++-- src/include/commands/cluster.h | 2 +- src/include/commands/defrem.h | 18 +++---- src/include/commands/tablecmds.h | 2 + src/include/commands/vacuum.h | 2 + src/include/nodes/parsenodes.h | 2 + src/test/regress/input/tablespace.source | 27 ++++++++--- src/test/regress/output/tablespace.source | 43 ++++++++++++----- 13 files changed, 219 insertions(+), 67 deletions(-) diff --git a/doc/src/sgml/ref/cluster.sgml b/doc/src/sgml/ref/cluster.sgml index 4da60d8d56..ebf23b8434 100644 --- a/doc/src/sgml/ref/cluster.sgml +++ b/doc/src/sgml/ref/cluster.sgml @@ -21,7 +21,7 @@ PostgreSQL documentation <refsynopsisdiv> <synopsis> -CLUSTER [VERBOSE] <replaceable class="parameter">table_name</replaceable> [ USING <replaceable class="parameter">index_name</replaceable> ] +CLUSTER [VERBOSE] <replaceable class="parameter">table_name</replaceable> [ USING <replaceable class="parameter">index_name</replaceable> ] [ TABLESPACE <replaceable class="parameter">new_tablespace</replaceable> ] CLUSTER [VERBOSE] </synopsis> </refsynopsisdiv> @@ -99,6 +99,15 @@ CLUSTER [VERBOSE] </listitem> </varlistentry> + <varlistentry> + <term><replaceable class="parameter">new_tablespace</replaceable></term> + <listitem> + <para> + The name of the specific tablespace to store clustered relations. + </para> + </listitem> + </varlistentry> + <varlistentry> <term><literal>VERBOSE</literal></term> <listitem> diff --git a/doc/src/sgml/ref/vacuum.sgml b/doc/src/sgml/ref/vacuum.sgml index 846056a353..e688edb06d 100644 --- a/doc/src/sgml/ref/vacuum.sgml +++ b/doc/src/sgml/ref/vacuum.sgml @@ -22,7 +22,8 @@ PostgreSQL documentation <refsynopsisdiv> <synopsis> VACUUM [ ( <replaceable class="parameter">option</replaceable> [, ...] ) ] [ <replaceable class="parameter">table_and_columns</replaceable> [, ...] ] -VACUUM [ FULL ] [ FREEZE ] [ VERBOSE ] [ ANALYZE ] [ <replaceable class="parameter">table_and_columns</replaceable> [, ...] ] +VACUUM [ FULL ] [ FREEZE ] [ VERBOSE ] [ ANALYZE ] [ TABLESPACE <replaceable class="parameter">new_tablespace</replaceable> ] [ <replaceable class="parameter">table_and_columns</replaceable> [, ...] ] +VACUUM ( FULL [, ...] ) [ TABLESPACE <replaceable class="parameter">new_tablespace</replaceable> ] [ <replaceable class="parameter">table_and_columns</replaceable> [, ...] ] <phrase>where <replaceable class="parameter">option</replaceable> can be one of:</phrase> @@ -299,6 +300,16 @@ VACUUM [ FULL ] [ FREEZE ] [ VERBOSE ] [ ANALYZE ] [ <replaceable class="paramet </para> </listitem> </varlistentry> + + <varlistentry> + <term><replaceable class="parameter">new_tablespace</replaceable></term> + <listitem> + <para> + The name of the specific tablespace to write new copy of the table. + </para> + </listitem> + </varlistentry> + </variablelist> </refsect1> diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c index afc752e9d6..e916eba52f 100644 --- a/src/backend/commands/cluster.c +++ b/src/backend/commands/cluster.c @@ -37,6 +37,7 @@ #include "commands/cluster.h" #include "commands/progress.h" #include "commands/tablecmds.h" +#include "commands/tablespace.h" #include "commands/vacuum.h" #include "miscadmin.h" #include "optimizer/optimizer.h" @@ -67,10 +68,10 @@ 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); + bool verbose, bool *pSwapToastByContent, + TransactionId *pFreezeXid, MultiXactId *pCutoffMulti); static List *get_tables_to_cluster(MemoryContext cluster_context); @@ -101,6 +102,13 @@ static List *get_tables_to_cluster(MemoryContext cluster_context); void cluster(ClusterStmt *stmt, bool isTopLevel) { + /* Oid of tablespace to use for clustered relation. */ + Oid tableSpaceOid = InvalidOid; + + /* Select tablespace Oid to use. */ + if (stmt->tablespacename) + tableSpaceOid = get_tablespace_oid(stmt->tablespacename, false); + if (stmt->relation != NULL) { /* This is the single-relation case. */ @@ -182,7 +190,7 @@ cluster(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 { @@ -230,7 +238,7 @@ cluster(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(); @@ -262,7 +270,7 @@ cluster(ClusterStmt *stmt, bool isTopLevel) * and error messages should refer to the operation as VACUUM not CLUSTER. */ 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); @@ -425,7 +433,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 */ @@ -584,7 +592,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; @@ -595,6 +603,10 @@ rebuild_relation(Relation OldHeap, Oid indexOid, bool verbose) TransactionId frozenXid; MultiXactId cutoffMulti; + /* Use new TeableSpace if passed. */ + if (OidIsValid(NewTableSpaceOid)) + tableSpace = NewTableSpaceOid; + /* Mark the correct index as clustered */ if (OidIsValid(indexOid)) mark_index_clustered(OldHeap, indexOid, true); diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index d1f954265c..a1bc0a9bb8 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -12905,30 +12905,7 @@ ATExecSetTableSpace(Oid tableOid, Oid newTableSpace, LOCKMODE lockmode) return; } - /* - * We cannot support moving mapped relations into different tablespaces. - * (In particular this eliminates all shared catalogs.) - */ - if (RelationIsMapped(rel)) - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("cannot move system relation \"%s\"", - RelationGetRelationName(rel)))); - - /* Can't move a non-shared relation into pg_global */ - if (newTableSpace == GLOBALTABLESPACE_OID) - ereport(ERROR, - (errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg("only shared relations can be placed in pg_global tablespace"))); - - /* - * Don't allow moving temp tables of other backends ... their local buffer - * manager is not going to cope. - */ - if (RELATION_IS_OTHER_TEMP(rel)) - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("cannot move temporary tables of other sessions"))); + check_relation_is_movable(rel, newTableSpace); reltoastrelid = rel->rd_rel->reltoastrelid; /* Fetch the list of indexes on toast relation if necessary */ @@ -13525,6 +13502,38 @@ CreateInheritance(Relation child_rel, Relation parent_rel) table_close(catalogRelation, RowExclusiveLock); } +/* + * Validate that relation can be moved to the specified tablespace. + */ +extern void +check_relation_is_movable(Relation rel, Oid tablespaceOid) +{ + /* + * We cannot support moving mapped relations into different tablespaces. + * (In particular this eliminates all shared catalogs.) + */ + if (RelationIsMapped(rel)) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot move system relation \"%s\"", + RelationGetRelationName(rel)))); + + /* Can't move a non-shared relation into pg_global */ + if (tablespaceOid == GLOBALTABLESPACE_OID) + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("only shared relations can be placed in pg_global tablespace"))); + + /* + * Don't allow moving temp tables of other backends ... their local buffer + * manager is not going to cope. + */ + if (RELATION_IS_OTHER_TEMP(rel)) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot move temporary tables of other sessions"))); +} + /* * Obtain the source-text form of the constraint expression for a check * constraint, given its pg_constraint tuple diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c index 59731d687f..7b20314b98 100644 --- a/src/backend/commands/vacuum.c +++ b/src/backend/commands/vacuum.c @@ -31,6 +31,7 @@ #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" @@ -38,6 +39,7 @@ #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" @@ -107,6 +109,8 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel) bool parallel_option = false; ListCell *lc; + Oid tableSpaceOid = InvalidOid; /* Oid of tablespace to use for relations + * store after VACUUM FULL. */ /* Set default value */ params.index_cleanup = VACOPT_TERNARY_DEFAULT; params.truncate = VACOPT_TERNARY_DEFAULT; @@ -241,6 +245,18 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel) params.multixact_freeze_table_age = -1; } + /* Get tablespace Oid to use. */ + if (vacstmt->tablespacename) + { + if (params.options & VACOPT_FULL) + tableSpaceOid = get_tablespace_oid(vacstmt->tablespacename, false); + else + ereport(ERROR, + (errmsg("incompatible SET TABLESPACE option"), + errdetail("You can only use SET TABLESPACE with VACUUM FULL."))); + } + params.tablespace_oid = tableSpaceOid; + /* user-invoked vacuum is never "for wraparound" */ params.is_wraparound = false; @@ -1672,8 +1688,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, + new_tablespaceoid = InvalidOid; int save_sec_context; int save_nestlevel; @@ -1807,6 +1824,22 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params) return true; } + /* + * We cannot support moving system relations into different tablespaces. + */ + if (params->options & VACOPT_FULL && OidIsValid(params->tablespace_oid) && IsSystemRelation(onerel)) + { + 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 + { + new_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 @@ -1876,7 +1909,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, new_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 df8f63f989..1f388017be 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -10589,14 +10589,14 @@ CreateConversionStmt: /***************************************************************************** * * QUERY: - * CLUSTER [VERBOSE] <qualified_name> [ USING <index_name> ] - * CLUSTER [VERBOSE] + * CLUSTER [VERBOSE] <qualified_name> [ USING <index_name> ] [ TABLESPACE <tablespace_name> ] + * CLUSTER [VERBOSE] [ TABLESPACE <tablespace_name> ] * CLUSTER [VERBOSE] <index_name> ON <qualified_name> (for pre-8.3) * *****************************************************************************/ ClusterStmt: - CLUSTER opt_verbose qualified_name cluster_index_specification + CLUSTER opt_verbose qualified_name cluster_index_specification OptTableSpace { ClusterStmt *n = makeNode(ClusterStmt); n->relation = $3; @@ -10604,6 +10604,7 @@ ClusterStmt: n->options = 0; if ($2) n->options |= CLUOPT_VERBOSE; + n->tablespacename = $5; $$ = (Node*)n; } | CLUSTER opt_verbose @@ -10625,6 +10626,7 @@ ClusterStmt: n->options = 0; if ($2) n->options |= CLUOPT_VERBOSE; + n->tablespacename = NULL; $$ = (Node*)n; } ; @@ -10639,6 +10641,8 @@ cluster_index_specification: * * QUERY: * VACUUM + * VACUUM FULL [ TABLESPACE <tablespace_name> ] [ <table_and_columns> [, ...] ] + * VACUUM (FULL) [ TABLESPACE <tablespace_name> ] [ <table_and_columns> [, ...] ] * ANALYZE * *****************************************************************************/ @@ -10661,6 +10665,28 @@ VacuumStmt: VACUUM opt_full opt_freeze opt_verbose opt_analyze opt_vacuum_relati makeDefElem("analyze", NULL, @5)); n->rels = $6; n->is_vacuumcmd = true; + n->tablespacename = NULL; + $$ = (Node *)n; + } + | VACUUM opt_full opt_freeze opt_verbose opt_analyze TABLESPACE name opt_vacuum_relation_list + { + VacuumStmt *n = makeNode(VacuumStmt); + n->options = NIL; + if ($2) + n->options = lappend(n->options, + makeDefElem("full", NULL, @2)); + if ($3) + n->options = lappend(n->options, + makeDefElem("freeze", NULL, @3)); + if ($4) + n->options = lappend(n->options, + makeDefElem("verbose", NULL, @4)); + if ($5) + n->options = lappend(n->options, + makeDefElem("analyze", NULL, @5)); + n->tablespacename = $7; + n->rels = $8; + n->is_vacuumcmd = true; $$ = (Node *)n; } | VACUUM '(' vac_analyze_option_list ')' opt_vacuum_relation_list @@ -10669,6 +10695,16 @@ VacuumStmt: VACUUM opt_full opt_freeze opt_verbose opt_analyze opt_vacuum_relati n->options = $3; n->rels = $5; n->is_vacuumcmd = true; + n->tablespacename = NULL; + $$ = (Node *) n; + } + | VACUUM '(' vac_analyze_option_list ')' TABLESPACE name opt_vacuum_relation_list + { + VacuumStmt *n = makeNode(VacuumStmt); + n->options = $3; + n->tablespacename = $6; + n->rels = $7; + n->is_vacuumcmd = true; $$ = (Node *) n; } ; diff --git a/src/include/commands/cluster.h b/src/include/commands/cluster.h index e05884781b..ab7a0ad642 100644 --- a/src/include/commands/cluster.h +++ b/src/include/commands/cluster.h @@ -19,7 +19,7 @@ extern void cluster(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/defrem.h b/src/include/commands/defrem.h index 6c4f0996fa..bb5c359922 100644 --- a/src/include/commands/defrem.h +++ b/src/include/commands/defrem.h @@ -25,15 +25,15 @@ extern void RemoveObjects(DropStmt *stmt); /* commands/indexcmds.c */ extern ObjectAddress DefineIndex(Oid relationId, - IndexStmt *stmt, - Oid indexRelationId, - Oid parentIndexId, - Oid parentConstraintId, - bool is_alter_table, - bool check_rights, - bool check_not_in_use, - bool skip_build, - bool quiet); + IndexStmt *stmt, + Oid indexRelationId, + Oid parentIndexId, + Oid parentConstraintId, + bool is_alter_table, + bool check_rights, + bool check_not_in_use, + bool skip_build, + bool quiet); extern void ReindexIndex(RangeVar *indexRelation, char *newTableSpaceName, int options, bool concurrent); extern Oid ReindexTable(RangeVar *relation, char *newTableSpaceName, int options, bool concurrent); extern void ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind, diff --git a/src/include/commands/tablecmds.h b/src/include/commands/tablecmds.h index c1581ad178..21f9cef535 100644 --- a/src/include/commands/tablecmds.h +++ b/src/include/commands/tablecmds.h @@ -55,6 +55,8 @@ extern void AlterRelationNamespaceInternal(Relation classRel, Oid relOid, extern void CheckTableNotInUse(Relation rel, const char *stmt); +extern void check_relation_is_movable(Relation rel, Oid tablespaceOid); + extern void ExecuteTruncate(TruncateStmt *stmt); extern void ExecuteTruncateGuts(List *explicit_rels, List *relids, List *relids_logged, DropBehavior behavior, bool restart_seqs); diff --git a/src/include/commands/vacuum.h b/src/include/commands/vacuum.h index 2779bea5c9..55003488ec 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 Oid to use for store relations + * after VACUUM FULL */ } VacuumParams; /* GUC parameters */ diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index 5075a79bd3..5fb0f72141 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -3202,6 +3202,7 @@ typedef struct ClusterStmt NodeTag type; RangeVar *relation; /* relation being indexed, or NULL if all */ char *indexname; /* original index defined */ + char *tablespacename; /* tablespace name to use for clustered relation. */ int options; /* OR of ClusterOption flags */ } ClusterStmt; @@ -3218,6 +3219,7 @@ typedef struct VacuumStmt List *options; /* list of DefElem nodes */ List *rels; /* list of VacuumRelation, or NIL for all */ bool is_vacuumcmd; /* true for VACUUM, false for ANALYZE */ + char *tablespacename; /* tablespace name to use for vacuumed relation. */ } VacuumStmt; /* diff --git a/src/test/regress/input/tablespace.source b/src/test/regress/input/tablespace.source index 7d698d4294..8ed97afb9c 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 @@ -45,23 +45,38 @@ WHERE reltablespace=(SELECT oid FROM pg_tablespace WHERE spcname='regress_tblspa REINDEX INDEX regress_tblspace_test_tbl_idx TABLESPACE regress_tblspace; -- ok REINDEX TABLE regress_tblspace_test_tbl TABLESPACE regress_tblspace; -- ok REINDEX TABLE pg_authid TABLESPACE regress_tblspace; -- fail -REINDEX SYSTEM CONCURRENTLY postgres TABLESPACE regress_tblspace; -- fail -REINDEX TABLE CONCURRENTLY pg_am TABLESPACE regress_tblspace; -- fail +REINDEX SCHEMA pg_catalog TABLESPACE regress_tblspace; -- fail +REINDEX DATABASE postgres TABLESPACE regress_tblspace; -- fail +REINDEX SYSTEM postgres TABLESPACE regress_tblspace; -- fail REINDEX INDEX regress_tblspace_test_tbl_idx TABLESPACE pg_global; -- fail -REINDEX TABLE pg_am TABLESPACE regress_tblspace; -- ok + +REINDEX SCHEMA pg_catalog TABLESPACE regress_tblspace; -- fail +REINDEX DATABASE postgres TABLESPACE regress_tblspace; -- fail +REINDEX SYSTEM postgres TABLESPACE regress_tblspace; -- fail + +-- check CLUSTER with TABLESPACE change +CLUSTER regress_tblspace_test_tbl USING regress_tblspace_test_tbl_idx TABLESPACE regress_tblspace; -- ok +CLUSTER pg_authid USING pg_authid_rolname_index TABLESPACE regress_tblspace; -- fail + +-- check VACUUM with TABLESPACE change +VACUUM (FULL) TABLESPACE regress_tblspace pg_authid; -- skip with warning +VACUUM (ANALYSE) TABLESPACE regress_tblspace; -- 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') +AND relname IN ('regress_tblspace_test_tbl_idx', 'regress_tblspace_test_tbl') ORDER BY relname; -- move back to pg_default tablespace REINDEX TABLE CONCURRENTLY regress_tblspace_test_tbl TABLESPACE pg_default; -- ok -REINDEX TABLE pg_am TABLESPACE pg_default; -- ok +CLUSTER regress_tblspace_test_tbl USING regress_tblspace_test_tbl_idx TABLESPACE pg_default; -- ok +VACUUM (FULL, ANALYSE, FREEZE) TABLESPACE pg_default 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'); +WHERE reltablespace=(SELECT oid FROM pg_tablespace WHERE spcname='regress_tblspace') +AND relname IN ('regress_tblspace_test_tbl_idx', 'regress_tblspace_test_tbl'); -- 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 bd17feaa73..5634cf20ff 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 @@ -33,6 +33,7 @@ REINDEX TABLE regress_tblspace_test_tbl TABLESPACE regress_tblspace; ROLLBACK; BEGIN; REINDEX TABLE pg_am TABLESPACE regress_tblspace; +ERROR: cannot move system relation "pg_am_name_index" ROLLBACK; SELECT relname FROM pg_class WHERE reltablespace=(SELECT oid FROM pg_tablespace WHERE spcname='regress_tblspace'); @@ -56,30 +57,50 @@ REINDEX INDEX regress_tblspace_test_tbl_idx TABLESPACE regress_tblspace; -- ok REINDEX TABLE regress_tblspace_test_tbl TABLESPACE regress_tblspace; -- ok REINDEX TABLE pg_authid TABLESPACE regress_tblspace; -- fail ERROR: cannot move system relation "pg_authid_rolname_index" -REINDEX SYSTEM CONCURRENTLY postgres TABLESPACE regress_tblspace; -- fail -ERROR: cannot reindex system catalogs concurrently -REINDEX TABLE CONCURRENTLY pg_am TABLESPACE regress_tblspace; -- fail -ERROR: cannot reindex system catalogs concurrently +REINDEX SCHEMA pg_catalog TABLESPACE regress_tblspace; -- fail +WARNING: cannot change tablespace of indexes for mapped relations, skipping all +REINDEX DATABASE postgres TABLESPACE regress_tblspace; -- fail +ERROR: can only reindex the currently open database +REINDEX SYSTEM postgres TABLESPACE regress_tblspace; -- fail +ERROR: can only reindex the currently open database REINDEX INDEX regress_tblspace_test_tbl_idx TABLESPACE pg_global; -- fail ERROR: cannot move non-shared relation to tablespace "pg_global" -REINDEX TABLE pg_am TABLESPACE regress_tblspace; -- ok +REINDEX SCHEMA pg_catalog TABLESPACE regress_tblspace; -- fail +WARNING: cannot change tablespace of indexes for mapped relations, skipping all +REINDEX DATABASE postgres TABLESPACE regress_tblspace; -- fail +ERROR: can only reindex the currently open database +REINDEX SYSTEM postgres TABLESPACE regress_tblspace; -- fail +ERROR: can only reindex the currently open database +-- check CLUSTER with TABLESPACE change +CLUSTER regress_tblspace_test_tbl USING regress_tblspace_test_tbl_idx TABLESPACE regress_tblspace; -- ok +CLUSTER pg_authid USING pg_authid_rolname_index TABLESPACE regress_tblspace; -- fail +ERROR: cannot cluster a shared catalog +-- check VACUUM with TABLESPACE change +VACUUM (FULL) TABLESPACE regress_tblspace pg_authid; -- skip with warning +WARNING: skipping tablespace change of "pg_authid" +DETAIL: Cannot move system relation, only VACUUM is performed. +VACUUM (ANALYSE) TABLESPACE regress_tblspace; -- fail +ERROR: incompatible SET TABLESPACE option +DETAIL: You can only use SET TABLESPACE with VACUUM FULL. -- check that all relations moved to new tablespace SELECT relname FROM pg_class WHERE reltablespace=(SELECT oid FROM pg_tablespace WHERE spcname='regress_tblspace') +AND relname IN ('regress_tblspace_test_tbl_idx', 'regress_tblspace_test_tbl') ORDER BY relname; relname ------------------------------- - pg_am_name_index - pg_am_oid_index + regress_tblspace_test_tbl regress_tblspace_test_tbl_idx -(3 rows) +(2 rows) -- move back to pg_default tablespace REINDEX TABLE CONCURRENTLY regress_tblspace_test_tbl TABLESPACE pg_default; -- ok -REINDEX TABLE pg_am TABLESPACE pg_default; -- ok +CLUSTER regress_tblspace_test_tbl USING regress_tblspace_test_tbl_idx TABLESPACE pg_default; -- ok +VACUUM (FULL, ANALYSE, FREEZE) TABLESPACE pg_default 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'); +WHERE reltablespace=(SELECT oid FROM pg_tablespace WHERE spcname='regress_tblspace') +AND relname IN ('regress_tblspace_test_tbl_idx', 'regress_tblspace_test_tbl'); relname --------- (0 rows) -- 2.17.0
>From c416b681a2dadcd9cb36253a0bb26177007203ad Mon Sep 17 00:00:00 2001 From: Justin Pryzby <pryz...@telsasoft.com> Date: Thu, 5 Mar 2020 19:48:08 -0600 Subject: [PATCH v11 3/5] Capitalize consistently --- src/backend/commands/cluster.c | 8 ++++---- src/backend/commands/vacuum.c | 6 +++--- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c index e916eba52f..794d4be30c 100644 --- a/src/backend/commands/cluster.c +++ b/src/backend/commands/cluster.c @@ -103,11 +103,11 @@ void cluster(ClusterStmt *stmt, bool isTopLevel) { /* Oid of tablespace to use for clustered relation. */ - Oid tableSpaceOid = InvalidOid; + Oid tablespaceOid = InvalidOid; /* Select tablespace Oid to use. */ if (stmt->tablespacename) - tableSpaceOid = get_tablespace_oid(stmt->tablespacename, false); + tablespaceOid = get_tablespace_oid(stmt->tablespacename, false); if (stmt->relation != NULL) { @@ -190,7 +190,7 @@ cluster(ClusterStmt *stmt, bool isTopLevel) table_close(rel, NoLock); /* Do the job. */ - cluster_rel(tableOid, indexOid, tableSpaceOid, stmt->options); + cluster_rel(tableOid, indexOid, tablespaceOid, stmt->options); } else { @@ -238,7 +238,7 @@ cluster(ClusterStmt *stmt, bool isTopLevel) /* functions in indexes may want a snapshot set */ PushActiveSnapshot(GetTransactionSnapshot()); /* Do the job. */ - cluster_rel(rvtc->tableOid, rvtc->indexOid, tableSpaceOid, + cluster_rel(rvtc->tableOid, rvtc->indexOid, tablespaceOid, stmt->options | CLUOPT_RECHECK); PopActiveSnapshot(); CommitTransactionCommand(); diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c index 7b20314b98..6dfdce32eb 100644 --- a/src/backend/commands/vacuum.c +++ b/src/backend/commands/vacuum.c @@ -109,7 +109,7 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel) bool parallel_option = false; ListCell *lc; - Oid tableSpaceOid = InvalidOid; /* Oid of tablespace to use for relations + Oid tablespaceOid = InvalidOid; /* Oid of tablespace to use for relations * store after VACUUM FULL. */ /* Set default value */ params.index_cleanup = VACOPT_TERNARY_DEFAULT; @@ -249,13 +249,13 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel) if (vacstmt->tablespacename) { if (params.options & VACOPT_FULL) - tableSpaceOid = get_tablespace_oid(vacstmt->tablespacename, false); + tablespaceOid = get_tablespace_oid(vacstmt->tablespacename, false); else ereport(ERROR, (errmsg("incompatible SET TABLESPACE option"), errdetail("You can only use SET TABLESPACE with VACUUM FULL."))); } - params.tablespace_oid = tableSpaceOid; + params.tablespace_oid = tablespaceOid; /* user-invoked vacuum is never "for wraparound" */ params.is_wraparound = false; -- 2.17.0
>From c3e94851f6620657ab90bd6f3f3466d862529d4e Mon Sep 17 00:00:00 2001 From: Justin Pryzby <pryz...@telsasoft.com> Date: Fri, 28 Feb 2020 19:20:48 -0600 Subject: [PATCH v11 4/5] fixes --- src/backend/catalog/index.c | 27 +++++++++----- src/backend/commands/cluster.c | 21 ++++++++++- src/backend/commands/indexcmds.c | 43 ++++++++++------------- src/backend/commands/vacuum.c | 3 +- src/backend/nodes/copyfuncs.c | 1 + src/backend/nodes/equalfuncs.c | 1 + src/backend/parser/gram.y | 3 ++ src/backend/postmaster/autovacuum.c | 1 + src/test/regress/output/tablespace.source | 11 +++--- 9 files changed, 70 insertions(+), 41 deletions(-) diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c index e6c06e6a06..05c4b51165 100644 --- a/src/backend/catalog/index.c +++ b/src/backend/catalog/index.c @@ -3434,6 +3434,7 @@ reindex_index(Oid indexId, Oid tablespaceOid, bool skip_constraint_checks, volatile bool skipped_constraint = false; PGRUsage ru0; bool progress = (options & REINDEXOPT_REPORT_PROGRESS) != 0; + bool valid_tablespace = OidIsValid(tablespaceOid); pg_rusage_init(&ru0); @@ -3473,14 +3474,19 @@ reindex_index(Oid indexId, Oid tablespaceOid, bool skip_constraint_checks, RelationGetRelationName(iRel)); /* - * We cannot support moving mapped relations into different tablespaces. - * (In particular this eliminates all shared catalogs.) + * We don't support moving system relations into different tablespaces. */ - if (OidIsValid(tablespaceOid) && RelationIsMapped(iRel)) + if (OidIsValid(tablespaceOid) && + ((IsCatalogRelationOid(indexId) && !allowSystemTableMods) + || RelationIsMapped(iRel))) + { + check_relation_is_movable(iRel, tablespaceOid); ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("cannot move system relation \"%s\"", - RelationGetRelationName(iRel)))); + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), + errmsg("permission denied: \"%s\" is a system catalog", + RelationGetRelationName(iRel)), + errdetail("cannot change tablespace of system catalog"))); + } /* * Don't allow reindex on temp tables of other backends ... their local @@ -3508,11 +3514,14 @@ reindex_index(Oid indexId, Oid tablespaceOid, bool skip_constraint_checks, */ CheckTableNotInUse(iRel, "REINDEX INDEX"); + tablespaceOid = (tablespaceOid == MyDatabaseTableSpace) ? + InvalidOid : tablespaceOid; + /* * Set the new tablespace for the relation. Do that only in the * case where the reindex caller wishes to enforce a new tablespace. */ - if (OidIsValid(tablespaceOid) && + if (valid_tablespace && tablespaceOid != iRel->rd_rel->reltablespace) { Relation pg_class; @@ -3535,8 +3544,8 @@ reindex_index(Oid indexId, Oid tablespaceOid, bool skip_constraint_checks, RelationDropStorage(iRel); /* Update the pg_class row */ - rd_rel->reltablespace = (tablespaceOid == MyDatabaseTableSpace) ? - InvalidOid : tablespaceOid; + rd_rel->reltablespace = tablespaceOid; + CatalogTupleUpdate(pg_class, &tuple->t_self, tuple); heap_freetuple(tuple); diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c index 794d4be30c..fdc62a1996 100644 --- a/src/backend/commands/cluster.c +++ b/src/backend/commands/cluster.c @@ -383,6 +383,18 @@ cluster_rel(Oid tableOid, Oid indexOid, Oid tableSpaceOid, int options) (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("cannot cluster a shared catalog"))); + if (!allowSystemTableMods && IsSystemRelation(OldHeap) && + OidIsValid(tableSpaceOid)) + // tableSpaceOid != OldHeap->rd_rel->reltablespace) + { + check_relation_is_movable(OldHeap, tableSpaceOid); + ereport(ERROR, + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), + errmsg("permission denied: \"%s\" is a system catalog", + RelationGetRelationName(OldHeap)), + errdetail("cannot change tablespace of system catalog"))); + } + /* * Don't process temp tables of other backends ... their local buffer * manager is not going to cope. @@ -603,7 +615,7 @@ rebuild_relation(Relation OldHeap, Oid indexOid, Oid NewTableSpaceOid, bool verb TransactionId frozenXid; MultiXactId cutoffMulti; - /* Use new TeableSpace if passed. */ + /* Use new TableSpace if passed. */ if (OidIsValid(NewTableSpaceOid)) tableSpace = NewTableSpaceOid; @@ -1051,6 +1063,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/indexcmds.c b/src/backend/commands/indexcmds.c index 1d394d5248..448e2f9054 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -2521,7 +2521,7 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind, char ListCell *l; int num_keys; bool concurrent_warning = false; - bool mapped_warning = false; + bool tblspc_warning = false; AssertArg(objectName); Assert(objectKind == REINDEX_OBJECT_SCHEMA || @@ -2562,17 +2562,8 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind, char /* Define new tablespaceOid if it is wanted by caller */ if (newTableSpaceName) - { tablespaceOid = get_tablespace_oid(newTableSpaceName, 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\"", - newTableSpaceName))); - } - /* * Create a memory context that will survive forced transaction commits we * do below. Since it is a child of PortalContext, it will go away @@ -2664,18 +2655,18 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind, char } /* - * Skip all mapped relations if TABLESPACE is specified. - * relfilenode == 0 checks after that, similarly to - * RelationIsMapped(). + * Skip all system relations if TABLESPACE is specified. + * Skip mapped relations (relfilenode=0) even if allowing system table mods. */ if (OidIsValid(tablespaceOid) && - !OidIsValid(classtuple->relfilenode)) + ((!allowSystemTableMods && IsSystemClass(relid, classtuple)) || + !OidIsValid(classtuple->relfilenode))) { - if (!mapped_warning) + if (!tblspc_warning) ereport(WARNING, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("cannot change tablespace of indexes for mapped relations, skipping all"))); - mapped_warning = true; + errmsg("cannot change tablespace of indexes on system catalogs, skipping all"))); + tblspc_warning = true; continue; } @@ -2843,11 +2834,12 @@ ReindexRelationConcurrently(Oid relationOid, Oid tablespaceOid, int options) /* Open relation to get its indexes */ heapRelation = table_open(relationOid, ShareUpdateExclusiveLock); - if (OidIsValid(tablespaceOid) && RelationIsMapped(heapRelation)) + if (OidIsValid(tablespaceOid) && IsSystemRelation(heapRelation)) ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("cannot change tablespace of indexes for mapped relation \"%s\"", - RelationGetRelationName(heapRelation)))); + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), + errmsg("permission denied: \"%s\" is a system catalog", + RelationGetRelationName(heapRelation)), + errdetail("cannot change tablespace of system catalog"))); /* Add all the valid indexes of relation to list */ foreach(lc, RelationGetIndexList(heapRelation)) @@ -3031,11 +3023,12 @@ ReindexRelationConcurrently(Oid relationOid, Oid tablespaceOid, int options) if (indexRel->rd_rel->relpersistence == RELPERSISTENCE_TEMP) elog(ERROR, "cannot reindex a temporary table concurrently"); - if (OidIsValid(tablespaceOid) && RelationIsMapped(heapRel)) + if (OidIsValid(tablespaceOid) && IsSystemRelation(heapRel)) ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("cannot change tablespace of mapped relation \"%s\"", - RelationGetRelationName(heapRel)))); + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), + errmsg("permission denied: \"%s\" is a system catalog", + RelationGetRelationName(heapRel)), + errdetail("cannot change tablespace of system catalog"))); pgstat_progress_start_command(PROGRESS_COMMAND_CREATE_INDEX, RelationGetRelid(heapRel)); diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c index 6dfdce32eb..095dbbce96 100644 --- a/src/backend/commands/vacuum.c +++ b/src/backend/commands/vacuum.c @@ -1827,7 +1827,8 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params) /* * We cannot support moving system relations into different tablespaces. */ - if (params->options & VACOPT_FULL && OidIsValid(params->tablespace_oid) && IsSystemRelation(onerel)) + if (params->options & VACOPT_FULL && OidIsValid(params->tablespace_oid) + && IsSystemRelation(onerel)) { ereport(WARNING, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c index 7bf9aa7e75..891bc37b08 100644 --- a/src/backend/nodes/copyfuncs.c +++ b/src/backend/nodes/copyfuncs.c @@ -3901,6 +3901,7 @@ _copyVacuumStmt(const VacuumStmt *from) COPY_NODE_FIELD(options); COPY_NODE_FIELD(rels); COPY_SCALAR_FIELD(is_vacuumcmd); + COPY_STRING_FIELD(tablespacename); return newnode; } diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c index 048c59fd68..da55563ca8 100644 --- a/src/backend/nodes/equalfuncs.c +++ b/src/backend/nodes/equalfuncs.c @@ -1702,6 +1702,7 @@ _equalVacuumStmt(const VacuumStmt *a, const VacuumStmt *b) COMPARE_NODE_FIELD(options); COMPARE_NODE_FIELD(rels); COMPARE_SCALAR_FIELD(is_vacuumcmd); + COMPARE_SCALAR_FIELD(tablespacename); return true; } diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index 1f388017be..300e6409f8 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -10615,6 +10615,7 @@ ClusterStmt: n->options = 0; if ($2) n->options |= CLUOPT_VERBOSE; + n->tablespacename = NULL; $$ = (Node*)n; } /* kept for pre-8.3 compatibility */ @@ -10718,6 +10719,7 @@ AnalyzeStmt: analyze_keyword opt_verbose opt_vacuum_relation_list makeDefElem("verbose", NULL, @2)); n->rels = $3; n->is_vacuumcmd = false; + n->tablespacename = NULL; $$ = (Node *)n; } | analyze_keyword '(' vac_analyze_option_list ')' opt_vacuum_relation_list @@ -10726,6 +10728,7 @@ AnalyzeStmt: analyze_keyword opt_verbose opt_vacuum_relation_list n->options = $3; n->rels = $5; n->is_vacuumcmd = false; + n->tablespacename = NULL; $$ = (Node *) n; } ; diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c index da75e755f0..ad538a872f 100644 --- a/src/backend/postmaster/autovacuum.c +++ b/src/backend/postmaster/autovacuum.c @@ -2895,6 +2895,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/test/regress/output/tablespace.source b/src/test/regress/output/tablespace.source index 5634cf20ff..0efe6cf4cc 100644 --- a/src/test/regress/output/tablespace.source +++ b/src/test/regress/output/tablespace.source @@ -33,7 +33,8 @@ REINDEX TABLE regress_tblspace_test_tbl TABLESPACE regress_tblspace; ROLLBACK; BEGIN; REINDEX TABLE pg_am TABLESPACE regress_tblspace; -ERROR: cannot move system relation "pg_am_name_index" +ERROR: permission denied: "pg_am_name_index" is a system catalog +DETAIL: cannot change tablespace of system catalog ROLLBACK; SELECT relname FROM pg_class WHERE reltablespace=(SELECT oid FROM pg_tablespace WHERE spcname='regress_tblspace'); @@ -43,9 +44,9 @@ WHERE reltablespace=(SELECT oid FROM pg_tablespace WHERE spcname='regress_tblspa -- first, let us reindex and move the entire database, after that return everything back REINDEX DATABASE regression TABLESPACE regress_tblspace; -- ok with warning -WARNING: cannot change tablespace of indexes for mapped relations, skipping all +WARNING: cannot change tablespace of indexes on system catalogs, skipping all REINDEX DATABASE regression TABLESPACE pg_default; -- ok with warning -WARNING: cannot change tablespace of indexes for mapped relations, skipping all +WARNING: cannot change tablespace of indexes on system catalogs, skipping all SELECT relname FROM pg_class WHERE reltablespace=(SELECT oid FROM pg_tablespace WHERE spcname='regress_tblspace'); relname @@ -58,7 +59,7 @@ REINDEX TABLE regress_tblspace_test_tbl TABLESPACE regress_tblspace; -- ok REINDEX TABLE pg_authid TABLESPACE regress_tblspace; -- fail ERROR: cannot move system relation "pg_authid_rolname_index" REINDEX SCHEMA pg_catalog TABLESPACE regress_tblspace; -- fail -WARNING: cannot change tablespace of indexes for mapped relations, skipping all +WARNING: cannot change tablespace of indexes on system catalogs, skipping all REINDEX DATABASE postgres TABLESPACE regress_tblspace; -- fail ERROR: can only reindex the currently open database REINDEX SYSTEM postgres TABLESPACE regress_tblspace; -- fail @@ -66,7 +67,7 @@ ERROR: can only reindex the currently open database REINDEX INDEX regress_tblspace_test_tbl_idx TABLESPACE pg_global; -- fail ERROR: cannot move non-shared relation to tablespace "pg_global" REINDEX SCHEMA pg_catalog TABLESPACE regress_tblspace; -- fail -WARNING: cannot change tablespace of indexes for mapped relations, skipping all +WARNING: cannot change tablespace of indexes on system catalogs, skipping all REINDEX DATABASE postgres TABLESPACE regress_tblspace; -- fail ERROR: can only reindex the currently open database REINDEX SYSTEM postgres TABLESPACE regress_tblspace; -- fail -- 2.17.0
>From eb292e4b78408c04e9f1481bb36db62aafe1aab9 Mon Sep 17 00:00:00 2001 From: Justin Pryzby <pryz...@telsasoft.com> Date: Mon, 9 Mar 2020 15:34:41 -0500 Subject: [PATCH v11 5/5] Specially handle toast relations during REINDEX CONCURRENTLY.. Is this fine ? It says "cannot reindex system catalogs concurrently" (once), and hits the pg_toast tables for information_schema. Should it skip toast indexes (like it said) ? Or should it REINDEX them on the same tablespace? template1=# REINDEX DATABASE CONCURRENTLY template1 TABLESPACE pg_default; 2020-03-09 15:33:51.792 CDT [6464] WARNING: cannot reindex system catalogs concurrently, skipping all WARNING: cannot reindex system catalogs concurrently, skipping all 2020-03-09 15:33:51.794 CDT [6464] WARNING: skipping tablespace change of "pg_toast_12558_index" 2020-03-09 15:33:51.794 CDT [6464] DETAIL: Cannot move system relation, only REINDEX CONCURRENTLY is performed. WARNING: skipping tablespace change of "pg_toast_12558_index" DETAIL: Cannot move system relation, only REINDEX CONCURRENTLY is performed. 2020-03-09 15:33:51.924 CDT [6464] WARNING: skipping tablespace change of "pg_toast_12543_index" 2020-03-09 15:33:51.924 CDT [6464] DETAIL: Cannot move system relation, only REINDEX CONCURRENTLY is performed. WARNING: skipping tablespace change of "pg_toast_12543_index" DETAIL: Cannot move system relation, only REINDEX CONCURRENTLY is performed. 2020-03-09 15:33:51.982 CDT [6464] WARNING: skipping tablespace change of "pg_toast_12548_index" 2020-03-09 15:33:51.982 CDT [6464] DETAIL: Cannot move system relation, only REINDEX CONCURRENTLY is performed. WARNING: skipping tablespace change of "pg_toast_12548_index" DETAIL: Cannot move system relation, only REINDEX CONCURRENTLY is performed. 2020-03-09 15:33:52.048 CDT [6464] WARNING: skipping tablespace change of "pg_toast_12553_index" 2020-03-09 15:33:52.048 CDT [6464] DETAIL: Cannot move system relation, only REINDEX CONCURRENTLY is performed. WARNING: skipping tablespace change of "pg_toast_12553_index" DETAIL: Cannot move system relation, only REINDEX CONCURRENTLY is performed. REINDEX --- src/backend/commands/indexcmds.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index 448e2f9054..24db276ef2 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -3023,7 +3023,13 @@ ReindexRelationConcurrently(Oid relationOid, Oid tablespaceOid, int options) if (indexRel->rd_rel->relpersistence == RELPERSISTENCE_TEMP) elog(ERROR, "cannot reindex a temporary table concurrently"); - if (OidIsValid(tablespaceOid) && IsSystemRelation(heapRel)) + if (OidIsValid(tablespaceOid) && IsToastRelation(indexRel)) + ereport(WARNING, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("skipping tablespace change of \"%s\"", + RelationGetRelationName(indexRel)), + errdetail("Cannot move system relation, only REINDEX CONCURRENTLY is performed."))); + else if (OidIsValid(tablespaceOid) && IsSystemRelation(indexRel)) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), errmsg("permission denied: \"%s\" is a system catalog", @@ -3049,7 +3055,7 @@ ReindexRelationConcurrently(Oid relationOid, Oid tablespaceOid, int options) /* Create new index definition based on given index */ newIndexId = index_concurrently_create_copy(heapRel, indexId, - tablespaceOid, + IsToastRelation(indexRel) ? InvalidOid : tablespaceOid, concurrentName); /* -- 2.17.0