Hi Steve,
Thank you for review.
On 17.11.2019 3:53, Steve Singer wrote:
The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, failed
Spec compliant: not tested
Documentation: tested, failed
* I had to replace heap_open/close with table_open/close to get the
patch to compile against master
In the documentation
+ <para>
+ This specifies a tablespace, where all rebuilt indexes will be created.
+ Can be used only with <literal>REINDEX INDEX</literal> and
+ <literal>REINDEX TABLE</literal>, since the system indexes are not
+ movable, but <literal>SCHEMA</literal>, <literal>DATABASE</literal> or
+ <literal>SYSTEM</literal> very likely will has one.
+ </para>
I found the "SCHEMA,DATABASE or SYSTEM very likely will has one." portion
confusing and would be inclined to remove it or somehow reword it.
In the attached new version REINDEX with TABLESPACE and {SCHEMA,
DATABASE, SYSTEM} now behaves more like with CONCURRENTLY, i.e. it skips
unsuitable relations and shows warning. So this section in docs has been
updated as well.
Also the whole patch has been reworked. I noticed that my code in
reindex_index was doing pretty much the same as inside
RelationSetNewRelfilenode. So I just added a possibility to specify new
tablespace for RelationSetNewRelfilenode instead. Thus, even with
addition of new tests the patch becomes less complex.
Consider the following
-------------
create index foo_bar_idx on foo(bar) tablespace pg_default;
CREATE INDEX
reindex=# \d foo
Table "public.foo"
Column | Type | Collation | Nullable | Default
--------+---------+-----------+----------+---------
id | integer | | not null |
bar | text | | |
Indexes:
"foo_pkey" PRIMARY KEY, btree (id)
"foo_bar_idx" btree (bar)
reindex=# reindex index foo_bar_idx tablespace tst1;
REINDEX
reindex=# reindex index foo_bar_idx tablespace pg_default;
REINDEX
reindex=# \d foo
Table "public.foo"
Column | Type | Collation | Nullable | Default
--------+---------+-----------+----------+---------
id | integer | | not null |
bar | text | | |
Indexes:
"foo_pkey" PRIMARY KEY, btree (id)
"foo_bar_idx" btree (bar), tablespace "pg_default"
--------
It is a bit strange that it says "pg_default" as the tablespace. If I do
this with a alter table to the table, moving the table back to pg_default
makes it look as it did before.
Otherwise the first patch seems fine.
Yes, I missed the fact that default tablespace of database is stored
implicitly as InvalidOid, but I was setting it explicitly as specified.
I have changed this behavior to stay consistent with ALTER TABLE.
With the second patch(for NOWAIT) I did the following
T1: begin;
T1: insert into foo select generate_series(1,1000);
T2: reindex index foo_bar_idx set tablespace tst1 nowait;
T2 is waiting for a lock. This isn't what I would expect.
Indeed, I have added nowait option for RangeVarGetRelidExtended, so it
should not wait if index is locked. However, for reindex we also have to
put share lock on the parent table relation, which is done by opening it
via table_open(heapId, ShareLock).
The only one solution I can figure out right now is to wrap all such
opens with ConditionalLockRelationOid(relId, ShareLock) and then do
actual open with NoLock. This is how something similar is implemented in
VACUUM if VACOPT_SKIP_LOCKED is specified. However, there are multiple
code paths with table_open, so it becomes a bit ugly.
I will leave the second patch aside for now and experiment with it.
Actually, its main idea was to mimic ALTER INDEX ... SET TABLESPACE
[NOWAIT] syntax, but probably it is better to stick with more brief
plain TABLESPACE like in CREATE INDEX.
Regards
--
Alexey Kondratov
Postgres Professional https://www.postgrespro.com
Russian Postgres Company
P.S. I have also added all previous thread participants to CC in order to do
not split the thread. Sorry if it was a bad idea.
>From 22990d58fb549536ca33a1b02c5a21a248deee5d Mon Sep 17 00:00:00 2001
From: Alexey Kondratov <kondratov.alek...@gmail.com>
Date: Wed, 20 Nov 2019 20:09:50 +0300
Subject: [PATCH v4] Allow REINDEX and REINDEX CONCURRENTLY to change
TABLESPACE
---
doc/src/sgml/ref/reindex.sgml | 24 ++++++-
src/backend/catalog/index.c | 26 +++++--
src/backend/commands/cluster.c | 2 +-
src/backend/commands/indexcmds.c | 88 +++++++++++++++++++----
src/backend/commands/sequence.c | 8 ++-
src/backend/commands/tablecmds.c | 9 ++-
src/backend/parser/gram.y | 21 ++++--
src/backend/tcop/utility.c | 6 +-
src/backend/utils/cache/relcache.c | 18 ++++-
src/include/catalog/index.h | 7 +-
src/include/commands/defrem.h | 6 +-
src/include/nodes/parsenodes.h | 1 +
src/include/utils/relcache.h | 2 +-
src/test/regress/input/tablespace.source | 42 +++++++++++
src/test/regress/output/tablespace.source | 53 ++++++++++++++
15 files changed, 268 insertions(+), 45 deletions(-)
diff --git a/doc/src/sgml/ref/reindex.sgml b/doc/src/sgml/ref/reindex.sgml
index 10881ab03a8..d0f73459e33 100644
--- a/doc/src/sgml/ref/reindex.sgml
+++ b/doc/src/sgml/ref/reindex.sgml
@@ -21,7 +21,7 @@ PostgreSQL documentation
<refsynopsisdiv>
<synopsis>
-REINDEX [ ( VERBOSE ) ] { INDEX | TABLE | SCHEMA | DATABASE | SYSTEM } [ CONCURRENTLY ] <replaceable class="parameter">name</replaceable>
+REINDEX [ ( VERBOSE ) ] { INDEX | TABLE | SCHEMA | DATABASE | SYSTEM } [ CONCURRENTLY ] <replaceable class="parameter">name</replaceable> [ TABLESPACE <replaceable class="parameter">new_tablespace</replaceable> ]
</synopsis>
</refsynopsisdiv>
@@ -165,6 +165,28 @@ REINDEX [ ( VERBOSE ) ] { INDEX | TABLE | SCHEMA | DATABASE | SYSTEM } [ CONCURR
</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" and temporary 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 67f637de11d..7130a7b3d1b 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -1238,7 +1238,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,
@@ -1368,7 +1369,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,
@@ -3352,7 +3353,7 @@ 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,
@@ -3400,6 +3401,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.
@@ -3442,7 +3453,7 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
SetReindexProcessing(heapId, indexId);
/* Create a new physical relation for the index */
- RelationSetNewRelfilenode(iRel, persistence);
+ RelationSetNewRelfilenode(iRel, persistence, tablespaceOid);
/* Initialize the index and rebuild */
/* Note: we do not need to re-establish pkey setting */
@@ -3585,7 +3596,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;
@@ -3659,7 +3670,8 @@ 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();
@@ -3692,7 +3704,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 b8c349f245b..ba018867b68 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 374e2d0efe4..efff04d457d 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);
@@ -2314,10 +2314,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;
@@ -2347,12 +2348,24 @@ ReindexIndex(RangeVar *indexRelation, int options, bool concurrent)
}
persistence = irel->rd_rel->relpersistence;
+
+ 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_INVALID_PARAMETER_VALUE),
+ errmsg("only shared relations can be placed in pg_global tablespace")));
+ }
+
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);
}
@@ -2431,10 +2444,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,
@@ -2442,9 +2456,20 @@ ReindexTable(RangeVar *relation, int options, bool concurrent)
0,
RangeVarCallbackOwnsTable, NULL);
+ 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_INVALID_PARAMETER_VALUE),
+ errmsg("only shared relations can be placed in pg_global tablespace")));
+ }
+
if (concurrent)
{
- result = ReindexRelationConcurrently(heapOid, options);
+ result = ReindexRelationConcurrently(heapOid, tableSpaceOid, options);
if (!result)
ereport(NOTICE,
@@ -2454,6 +2479,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);
@@ -2475,10 +2501,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];
@@ -2488,7 +2515,7 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
List *relids = NIL;
ListCell *l;
int num_keys;
- bool concurrent_warning = false;
+ bool system_warning = false;
AssertArg(objectName);
Assert(objectKind == REINDEX_OBJECT_SCHEMA ||
@@ -2527,6 +2554,17 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
objectName);
}
+ 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_INVALID_PARAMETER_VALUE),
+ errmsg("only shared relations can be placed in pg_global tablespace")));
+ }
+
/*
* Create a memory context that will survive forced transaction commits we
* do below. Since it is a child of PortalContext, it will go away
@@ -2609,11 +2647,23 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
if (concurrent &&
IsCatalogRelationOid(relid))
{
- if (!concurrent_warning)
+ if (!system_warning)
ereport(WARNING,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("cannot reindex system catalogs concurrently, skipping all")));
- concurrent_warning = true;
+ system_warning = true;
+ continue;
+ }
+
+ /* Skip all mapped relations if TABLESPACE is specified */
+ if (OidIsValid(tableSpaceOid) &&
+ classtuple->relfilenode == 0)
+ {
+ if (!system_warning)
+ ereport(WARNING,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("cannot move indexes of system relations, skipping all")));
+ system_warning = true;
continue;
}
@@ -2650,7 +2700,7 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
if (concurrent)
{
- (void) ReindexRelationConcurrently(relid, options);
+ (void) ReindexRelationConcurrently(relid, tableSpaceOid, options);
/* ReindexRelationConcurrently() does the verbose output */
}
else
@@ -2658,6 +2708,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);
@@ -2698,7 +2749,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;
@@ -2771,6 +2822,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 move indexes of system relation \"%s\"",
+ RelationGetRelationName(heapRelation))));
+
/* Add all the valid indexes of relation to list */
foreach(lc, RelationGetIndexList(heapRelation))
{
@@ -2939,6 +2996,12 @@ ReindexRelationConcurrently(Oid relationOid, int options)
heapRel = table_open(indexRel->rd_index->indrelid,
ShareUpdateExclusiveLock);
+ if (OidIsValid(tablespaceOid) && RelationIsMapped(heapRel))
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("cannot move system relation \"%s\"",
+ RelationGetRelationName(heapRel))));
+
pgstat_progress_start_command(PROGRESS_COMMAND_CREATE_INDEX,
RelationGetRelid(heapRel));
pgstat_progress_update_param(PROGRESS_CREATEIDX_COMMAND,
@@ -2958,6 +3021,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/sequence.c b/src/backend/commands/sequence.c
index a13322b6938..3422122d58f 100644
--- a/src/backend/commands/sequence.c
+++ b/src/backend/commands/sequence.c
@@ -314,7 +314,9 @@ ResetSequence(Oid seq_relid)
/*
* Create a new storage file for the sequence.
*/
- RelationSetNewRelfilenode(seq_rel, seq_rel->rd_rel->relpersistence);
+ RelationSetNewRelfilenode(seq_rel,
+ seq_rel->rd_rel->relpersistence,
+ InvalidOid);
/*
* Ensure sequence's relfrozenxid is at 0, since it won't contain any
@@ -489,7 +491,9 @@ AlterSequence(ParseState *pstate, AlterSeqStmt *stmt)
* Create a new storage file for the sequence, making the state
* changes transactional.
*/
- RelationSetNewRelfilenode(seqrel, seqrel->rd_rel->relpersistence);
+ RelationSetNewRelfilenode(seqrel,
+ seqrel->rd_rel->relpersistence,
+ InvalidOid);
/*
* Ensure sequence's relfrozenxid is at 0, since it won't contain any
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 45aae5955d0..80df1adbe58 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -1802,7 +1802,9 @@ ExecuteTruncateGuts(List *explicit_rels, List *relids, List *relids_logged,
* as the relfilenode value. The old storage file is scheduled for
* deletion at commit.
*/
- RelationSetNewRelfilenode(rel, rel->rd_rel->relpersistence);
+ RelationSetNewRelfilenode(rel,
+ rel->rd_rel->relpersistence,
+ InvalidOid);
heap_relid = RelationGetRelid(rel);
@@ -1816,14 +1818,15 @@ ExecuteTruncateGuts(List *explicit_rels, List *relids, List *relids_logged,
AccessExclusiveLock);
RelationSetNewRelfilenode(toastrel,
- toastrel->rd_rel->relpersistence);
+ toastrel->rd_rel->relpersistence,
+ InvalidOid);
table_close(toastrel, NoLock);
}
/*
* 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/parser/gram.y b/src/backend/parser/gram.y
index 2f7bd662e8a..f1c775c6528 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -551,7 +551,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
%type <list> constraints_set_list
%type <boolean> constraints_set_mode
-%type <str> OptTableSpace OptConsTableSpace
+%type <str> OptTableSpace OptConsTableSpace opt_tablespace_name
%type <rolespec> OptTableSpaceOwner
%type <ival> opt_check_option
@@ -3939,6 +3939,11 @@ OptTableSpace: TABLESPACE name { $$ = $2; }
| /*EMPTY*/ { $$ = NULL; }
;
+opt_tablespace_name:
+ TABLESPACE name { $$ = $2; }
+ | /*EMPTY*/ { $$ = NULL; }
+ ;
+
OptConsTableSpace: USING INDEX TABLESPACE name { $$ = $4; }
| /*EMPTY*/ { $$ = NULL; }
;
@@ -8358,31 +8363,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 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;
@@ -8390,9 +8397,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;
@@ -8400,6 +8408,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 3a03ca7e2f0..63fc4f95ab9 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -779,10 +779,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:
@@ -798,7 +798,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/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
index ad1ff01b320..070d4b947dd 100644
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -3403,7 +3403,9 @@ RelationBuildLocalRelation(const char *relname,
* RelationSetNewRelfilenode
*
* Assign a new relfilenode (physical file name), and possibly a new
- * persistence setting, to the relation.
+ * persistence setting, to the relation. If tablespaceOid is different
+ * from InvalidOid, then new filenode will be created there instead of
+ * current location.
*
* This allows a full rewrite of the relation to be done with transactional
* safety (since the filenode assignment can be rolled back). Note however
@@ -3414,7 +3416,7 @@ RelationBuildLocalRelation(const char *relname,
* Caller must already hold exclusive lock on the relation.
*/
void
-RelationSetNewRelfilenode(Relation relation, char persistence)
+RelationSetNewRelfilenode(Relation relation, char persistence, Oid tablespaceOid)
{
Oid newrelfilenode;
Relation pg_class;
@@ -3423,9 +3425,13 @@ RelationSetNewRelfilenode(Relation relation, char persistence)
MultiXactId minmulti = InvalidMultiXactId;
TransactionId freezeXid = InvalidTransactionId;
RelFileNode newrnode;
+ Oid newTablespaceOid = relation->rd_rel->reltablespace;
+
+ if (OidIsValid(tablespaceOid))
+ newTablespaceOid = tablespaceOid;
/* Allocate a new relfilenode */
- newrelfilenode = GetNewRelFileNode(relation->rd_rel->reltablespace, NULL,
+ newrelfilenode = GetNewRelFileNode(newTablespaceOid, NULL,
persistence);
/*
@@ -3455,6 +3461,8 @@ RelationSetNewRelfilenode(Relation relation, char persistence)
*/
newrnode = relation->rd_node;
newrnode.relNode = newrelfilenode;
+ if (OidIsValid(tablespaceOid))
+ newrnode.spcNode = newTablespaceOid;
switch (relation->rd_rel->relkind)
{
@@ -3525,6 +3533,10 @@ RelationSetNewRelfilenode(Relation relation, char persistence)
/* Normal case, update the pg_class entry */
classform->relfilenode = newrelfilenode;
+ /* Update tablespace in pg_class entry if it is changed */
+ if (OidIsValid(tablespaceOid))
+ classform->reltablespace = (newTablespaceOid == MyDatabaseTableSpace) ? InvalidOid : tablespaceOid;
+
/* relpages etc. never change for sequences */
if (relation->rd_rel->relkind != RELKIND_SEQUENCE)
{
diff --git a/src/include/catalog/index.h b/src/include/catalog/index.h
index 1113d25b2d8..ef6103a1747 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 1dc6dc2ca04..36878728694 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 ff626cbe61e..62cc4c0013b 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -3334,6 +3334,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/include/utils/relcache.h b/src/include/utils/relcache.h
index 2f2ace35b05..6c326a26333 100644
--- a/src/include/utils/relcache.h
+++ b/src/include/utils/relcache.h
@@ -107,7 +107,7 @@ extern Relation RelationBuildLocalRelation(const char *relname,
/*
* Routine to manage assignment of new relfilenode to a relation
*/
-extern void RelationSetNewRelfilenode(Relation relation, char persistence);
+extern void RelationSetNewRelfilenode(Relation relation, char persistence, Oid tablespaceOid);
/*
* Routines for flushing/rebuilding relcache entries in various scenarios
diff --git a/src/test/regress/input/tablespace.source b/src/test/regress/input/tablespace.source
index a5f61a35dc5..07732ad3dc5 100644
--- a/src/test/regress/input/tablespace.source
+++ b/src/test/regress/input/tablespace.source
@@ -17,6 +17,45 @@ 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');
+
+-- 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 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 +318,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 162b591b315..3ce973b5769 100644
--- a/src/test/regress/output/tablespace.source
+++ b/src/test/regress/output/tablespace.source
@@ -20,6 +20,57 @@ 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)
+
+-- 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 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 +787,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.1
