Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1iCkSu-00028l-E1 for pgsql-hackers@arkaria.postgresql.org; Tue, 24 Sep 2019 13:02:49 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.89) (envelope-from ) id 1iCkSs-00055p-Lq for pgsql-hackers@arkaria.postgresql.org; Tue, 24 Sep 2019 13:02:46 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1iCkSr-00055i-W5 for pgsql-hackers@lists.postgresql.org; Tue, 24 Sep 2019 13:02:46 +0000 Received: from cyclops.postgrespro.ru ([93.174.131.138] helo=mail.postgrespro.ru) by magus.postgresql.org with esmtp (Exim 4.89) (envelope-from ) id 1iCkSo-0007og-9l for pgsql-hackers@postgresql.org; Tue, 24 Sep 2019 13:02:45 +0000 Received: from localhost (localhost [127.0.0.1]) by mail.postgrespro.ru (Postfix) with ESMTP id 9FBAD21C4A3C; Tue, 24 Sep 2019 16:02:40 +0300 (MSK) X-Virus-Scanned: Debian amavisd-new at postgrespro.ru X-Spam-Flag: NO X-Spam-Score: 0 X-Spam-Level: X-Spam-Status: No, score=x tagged_above=-99 required=4 WHITELISTED tests=[] autolearn=unavailable Received: from [192.168.27.223] (gw.postgrespro.ru [93.174.131.141]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client did not present a certificate) by mail.postgrespro.ru (Postfix) with ESMTPSA id 5526721C445B; Tue, 24 Sep 2019 16:02:40 +0300 (MSK) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=postgrespro.ru; s=mail; t=1569330160; bh=fT8THx+suEpHStG0O8aUQw7bYS/iD0+sR1CVH5w4W5w=; h=Subject:To:Cc:References:From:Date:In-Reply-To; b=PIeYBjSZg3FLWw9BRbiJalNSGeWN2k4PLKyilJ1xBKqLixirLGrM5eleR4f2M15LH +VZtjeU/VB3FvLtKlROeiMBPO3MaBH+G3vO16TxGraf4AAdONsCPjDIghWWh5/gFad u5IfBfjbLuHHUZ/W3+aPGBZHHtbaN0nAzAVOYl88= Subject: Re: Allow CLUSTER, VACUUM FULL and REINDEX to change tablespace on the fly To: Alvaro Herrera , Michael Paquier Cc: Robert Haas , Surafel Temesgen , Alexander Korotkov , Masahiko Sawada , PostgreSQL Hackers , Jose Luis Tallon References: <20190920163808.GA18848@alvherre.pgsql> From: Alexey Kondratov Message-ID: Date: Tue, 24 Sep 2019 16:02:39 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.8.0 MIME-Version: 1.0 In-Reply-To: <20190920163808.GA18848@alvherre.pgsql> Content-Type: multipart/mixed; boundary="------------0FADFCA1768281ADAB10366C" Content-Language: en-US List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk This is a multi-part message in MIME format. --------------0FADFCA1768281ADAB10366C Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit On 20.09.2019 19:38, Alvaro Herrera wrote: > On 2019-Sep-19, Robert Haas wrote: > >> So, earlier in this thread, I suggested making this part of ALTER >> TABLE, and several people seemed to like that idea. Did we have a >> reason for dropping that approach? > Hmm, my own reading of that was to add tablespace changing abilities to > ALTER TABLE *in addition* to this patch, not instead of it. That was my understanding too. On 20.09.2019 11:26, Jose Luis Tallon wrote: > On 20/9/19 4:06, Michael Paquier wrote: >> Personally, I don't find this idea very attractive as ALTER TABLE is >> already complicated enough with all the subqueries we already support >> in the command, all the logic we need to maintain to make combinations >> of those subqueries in a minimum number of steps, and also the number >> of bugs we have seen because of the amount of complication present. > > Yes, but please keep the other options: At it is, cluster, vacuum full > and reindex already rewrite the table in full; Being able to write the > result to a different tablespace than the original object was stored > in enables a whole world of very interesting possibilities.... > including a quick way out of a "so little disk space available that > vacuum won't work properly" situation --- which I'm sure MANY users > will appreciate, including me Yes, sure, that was my main motivation. The first message in the thread contains a patch, which adds SET TABLESPACE support to all of CLUSTER, VACUUM FULL and REINDEX. However, there came up an idea to integrate CLUSTER/VACUUM FULL with ALTER TABLE and do their work + all the ALTER TABLE stuff in a single table rewrite. I've dig a little bit into this and ended up with some architectural questions and concerns [1]. So I decided to start with a simple REINDEX patch. Anyway, I've followed Michael's advice and split the last patch into two: 1) Adds all the main functionality, but with simplified 'REINDEX INDEX [ CONCURRENTLY ] ... [ TABLESPACE ... ]' grammar; 2) Adds a more sophisticated syntax with '[ SET TABLESPACE ... [ NOWAIT ] ]'. Patch 1 contains all the docs and tests and may be applied/committed separately or together with 2, which is fully optional. Recent merge conflicts and reindex_index validations order are also fixed in the attached version. [1] https://www.postgresql.org/message-id/6b2a5c4de19f111ef24b63428033bb67%40postgrespro.ru Regards -- Alexey Kondratov Postgres Professional https://www.postgrespro.com Russian Postgres Company --------------0FADFCA1768281ADAB10366C Content-Type: text/x-patch; name="v3-0001-Allow-REINDEX-and-REINDEX-CONCURRENTLY-to-change-.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename*0="v3-0001-Allow-REINDEX-and-REINDEX-CONCURRENTLY-to-change-.pa"; filename*1="tch" From 4f06996f1e86dee389cb0f901cb83dba77c2abd8 Mon Sep 17 00:00:00 2001 From: Alexey Kondratov Date: Tue, 24 Sep 2019 12:29:57 +0300 Subject: [PATCH v3 1/2] Allow REINDEX and REINDEX CONCURRENTLY to change TABLESPACE --- doc/src/sgml/ref/reindex.sgml | 23 ++++++ src/backend/catalog/index.c | 99 ++++++++++++++++++++--- src/backend/commands/cluster.c | 2 +- src/backend/commands/indexcmds.c | 34 +++++--- src/backend/commands/tablecmds.c | 59 ++++++++------ src/backend/parser/gram.y | 21 +++-- src/backend/tcop/utility.c | 16 +++- src/include/catalog/index.h | 7 +- src/include/commands/defrem.h | 6 +- src/include/commands/tablecmds.h | 2 + src/include/nodes/parsenodes.h | 1 + src/test/regress/input/tablespace.source | 31 +++++++ src/test/regress/output/tablespace.source | 41 ++++++++++ 13 files changed, 279 insertions(+), 63 deletions(-) diff --git a/doc/src/sgml/ref/reindex.sgml b/doc/src/sgml/ref/reindex.sgml index 10881ab03a..96c9363ad9 100644 --- a/doc/src/sgml/ref/reindex.sgml +++ b/doc/src/sgml/ref/reindex.sgml @@ -22,6 +22,7 @@ PostgreSQL documentation REINDEX [ ( VERBOSE ) ] { INDEX | TABLE | SCHEMA | DATABASE | SYSTEM } [ CONCURRENTLY ] name +REINDEX [ ( VERBOSE ) ] { INDEX | TABLE } [ CONCURRENTLY ] name [ TABLESPACE new_tablespace ] @@ -165,6 +166,28 @@ REINDEX [ ( VERBOSE ) ] { INDEX | TABLE | SCHEMA | DATABASE | SYSTEM } [ CONCURR + + TABLESPACE + + + This specifies a tablespace, where all rebuilt indexes will be created. + Can be used only with REINDEX INDEX and + REINDEX TABLE, since the system indexes are not + movable, but SCHEMA, DATABASE or + SYSTEM very likely will has one. + + + + + + new_tablespace + + + The name of the specific tablespace to store rebuilt indexes. + + + + VERBOSE diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c index 098732cc4a..b2fed5dc75 100644 --- a/src/backend/catalog/index.c +++ b/src/backend/catalog/index.c @@ -1239,7 +1239,8 @@ index_create(Relation heapRelation, * on. This is called during concurrent reindex processing. */ 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, @@ -1369,7 +1370,7 @@ index_concurrently_create_copy(Relation heapRelation, Oid oldIndexId, const char newInfo, indexColNames, indexRelation->rd_rel->relam, - indexRelation->rd_rel->reltablespace, + tablespaceOid ? tablespaceOid : indexRelation->rd_rel->reltablespace, indexRelation->rd_indcollation, indclass->values, indcoloptions->values, @@ -3342,16 +3343,22 @@ IndexGetRelation(Oid indexId, bool missing_ok) * reindex_index - This routine is used to recreate a single index */ void -reindex_index(Oid indexId, bool skip_constraint_checks, char persistence, +reindex_index(Oid indexId, Oid tablespaceOid, bool skip_constraint_checks, char persistence, int options) { Relation iRel, - heapRelation; - Oid heapId; + heapRelation, + pg_class; + Oid heapId, + newIndexRelfilenodeOid = InvalidOid; IndexInfo *indexInfo; volatile bool skipped_constraint = false; PGRUsage ru0; bool progress = (options & REINDEXOPT_REPORT_PROGRESS) != 0; + RelFileNode newrnode; + SMgrRelation dstrel; + HeapTuple tuple; + Form_pg_class rd_rel; pg_rusage_init(&ru0); @@ -3390,11 +3397,15 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence, elog(ERROR, "unsupported relation kind for index \"%s\"", RelationGetRelationName(iRel)); - /* - * Don't allow reindex on temp tables of other backends ... their local - * buffer manager is not going to cope. - */ - if (RELATION_IS_OTHER_TEMP(iRel)) + if (OidIsValid(tablespaceOid)) + /* Check that relocation is possible during reindex. */ + check_relation_is_movable(iRel, tablespaceOid); + else if (RELATION_IS_OTHER_TEMP(iRel)) + /* + * Don't allow reindex on temp tables of other backends ... their local + * buffer manager is not going to cope. Check only if TABLESPACE is not + * passed, since the same validation exists in the check_relation_is_movable. + */ ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("cannot reindex temporary tables of other sessions"))); @@ -3405,6 +3416,68 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence, */ CheckTableNotInUse(iRel, "REINDEX INDEX"); + if (OidIsValid(tablespaceOid)) + { + pg_class = heap_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); + + /* Use binary-upgrade override for pg_class.oid/relfilenode? */ + if (IsBinaryUpgrade) + { + if (!OidIsValid(binary_upgrade_next_index_pg_class_oid)) + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("pg_class index OID value not set when in binary upgrade mode"))); + + newIndexRelfilenodeOid = binary_upgrade_next_index_pg_class_oid; + binary_upgrade_next_index_pg_class_oid = InvalidOid; + } + else + { + newIndexRelfilenodeOid = + GetNewRelFileNode(tablespaceOid, pg_class, heapRelation->rd_rel->relpersistence); + } + + /* Open old and new relation */ + newrnode = iRel->rd_node; + newrnode.relNode = newIndexRelfilenodeOid; + newrnode.spcNode = tablespaceOid; + dstrel = smgropen(newrnode, iRel->rd_backend); + + RelationOpenSmgr(iRel); + + /* + * Create and copy all forks of the relation, and schedule unlinking of + * old physical files. + * + * NOTE: any conflict in relfilenode value will be caught in + * RelationCreateStorage(). + */ + RelationCreateStorage(newrnode, iRel->rd_rel->relpersistence); + + /* Drop old relation, and close new one */ + RelationDropStorage(iRel); + smgrclose(dstrel); + + /* Update the pg_class row */ + rd_rel->reltablespace = tablespaceOid; + rd_rel->relfilenode = newIndexRelfilenodeOid; + CatalogTupleUpdate(pg_class, &tuple->t_self, tuple); + + InvokeObjectPostAlterHook(RelationRelationId, RelationGetRelid(iRel), 0); + + heap_freetuple(tuple); + + heap_close(pg_class, RowExclusiveLock); + + /* Make the updated catalog row versions visible */ + CommandCounterIncrement(); + } + /* * All predicate locks on the index are about to be made invalid. Promote * them to relation locks on the heap. @@ -3577,7 +3650,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; @@ -3651,7 +3724,7 @@ reindex_relation(Oid relid, int flags, int options) { Oid indexOid = lfirst_oid(indexId); - reindex_index(indexOid, !(flags & REINDEX_REL_CHECK_CONSTRAINTS), + reindex_index(indexOid, tablespaceOid, !(flags & REINDEX_REL_CHECK_CONSTRAINTS), persistence, options); CommandCounterIncrement(); @@ -3686,7 +3759,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 a23128d7a0..0f68f899da 100644 --- a/src/backend/commands/cluster.c +++ b/src/backend/commands/cluster.c @@ -1407,7 +1407,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 70f9b6729a..7f00dac426 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); @@ -2312,10 +2312,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; @@ -2345,12 +2346,16 @@ ReindexIndex(RangeVar *indexRelation, int options, bool concurrent) } persistence = irel->rd_rel->relpersistence; + + if (newTableSpaceName) + tableSpaceOid = get_tablespace_oid(newTableSpaceName, false); + index_close(irel, NoLock); if (concurrent) - ReindexRelationConcurrently(indOid, options); + ReindexRelationConcurrently(indOid, tableSpaceOid, options); else - reindex_index(indOid, false, persistence, + reindex_index(indOid, tableSpaceOid, false, persistence, options | REINDEXOPT_REPORT_PROGRESS); } @@ -2429,10 +2434,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(). */ heapOid = RangeVarGetRelidExtended(relation, @@ -2440,9 +2446,12 @@ ReindexTable(RangeVar *relation, int options, bool concurrent) 0, RangeVarCallbackOwnsTable, NULL); + if (newTableSpaceName) + tableSpaceOid = get_tablespace_oid(newTableSpaceName, false); + if (concurrent) { - result = ReindexRelationConcurrently(heapOid, options); + result = ReindexRelationConcurrently(heapOid, tableSpaceOid, options); if (!result) ereport(NOTICE, @@ -2452,6 +2461,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); @@ -2473,10 +2483,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]; @@ -2525,6 +2536,9 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind, objectName); } + if (newTableSpaceName) + tableSpaceOid = get_tablespace_oid(newTableSpaceName, 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 @@ -2648,7 +2662,7 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind, if (concurrent) { - (void) ReindexRelationConcurrently(relid, options); + (void) ReindexRelationConcurrently(relid, tableSpaceOid, options); /* ReindexRelationConcurrently() does the verbose output */ } else @@ -2656,6 +2670,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); @@ -2696,7 +2711,7 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind, * indexes, when relevant), otherwise returns false. */ static bool -ReindexRelationConcurrently(Oid relationOid, int options) +ReindexRelationConcurrently(Oid relationOid, Oid tablespaceOid, int options) { List *heapRelationIds = NIL; List *indexIds = NIL; @@ -2956,6 +2971,7 @@ ReindexRelationConcurrently(Oid relationOid, int options) /* Create new index definition based on given index */ newIndexId = index_concurrently_create_copy(heapRel, indexId, + tablespaceOid, concurrentName); /* Now open the relation of the new index, a lock is also needed on it */ diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 05593f3316..0f9b0d4108 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -1817,7 +1817,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); @@ -12337,30 +12337,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 */ @@ -12957,6 +12934,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/parser/gram.y b/src/backend/parser/gram.y index 3f67aaf30e..cba361f1bc 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -549,7 +549,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query); %type constraints_set_list %type constraints_set_mode -%type OptTableSpace OptConsTableSpace +%type OptTableSpace OptConsTableSpace opt_tablespace_name %type OptTableSpaceOwner %type opt_check_option @@ -3937,6 +3937,11 @@ OptTableSpace: TABLESPACE name { $$ = $2; } | /*EMPTY*/ { $$ = NULL; } ; +opt_tablespace_name: + TABLESPACE name { $$ = $2; } + | /*EMPTY*/ { $$ = NULL; } + ; + OptConsTableSpace: USING INDEX TABLESPACE name { $$ = $4; } | /*EMPTY*/ { $$ = NULL; } ; @@ -8356,31 +8361,33 @@ DropTransformStmt: DROP TRANSFORM opt_if_exists FOR Typename LANGUAGE name opt_d * * QUERY: * - * REINDEX [ (options) ] type [CONCURRENTLY] + * REINDEX [ (options) ] type [CONCURRENTLY] [ TABLESPACE ] *****************************************************************************/ ReindexStmt: - REINDEX reindex_target_type opt_concurrently qualified_name + REINDEX reindex_target_type opt_concurrently qualified_name opt_tablespace_name { 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 opt_tablespace_name { 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 opt_tablespace_name { ReindexStmt *n = makeNode(ReindexStmt); n->kind = $5; @@ -8388,9 +8395,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 opt_tablespace_name { ReindexStmt *n = makeNode(ReindexStmt); n->kind = $5; @@ -8398,6 +8406,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 c6faa6619d..fed40471e0 100644 --- a/src/backend/tcop/utility.c +++ b/src/backend/tcop/utility.c @@ -784,15 +784,25 @@ 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: case REINDEX_OBJECT_DATABASE: + /* + * We cannot move system relations to a new tablespace and + * the entire schema/database very likely will has one, + * so simply reject such cases. + */ + if (stmt->tablespacename) + ereport(ERROR, + (errmsg("incompatible TABLESPACE option"), + errdetail("You can only use TABLESPACE with REINDEX { INDEX | TABLE }."))); + /* * This cannot run inside a user transaction block; if * we were inside a transaction, then its commit- and @@ -803,7 +813,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/include/catalog/index.h b/src/include/catalog/index.h index 1113d25b2d..ef6103a174 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, @@ -129,8 +130,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 @@ -139,7 +140,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 1dc6dc2ca0..3687872869 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/commands/tablecmds.h b/src/include/commands/tablecmds.h index 9c25a805f2..4824bd6166 100644 --- a/src/include/commands/tablecmds.h +++ b/src/include/commands/tablecmds.h @@ -52,6 +52,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/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index d93a79a554..ed2c385050 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -3333,6 +3333,7 @@ typedef struct ReindexStmt const char *name; /* name of database to reindex */ int options; /* Reindex options flags */ bool concurrent; /* reindex concurrently? */ + char *tablespacename; } ReindexStmt; /* ---------------------- diff --git a/src/test/regress/input/tablespace.source b/src/test/regress/input/tablespace.source index a5f61a35dc..d2dacd85d9 100644 --- a/src/test/regress/input/tablespace.source +++ b/src/test/regress/input/tablespace.source @@ -17,6 +17,34 @@ 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 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 SCHEMA pg_catalog TABLESPACE regress_tblspace; -- fail +REINDEX DATABASE postgres TABLESPACE regress_tblspace; -- fail +REINDEX SYSTEM postgres 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'); + +-- move back to pg_default tablespace +REINDEX TABLE CONCURRENTLY regress_tblspace_test_tbl 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') +AND relname IN ('regress_tblspace_test_tbl_idx'); + -- create a schema we can use CREATE SCHEMA testschema; @@ -279,6 +307,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..fb2f33c0b1 100644 --- a/src/test/regress/output/tablespace.source +++ b/src/test/regress/output/tablespace.source @@ -20,6 +20,45 @@ 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 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 SCHEMA pg_catalog TABLESPACE regress_tblspace; -- fail +ERROR: incompatible TABLESPACE option +DETAIL: You can only use TABLESPACE with REINDEX { INDEX | TABLE }. +REINDEX DATABASE postgres TABLESPACE regress_tblspace; -- fail +ERROR: incompatible TABLESPACE option +DETAIL: You can only use TABLESPACE with REINDEX { INDEX | TABLE }. +REINDEX SYSTEM postgres TABLESPACE regress_tblspace; -- fail +ERROR: incompatible TABLESPACE option +DETAIL: You can only use TABLESPACE with REINDEX { INDEX | TABLE }. +-- 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'); + relname +------------------------------- + regress_tblspace_test_tbl_idx +(1 row) + +-- move back to pg_default tablespace +REINDEX TABLE CONCURRENTLY regress_tblspace_test_tbl 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') +AND relname IN ('regress_tblspace_test_tbl_idx'); + relname +--------- +(0 rows) + -- create a schema we can use CREATE SCHEMA testschema; -- try a table @@ -736,6 +775,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 base-commit: 6d05086c0a79e50d8e91ed953626ec7280cd2481 -- 2.17.1 --------------0FADFCA1768281ADAB10366C Content-Type: text/x-patch; name="v3-0002-Use-SET-TABLESPACE-syntax-with-NOWAIT-option.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename*0="v3-0002-Use-SET-TABLESPACE-syntax-with-NOWAIT-option.patch" From c9c072748b54e6dbde8bde5d932af52c229141f5 Mon Sep 17 00:00:00 2001 From: Alexey Kondratov Date: Wed, 18 Sep 2019 15:22:04 +0300 Subject: [PATCH v3 2/2] Use SET TABLESPACE syntax with NOWAIT option --- doc/src/sgml/ref/reindex.sgml | 8 ++++--- src/backend/commands/indexcmds.c | 4 ++-- src/backend/parser/gram.y | 26 +++++++++++++------- src/backend/tcop/utility.c | 10 ++++++-- src/include/nodes/parsenodes.h | 1 + src/test/regress/input/tablespace.source | 15 ++++++------ src/test/regress/output/tablespace.source | 29 +++++++++++++---------- 7 files changed, 57 insertions(+), 36 deletions(-) diff --git a/doc/src/sgml/ref/reindex.sgml b/doc/src/sgml/ref/reindex.sgml index 96c9363ad9..4f0f21b7f9 100644 --- a/doc/src/sgml/ref/reindex.sgml +++ b/doc/src/sgml/ref/reindex.sgml @@ -22,7 +22,7 @@ PostgreSQL documentation REINDEX [ ( VERBOSE ) ] { INDEX | TABLE | SCHEMA | DATABASE | SYSTEM } [ CONCURRENTLY ] name -REINDEX [ ( VERBOSE ) ] { INDEX | TABLE } [ CONCURRENTLY ] name [ TABLESPACE new_tablespace ] +REINDEX [ ( VERBOSE ) ] { INDEX | TABLE } [ CONCURRENTLY ] name [ SET TABLESPACE new_tablespace [NOWAIT] ] @@ -167,14 +167,16 @@ REINDEX [ ( VERBOSE ) ] { INDEX | TABLE } [ CONCURRENTLY ] options & REINDEXOPT_NOWAIT && stmt->tablespacename == NULL) + ereport(ERROR, + (errcode(ERRCODE_SYNTAX_ERROR), + errmsg("incompatible NOWAIT option"), + errdetail("You can only use NOWAIT with SET TABLESPACE."))); + /* we choose to allow this during "read only" transactions */ PreventCommandDuringRecovery("REINDEX"); /* forbidden in parallel mode due to CommandIsReadOnly */ @@ -800,8 +806,8 @@ standard_ProcessUtility(PlannedStmt *pstmt, */ if (stmt->tablespacename) ereport(ERROR, - (errmsg("incompatible TABLESPACE option"), - errdetail("You can only use TABLESPACE with REINDEX { INDEX | TABLE }."))); + (errmsg("incompatible SET TABLESPACE option"), + errdetail("You can only use SET TABLESPACE with REINDEX { INDEX | TABLE }."))); /* * This cannot run inside a user transaction block; if diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index ed2c385050..2fef8b249d 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -3314,6 +3314,7 @@ typedef struct ConstraintsSetStmt /* Reindex options */ #define REINDEXOPT_VERBOSE (1 << 0) /* print progress info */ #define REINDEXOPT_REPORT_PROGRESS (1 << 1) /* report pgstat progress */ +#define REINDEXOPT_NOWAIT (1 << 2) /* error if relation cannot be locked */ typedef enum ReindexObjectType { diff --git a/src/test/regress/input/tablespace.source b/src/test/regress/input/tablespace.source index d2dacd85d9..0b9df9f0d8 100644 --- a/src/test/regress/input/tablespace.source +++ b/src/test/regress/input/tablespace.source @@ -25,12 +25,13 @@ INSERT INTO regress_tblspace_test_tbl (num1, num2, num3) CREATE INDEX regress_tblspace_test_tbl_idx ON regress_tblspace_test_tbl (num1); -- 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 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 SET TABLESPACE regress_tblspace; -- ok +REINDEX TABLE regress_tblspace_test_tbl SET TABLESPACE regress_tblspace; -- ok +REINDEX TABLE pg_authid NOWAIT; -- fail +REINDEX TABLE pg_authid SET TABLESPACE regress_tblspace; -- fail +REINDEX SCHEMA pg_catalog SET TABLESPACE regress_tblspace; -- fail +REINDEX DATABASE postgres SET TABLESPACE regress_tblspace; -- fail +REINDEX SYSTEM postgres SET TABLESPACE regress_tblspace; -- fail -- check that all relations moved to new tablespace SELECT relname FROM pg_class @@ -38,7 +39,7 @@ WHERE reltablespace=(SELECT oid FROM pg_tablespace WHERE spcname='regress_tblspa AND relname IN ('regress_tblspace_test_tbl_idx'); -- move back to pg_default tablespace -REINDEX TABLE CONCURRENTLY regress_tblspace_test_tbl TABLESPACE pg_default; -- ok +REINDEX TABLE CONCURRENTLY regress_tblspace_test_tbl SET TABLESPACE pg_default NOWAIT; -- ok -- check that all relations moved back to pg_default SELECT relname FROM pg_class diff --git a/src/test/regress/output/tablespace.source b/src/test/regress/output/tablespace.source index fb2f33c0b1..4b5a1ae2fb 100644 --- a/src/test/regress/output/tablespace.source +++ b/src/test/regress/output/tablespace.source @@ -27,19 +27,22 @@ INSERT INTO regress_tblspace_test_tbl (num1, num2, num3) FROM generate_series(1, 20000) s(i); CREATE INDEX regress_tblspace_test_tbl_idx ON regress_tblspace_test_tbl (num1); -- 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 INDEX regress_tblspace_test_tbl_idx SET TABLESPACE regress_tblspace; -- ok +REINDEX TABLE regress_tblspace_test_tbl SET TABLESPACE regress_tblspace; -- ok +REINDEX TABLE pg_authid NOWAIT; -- fail +ERROR: incompatible NOWAIT option +DETAIL: You can only use NOWAIT with SET TABLESPACE. +REINDEX TABLE pg_authid SET TABLESPACE regress_tblspace; -- fail ERROR: cannot move system relation "pg_authid_rolname_index" -REINDEX SCHEMA pg_catalog TABLESPACE regress_tblspace; -- fail -ERROR: incompatible TABLESPACE option -DETAIL: You can only use TABLESPACE with REINDEX { INDEX | TABLE }. -REINDEX DATABASE postgres TABLESPACE regress_tblspace; -- fail -ERROR: incompatible TABLESPACE option -DETAIL: You can only use TABLESPACE with REINDEX { INDEX | TABLE }. -REINDEX SYSTEM postgres TABLESPACE regress_tblspace; -- fail -ERROR: incompatible TABLESPACE option -DETAIL: You can only use TABLESPACE with REINDEX { INDEX | TABLE }. +REINDEX SCHEMA pg_catalog SET TABLESPACE regress_tblspace; -- fail +ERROR: incompatible SET TABLESPACE option +DETAIL: You can only use SET TABLESPACE with REINDEX { INDEX | TABLE }. +REINDEX DATABASE postgres SET TABLESPACE regress_tblspace; -- fail +ERROR: incompatible SET TABLESPACE option +DETAIL: You can only use SET TABLESPACE with REINDEX { INDEX | TABLE }. +REINDEX SYSTEM postgres SET TABLESPACE regress_tblspace; -- fail +ERROR: incompatible SET TABLESPACE option +DETAIL: You can only use SET TABLESPACE with REINDEX { INDEX | TABLE }. -- check that all relations moved to new tablespace SELECT relname FROM pg_class WHERE reltablespace=(SELECT oid FROM pg_tablespace WHERE spcname='regress_tblspace') @@ -50,7 +53,7 @@ AND relname IN ('regress_tblspace_test_tbl_idx'); (1 row) -- move back to pg_default tablespace -REINDEX TABLE CONCURRENTLY regress_tblspace_test_tbl TABLESPACE pg_default; -- ok +REINDEX TABLE CONCURRENTLY regress_tblspace_test_tbl SET TABLESPACE pg_default NOWAIT; -- 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') -- 2.17.1 --------------0FADFCA1768281ADAB10366C--