On 27.11.2019 6:54, Michael Paquier wrote:
On Tue, Nov 26, 2019 at 11:09:55PM +0100, Masahiko Sawada wrote:
I looked at v4 patch. Here are some comments:
+ /* Skip all mapped relations if TABLESPACE is specified */
+ if (OidIsValid(tableSpaceOid) &&
+ classtuple->relfilenode == 0)
I think we can use OidIsValid(classtuple->relfilenode) instead.
Yes, definitely.
Yes, switched to !OidIsValid(classtuple->relfilenode). Also I added a
comment that it is meant to be equivalent to RelationIsMapped() and
extended tests.
This change says that temporary relation is not supported but it
actually seems to work. Which is correct?
Yeah, I don't really see a reason why it would not work.
My bad, I was keeping in mind RELATION_IS_OTHER_TEMP validation, but it
is for temp tables of other backends only, so it definitely should not
be in the doc. Removed.
Your patch has forgotten to update copyfuncs.c and equalfuncs.c with
the new tablespace string field.
Fixed, thanks.
It would be nice to add tab completion for this new clause in psql.
Added.
There is no need for opt_tablespace_name as new node for the parsing
grammar of gram.y as OptTableSpace is able to do the exact same job.
Sure, it was an artifact from the times, where I used optional SET
TABLESPACE clause. Removed.
@@ -3455,6 +3461,8 @@ RelationSetNewRelfilenode(Relation relation,
char persistence)
*/
newrnode = relation->rd_node;
newrnode.relNode = newrelfilenode;
+ if (OidIsValid(tablespaceOid))
+ newrnode.spcNode = newTablespaceOid;
The core of the patch is actually here. It seems to me that this is a
very bad idea because you actually hijack a logic which happens at a
much lower level which is based on the state of the tablespace stored
in the relation cache entry of the relation being reindexed, then the
tablespace choice actually happens in RelationInitPhysicalAddr() which
for the new relfilenode once the follow-up CCI is done. So this very
likely needs more thoughts, and bringing to the point: shouldn't you
actually be careful that the relation tablespace is correctly updated
before reindexing it and before creating its new relfilenode? This
way, RelationSetNewRelfilenode() does not need any additional work,
and I think that this saves from potential bugs in the choice of the
tablespace used with the new relfilenode.
When I did the first version of the patch I was looking on
ATExecSetTableSpace, which implements ALTER ... SET TABLESPACE. And
there is very similar pipeline there:
1) Find pg_class entry with SearchSysCacheCopy1
2) Create new relfilenode with GetNewRelFileNode
3) Set new tablespace for this relfilenode
4) Do some work with new relfilenode
5) Update pg_class entry with new tablespace
6) Do CommandCounterIncrement
The only difference is that point 3) and tablespace part of 5) were
missing in RelationSetNewRelfilenode, so I added them, and I do 4) after
6) in REINDEX. Thus, it seems that in my implementation of tablespace
change in REINDEX I am more sure that "the relation tablespace is
correctly updated before reindexing", since I do reindex after CCI
(point 6), doesn't it?
So why it is fine for ATExecSetTableSpace to do pretty much the same,
but not for REINDEX? Or the key point is in doing actual work before
CCI, but for me it seems a bit against what you have wrote?
Thus, I cannot get your point correctly here. Can you, please, elaborate
a little bit more your concerns?
ISTM the kind of above errors are the same: the given tablespace
exists but moving tablespace to it is not allowed since it's not
supported in PostgreSQL. So I think we can use
ERRCODE_FEATURE_NOT_SUPPORTED instead of
ERRCODE_INVALID_PARAMETER_VALUE (which is used at 3 places) .
Yes, it is also not project style to use full sentences in error
messages, so I would suggest instead (note the missing quotes in the
original patch):
cannot move non-shared relation to tablespace \"%s\"
Same here. I have taken this validation directly from tablecmds.c part
for ALTER ... SET TABLESPACE. And there is exactly the same message
"only shared relations can be placed in pg_global tablespace" with
ERRCODE_INVALID_PARAMETER_VALUE there.
However, I understand your point, but still, would it be better if I
stick to the same ERRCODE/message? Or should I introduce new
ERRCODE/message for the same case?
And I have somewhat missed to notice the timing of the review replies
as you did not have room to reply, so fixed the CF entry to "waiting
on author", and bumped it to next CF instead.
Thank you! Attached is a patch, that addresses all the issues above,
excepting the last two points (core part and error messages for
pg_global), which are not clear for me right now.
--
Alexey Kondratov
Postgres Professional https://www.postgrespro.com
Russian Postgres Company
>From 6b76365b462c9332ced841bdfd0f80d862082156 Mon Sep 17 00:00:00 2001
From: Alexey Kondratov <kondratov.alek...@gmail.com>
Date: Wed, 27 Nov 2019 20:40:17 +0300
Subject: [PATCH v5] 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 | 91 ++++++++++++++++++++---
src/backend/commands/sequence.c | 8 +-
src/backend/commands/tablecmds.c | 9 ++-
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/backend/utils/cache/relcache.c | 18 ++++-
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/include/utils/relcache.h | 2 +-
src/test/regress/input/tablespace.source | 49 ++++++++++++
src/test/regress/output/tablespace.source | 66 ++++++++++++++++
18 files changed, 293 insertions(+), 44 deletions(-)
diff --git a/doc/src/sgml/ref/reindex.sgml b/doc/src/sgml/ref/reindex.sgml
index 10881ab03a8..3985edf487a 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" 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..24fbcc3b291 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,26 @@ 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.
+ * This is meant to be equivalent to RelationIsMapped().
+ */
+ if (OidIsValid(tableSpaceOid) &&
+ !OidIsValid(classtuple->relfilenode))
+ {
+ 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 +2703,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 +2711,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 +2752,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 +2825,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 +2999,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 +3024,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 5440eb90153..1ba83554f60 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/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 2f267e4bb65..d932c4cbbdb 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -4386,6 +4386,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 da0e1d139ac..31f9a6ac196 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -2116,6 +2116,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 c5086846dec..bc4178e0cb0 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -8358,31 +8358,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;
@@ -8390,9 +8392,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;
@@ -8400,6 +8403,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/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index df268269939..3c650f480a3 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -3360,6 +3360,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 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..7d698d4294d 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 162b591b315..a892be124e6 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 move indexes of system relations, skipping all
+REINDEX DATABASE regression TABLESPACE pg_default; -- ok with warning
+WARNING: cannot move indexes of system relations, skipping all
+SELECT relname FROM pg_class
+WHERE reltablespace=(SELECT oid FROM pg_tablespace WHERE spcname='regress_tblspace');
+ relname
+---------
+(0 rows)
+
+-- check REINDEX with TABLESPACE change
+REINDEX 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: only shared relations can be placed in pg_global tablespace
+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.1
