On 2021-01-22 00:26, Justin Pryzby wrote:
On Thu, Jan 21, 2021 at 11:48:08PM +0300, Alexey Kondratov wrote:
Attached is a new patch set of first two patches, that should resolve
all
the issues raised before (ACL, docs, tests) excepting TOAST. Double
thanks
for suggestion to add more tests with nested partitioning. I have
found and
squashed a huge bug related to the returning back to the default
tablespace
using newly added tests.
Regarding TOAST. Now we skip moving toast indexes or throw error if
someone
wants to move TOAST index directly. I had a look on ALTER TABLE SET
TABLESPACE and it has a bit complicated logic:
1) You cannot move TOAST table directly.
2) But if you move basic relation that TOAST table belongs to, then
they are
moved altogether.
3) Same logic as 2) happens if one does ALTER TABLE ALL IN TABLESPACE
...
That way, ALTER TABLE allows moving TOAST tables (with indexes)
implicitly,
but does not allow doing that explicitly. In the same time I found
docs to
be vague about such behavior it only says:
All tables in the current database in a tablespace can be moved
by using the ALL IN TABLESPACE ... Note that system catalogs are
not moved by this command
Changing any part of a system catalog table is not permitted.
So actually ALTER TABLE treats TOAST relations as system sometimes,
but
sometimes not.
From the end user perspective it makes sense to move TOAST with main
table
when doing ALTER TABLE SET TABLESPACE. But should we touch indexes on
TOAST
table with REINDEX? We cannot move TOAST relation itself, since we are
doing
only a reindex, so we end up in the state when TOAST table and its
index are
placed in the different tablespaces. This state is not reachable with
ALTER
TABLE/INDEX, so it seem we should not allow it with REINDEX as well,
should
we?
+ * Even if a table's indexes were moved to a new tablespace, the
index
+ * on its toast table is not normally moved.
*/
ReindexParams newparams = *params;
newparams.options &= ~(REINDEXOPT_MISSING_OK);
+ if (!allowSystemTableMods)
+ newparams.tablespaceOid = InvalidOid;
I think you're right. So actually TOAST should never move, even if
allowSystemTableMods, right ?
I think so. I would prefer to do not move TOAST indexes implicitly at
all during reindex.
@@ -292,7 +315,11 @@ REINDEX [ ( <replaceable
class="parameter">option</replaceable> [, ...] ) ] { IN
with <command>REINDEX INDEX</command> or <command>REINDEX
TABLE</command>,
respectively. Each partition of the specified partitioned relation
is
reindexed in a separate transaction. Those commands cannot be used
inside
- a transaction block when working on a partitioned table or index.
+ a transaction block when working on a partitioned table or index.
If
+ <command>REINDEX</command> with <literal>TABLESPACE</literal>
executed
+ on partitioned relation fails it may have moved some partitions to
the new
+ tablespace. Repeated command will still reindex all partitions
even if they
+ are already in the new tablespace.
Minor corrections here:
If a <command>REINDEX</command> command fails when run on a partitioned
relation, and <literal>TABLESPACE</literal> was specified, then it may
have
moved indexes on some partitions to the new tablespace. Re-running the
command
will reindex all partitions and move previously-unprocessed indexes to
the new
tablespace.
Sounds good to me.
I have updated patches accordingly and also simplified tablespaceOid
checks and assignment in the newly added SetRelTableSpace(). Result is
attached as two separate patches for an ease of review, but no
objections to merge them and apply at once if everything is fine.
Regards
--
Alexey Kondratov
Postgres Professional https://www.postgrespro.com
Russian Postgres Company
From 87e47e9b5b3d6b49230045e5db8f844b14b34ba0 Mon Sep 17 00:00:00 2001
From: Alexey Kondratov <kondratov.alek...@gmail.com>
Date: Mon, 23 Mar 2020 21:10:29 +0300
Subject: [PATCH v4 2/2] Allow REINDEX to change tablespace
REINDEX already does full relation rewrite, this patch adds a
possibility to specify a new tablespace where new relfilenode
will be created.
---
doc/src/sgml/ref/reindex.sgml | 30 +++-
src/backend/catalog/index.c | 81 ++++++++++-
src/backend/commands/indexcmds.c | 81 ++++++++++-
src/bin/psql/tab-complete.c | 4 +-
src/include/catalog/index.h | 2 +
src/test/regress/input/tablespace.source | 85 ++++++++++++
src/test/regress/output/tablespace.source | 159 ++++++++++++++++++++++
7 files changed, 436 insertions(+), 6 deletions(-)
diff --git a/doc/src/sgml/ref/reindex.sgml b/doc/src/sgml/ref/reindex.sgml
index 627b36300c..a1c7736aec 100644
--- a/doc/src/sgml/ref/reindex.sgml
+++ b/doc/src/sgml/ref/reindex.sgml
@@ -27,6 +27,7 @@ REINDEX [ ( <replaceable class="parameter">option</replaceable> [, ...] ) ] { IN
CONCURRENTLY [ <replaceable class="parameter">boolean</replaceable> ]
VERBOSE [ <replaceable class="parameter">boolean</replaceable> ]
+ TABLESPACE <replaceable class="parameter">new_tablespace</replaceable>
</synopsis>
</refsynopsisdiv>
@@ -187,6 +188,20 @@ REINDEX [ ( <replaceable class="parameter">option</replaceable> [, ...] ) ] { IN
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><literal>TABLESPACE</literal></term>
+ <listitem>
+ <para>
+ Specifies that indexes will be rebuilt on a new tablespace.
+ Cannot be used with "mapped" and system (unless <varname>allow_system_table_mods</varname>
+ is set to <literal>TRUE</literal>) relations. If <literal>SCHEMA</literal>,
+ <literal>DATABASE</literal> or <literal>SYSTEM</literal> are specified, then
+ all "mapped" and system relations will be skipped and a single
+ <literal>WARNING</literal> will be generated.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry>
<term><literal>VERBOSE</literal></term>
<listitem>
@@ -210,6 +225,14 @@ REINDEX [ ( <replaceable class="parameter">option</replaceable> [, ...] ) ] { IN
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><replaceable class="parameter">new_tablespace</replaceable></term>
+ <listitem>
+ <para>
+ The tablespace where indexes will be rebuilt.
+ </para>
+ </listitem>
+ </varlistentry>
</variablelist>
</refsect1>
@@ -292,7 +315,12 @@ REINDEX [ ( <replaceable class="parameter">option</replaceable> [, ...] ) ] { IN
with <command>REINDEX INDEX</command> or <command>REINDEX TABLE</command>,
respectively. Each partition of the specified partitioned relation is
reindexed in a separate transaction. Those commands cannot be used inside
- a transaction block when working on a partitioned table or index.
+ a transaction block when working on a partitioned table or index. If
+ a <command>REINDEX</command> command fails when run on a partitioned
+ relation, and <literal>TABLESPACE</literal> was specified, then it may have
+ moved indexes on some partitions to the new tablespace. Re-running the command
+ will reindex all partitions and move previously-unprocessed indexes to the new
+ tablespace.
</para>
<refsect2 id="sql-reindex-concurrently" xreflabel="Rebuilding Indexes Concurrently">
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index b8cd35e995..363e69a5bc 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -57,6 +57,7 @@
#include "commands/event_trigger.h"
#include "commands/progress.h"
#include "commands/tablecmds.h"
+#include "commands/tablespace.h"
#include "commands/trigger.h"
#include "executor/executor.h"
#include "miscadmin.h"
@@ -71,6 +72,7 @@
#include "storage/predicate.h"
#include "storage/procarray.h"
#include "storage/smgr.h"
+#include "utils/acl.h"
#include "utils/builtins.h"
#include "utils/fmgroids.h"
#include "utils/guc.h"
@@ -1394,9 +1396,13 @@ index_update_collation_versions(Oid relid, Oid coll)
* Create concurrently an index based on the definition of the one provided by
* caller. The index is inserted into catalogs and needs to be built later
* on. This is called during concurrent reindex processing.
+ *
+ * "tablespaceOid" is the new tablespace to use for this index.
+ * If InvalidOid, use the current tablespace.
*/
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,
@@ -1526,7 +1532,8 @@ index_concurrently_create_copy(Relation heapRelation, Oid oldIndexId, const char
newInfo,
indexColNames,
indexRelation->rd_rel->relam,
- indexRelation->rd_rel->reltablespace,
+ OidIsValid(tablespaceOid) ?
+ tablespaceOid : indexRelation->rd_rel->reltablespace,
indexRelation->rd_indcollation,
indclass->values,
indcoloptions->values,
@@ -3591,6 +3598,8 @@ IndexGetRelation(Oid indexId, bool missing_ok)
/*
* reindex_index - This routine is used to recreate a single index
+ *
+ * See comments of reindex_relation() for details about "tablespaceOid".
*/
void
reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
@@ -3603,9 +3612,22 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
volatile bool skipped_constraint = false;
PGRUsage ru0;
bool progress = ((params->options & REINDEXOPT_REPORT_PROGRESS) != 0);
+ bool set_tablespace = OidIsValid(params->tablespaceOid);
pg_rusage_init(&ru0);
+ /* Check permissions except when using database's default */
+ if (set_tablespace && params->tablespaceOid != MyDatabaseTableSpace)
+ {
+ AclResult aclresult;
+
+ aclresult = pg_tablespace_aclcheck(params->tablespaceOid,
+ GetUserId(), ACL_CREATE);
+ if (aclresult != ACLCHECK_OK)
+ aclcheck_error(aclresult, OBJECT_TABLESPACE,
+ get_tablespace_name(params->tablespaceOid));
+ }
+
/*
* Open and lock the parent heap relation. ShareLock is sufficient since
* we only need to be sure no schema or data changes are going on.
@@ -3654,6 +3676,35 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
get_namespace_name(RelationGetNamespace(iRel)),
RelationGetRelationName(iRel));
+ /*
+ * We don't support moving system relations into different tablespaces
+ * unless allow_system_table_mods=1.
+ */
+ if (set_tablespace &&
+ !allowSystemTableMods && IsSystemRelation(iRel))
+ ereport(ERROR,
+ (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ errmsg("permission denied: \"%s\" is a system catalog",
+ RelationGetRelationName(iRel))));
+
+ /*
+ * We cannot support moving mapped relations into different tablespaces.
+ * (In particular this eliminates all shared catalogs.)
+ */
+ if (set_tablespace && RelationIsMapped(iRel))
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("cannot change tablespace of mapped relation \"%s\"",
+ RelationGetRelationName(iRel))));
+
+ /* It's not a shared catalog, so refuse to move it to shared tablespace */
+ if (params->tablespaceOid == GLOBALTABLESPACE_OID)
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("cannot move non-shared relation to tablespace \"%s\"",
+ get_tablespace_name(params->tablespaceOid))));
+
+
/*
* Don't allow reindex on temp tables of other backends ... their local
* buffer manager is not going to cope.
@@ -3680,6 +3731,25 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
*/
CheckTableNotInUse(iRel, "REINDEX INDEX");
+ /*
+ * Set the new tablespace for the relation. Do that only in the
+ * case where the reindex caller wishes to enforce a new tablespace.
+ */
+ if (set_tablespace && SetRelTableSpace(indexId, params->tablespaceOid))
+ {
+ /*
+ * Mark the relation as ready to be dropped at transaction commit,
+ * before making visible the new tablespace change so as this won't
+ * miss things.
+ */
+ RelationDropStorage(iRel);
+
+ RelationAssumeNewRelfilenode(iRel);
+
+ /* Make sure the reltablespace change is visible */
+ CommandCounterIncrement();
+ }
+
/*
* All predicate locks on the index are about to be made invalid. Promote
* them to relation locks on the heap.
@@ -3814,6 +3884,9 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
* reindex_relation - This routine is used to recreate all indexes
* of a relation (and optionally its toast relation too, if any).
*
+ * "tablespaceOid" is the tablespace where the relation's indexes will be
+ * rebuilt, or InvalidOid to keep each index on its current tablespace.
+ *
* "flags" is a bitmask that can include any combination of these bits:
*
* REINDEX_REL_PROCESS_TOAST: if true, process the toast table too (if any).
@@ -3964,10 +4037,14 @@ reindex_relation(Oid relid, int flags, ReindexParams *params)
/*
* Note that this should fail if the toast relation is missing, so
* reset REINDEXOPT_MISSING_OK.
+ *
+ * Even if table's indexes were moved to a new tablespace, the index
+ * on its toast table is not moved implicitly.
*/
ReindexParams newparams = *params;
newparams.options &= ~(REINDEXOPT_MISSING_OK);
+ newparams.tablespaceOid = InvalidOid;
result |= reindex_relation(toast_relid, flags, &newparams);
}
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index f9f3ff3b62..f2bd5d3d5b 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -2474,6 +2474,7 @@ ExecReindex(ParseState *pstate, ReindexStmt *stmt, bool isTopLevel)
ListCell *lc;
bool concurrently = false;
bool verbose = false;
+ char *tablespace = NULL;
/* Parse option list */
foreach(lc, stmt->params)
@@ -2484,6 +2485,8 @@ ExecReindex(ParseState *pstate, ReindexStmt *stmt, bool isTopLevel)
verbose = defGetBoolean(opt);
else if (strcmp(opt->defname, "concurrently") == 0)
concurrently = defGetBoolean(opt);
+ else if (strcmp(opt->defname, "tablespace") == 0)
+ tablespace = defGetString(opt);
else
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
@@ -2500,6 +2503,9 @@ ExecReindex(ParseState *pstate, ReindexStmt *stmt, bool isTopLevel)
(verbose ? REINDEXOPT_VERBOSE : 0) |
(concurrently ? REINDEXOPT_CONCURRENTLY : 0);
+ params.tablespaceOid = tablespace != NULL ?
+ get_tablespace_oid(tablespace, false) : InvalidOid;
+
switch (stmt->kind)
{
case REINDEX_OBJECT_INDEX:
@@ -2729,7 +2735,10 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
MemoryContext old;
List *relids = NIL;
int num_keys;
+
bool concurrent_warning = false;
+ bool tablespace_warning = false;
+ bool mapped_warning = false;
AssertArg(objectName);
Assert(objectKind == REINDEX_OBJECT_SCHEMA ||
@@ -2856,6 +2865,35 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
continue;
}
+ if (OidIsValid(params->tablespaceOid) &&
+ IsSystemClass(relid, classtuple))
+ {
+ if (!allowSystemTableMods)
+ {
+ /* Skip all system relations, if not allowSystemTableMods */
+ if (!tablespace_warning)
+ ereport(WARNING,
+ (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ errmsg("cannot change tablespace of indexes on system relations, skipping all")));
+ tablespace_warning = true;
+ continue;
+ }
+ else if (!OidIsValid(classtuple->relfilenode))
+ {
+ /*
+ * Skip all mapped relations if TABLESPACE is specified.
+ * OidIsValid(relfilenode) checks that, similar to
+ * RelationIsMapped().
+ */
+ if (!mapped_warning)
+ ereport(WARNING,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("cannot change tablespace of indexes on mapped relations, skipping all")));
+ mapped_warning = true;
+ continue;
+ }
+ }
+
/* Save the list of relation OIDs in private context */
old = MemoryContextSwitchTo(private_context);
@@ -2926,6 +2964,19 @@ ReindexPartitions(Oid relid, ReindexParams *params, bool isTopLevel)
Assert(relkind == RELKIND_PARTITIONED_INDEX ||
relkind == RELKIND_PARTITIONED_TABLE);
+ /* Check permissions except when using database's default */
+ if (OidIsValid(params->tablespaceOid)
+ && params->tablespaceOid != MyDatabaseTableSpace)
+ {
+ AclResult aclresult;
+
+ aclresult = pg_tablespace_aclcheck(params->tablespaceOid,
+ GetUserId(), ACL_CREATE);
+ if (aclresult != ACLCHECK_OK)
+ aclcheck_error(aclresult, OBJECT_TABLESPACE,
+ get_tablespace_name(params->tablespaceOid));
+ }
+
/*
* Check if this runs in a transaction block, with an error callback to
* provide more context under which a problem happens.
@@ -2969,9 +3020,27 @@ ReindexPartitions(Oid relid, ReindexParams *params, bool isTopLevel)
MemoryContext old_context;
/*
- * This discards partitioned tables, partitioned indexes and foreign
- * tables.
+ * Foreign tables and partitioned relations are not themselves
+ * reindexed - leaf partitions are processed directly. But any
+ * tablespace change is recorded in the catalog for partitioned
+ * relations.
*/
+ if (partkind == RELKIND_PARTITIONED_INDEX)
+ (void) SetRelTableSpace(partoid, params->tablespaceOid);
+ else if (partkind == RELKIND_PARTITIONED_TABLE)
+ {
+ Relation rel = table_open(partoid, ShareLock);
+ List *indexIds = RelationGetIndexList(rel);
+ ListCell *lc;
+
+ table_close(rel, NoLock);
+ foreach (lc, indexIds)
+ {
+ Oid indexid = lfirst_oid(lc);
+ (void) SetRelTableSpace(indexid, params->tablespaceOid);
+ }
+ }
+
if (!RELKIND_HAS_STORAGE(partkind))
continue;
@@ -3390,6 +3459,13 @@ ReindexRelationConcurrently(Oid relationOid, ReindexParams *params)
return false;
}
+ /* It's not a shared catalog, so refuse to move it to shared tablespace */
+ if (params->tablespaceOid == GLOBALTABLESPACE_OID)
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("cannot move non-shared relation to tablespace \"%s\"",
+ get_tablespace_name(params->tablespaceOid))));
+
Assert(heapRelationIds != NIL);
/*-----
@@ -3461,6 +3537,7 @@ ReindexRelationConcurrently(Oid relationOid, ReindexParams *params)
/* Create new index definition based on given index */
newIndexId = index_concurrently_create_copy(heapRel,
idx->indexId,
+ params->tablespaceOid,
concurrentName);
/*
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 17f7265038..a75647b1cc 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -3641,7 +3641,9 @@ psql_completion(const char *text, int start, int end)
* one word, so the above test is correct.
*/
if (ends_with(prev_wd, '(') || ends_with(prev_wd, ','))
- COMPLETE_WITH("CONCURRENTLY", "VERBOSE");
+ COMPLETE_WITH("CONCURRENTLY", "TABLESPACE", "VERBOSE");
+ else if (TailMatches("TABLESPACE"))
+ COMPLETE_WITH_QUERY(Query_for_list_of_tablespaces);
}
/* SECURITY LABEL */
diff --git a/src/include/catalog/index.h b/src/include/catalog/index.h
index 266f8950dc..56e403ac61 100644
--- a/src/include/catalog/index.h
+++ b/src/include/catalog/index.h
@@ -33,6 +33,7 @@ typedef enum
typedef struct ReindexParams
{
bits32 options; /* bitmask of REINDEXOPT_* */
+ Oid tablespaceOid; /* tablespace to rebuild index */
} ReindexParams;
/* flag bits for ReindexParams->flags */
@@ -92,6 +93,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,
diff --git a/src/test/regress/input/tablespace.source b/src/test/regress/input/tablespace.source
index 1a181016d7..a211c51e70 100644
--- a/src/test/regress/input/tablespace.source
+++ b/src/test/regress/input/tablespace.source
@@ -17,6 +17,80 @@ 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 (with TOAST) to test REINDEX with TABLESPACE change
+CREATE TABLE regress_tblspace_test_tbl (num1 bigint, num2 double precision, t text);
+INSERT INTO regress_tblspace_test_tbl (num1, num2, t)
+ SELECT round(random()*100), random(), repeat('text', 1000000)
+ FROM generate_series(1, 10) s(i);
+CREATE INDEX regress_tblspace_test_tbl_idx ON regress_tblspace_test_tbl (num1);
+CREATE TABLE regress_tblspace_test_pt(col1 int, col2 int) PARTITION BY HASH (col1);
+CREATE TABLE regress_tblspace_test_pt_1
+ PARTITION OF regress_tblspace_test_pt
+ FOR VALUES WITH (modulus 2, remainder 0)
+ PARTITION BY HASH (col2);
+CREATE INDEX regress_tblspace_test_pt_idx_1 ON regress_tblspace_test_pt (col1);
+CREATE INDEX regress_tblspace_test_pt_idx_2 ON regress_tblspace_test_pt (col2);
+
+-- first, check a no-op case
+REINDEX (TABLESPACE pg_default) INDEX regress_tblspace_test_tbl_idx;
+REINDEX (TABLESPACE pg_default) TABLE regress_tblspace_test_tbl;
+REINDEX (TABLESPACE pg_default) INDEX regress_tblspace_test_pt_idx_2;
+REINDEX (TABLESPACE pg_default) TABLE regress_tblspace_test_pt;
+
+-- check that REINDEX with TABLESPACE change is transactional
+BEGIN;
+REINDEX (TABLESPACE regress_tblspace) INDEX regress_tblspace_test_tbl_idx;
+REINDEX (TABLESPACE regress_tblspace) TABLE regress_tblspace_test_tbl;
+ROLLBACK;
+SELECT relname FROM pg_class
+WHERE reltablespace=(SELECT oid FROM pg_tablespace WHERE spcname='regress_tblspace');
+
+-- reindex and move the entire database, after that return everything back
+REINDEX (TABLESPACE regress_tblspace) DATABASE regression; -- ok with warning
+REINDEX (TABLESPACE pg_default) DATABASE regression; -- ok with warning
+SELECT relname FROM pg_class
+WHERE reltablespace=(SELECT oid FROM pg_tablespace WHERE spcname='regress_tblspace');
+
+-- partitioned indexes are not directly reindexed, but we still have to set proper
+-- tablespace in pg_class for them
+REINDEX (TABLESPACE regress_tblspace) TABLE regress_tblspace_test_pt; -- ok
+REINDEX (TABLESPACE regress_tblspace) INDEX regress_tblspace_test_pt_idx_1; -- ok
+REINDEX (TABLESPACE regress_tblspace) INDEX regress_tblspace_test_pt_idx_2; -- ok
+SELECT relname FROM pg_class
+WHERE reltablespace=(SELECT oid FROM pg_tablespace WHERE spcname='regress_tblspace');
+
+-- indexes for new partitions should be created in the new tablespace
+CREATE TABLE regress_tblspace_test_pt_2
+ PARTITION OF regress_tblspace_test_pt
+ FOR VALUES WITH (modulus 2, remainder 1);
+CREATE TABLE regress_tblspace_test_pt_1_1
+ PARTITION OF regress_tblspace_test_pt_1
+ FOR VALUES WITH (modulus 2, remainder 0);
+SELECT relname FROM pg_class
+WHERE reltablespace=(SELECT oid FROM pg_tablespace WHERE spcname='regress_tblspace');
+
+-- check REINDEX with TABLESPACE change
+REINDEX (TABLESPACE regress_tblspace) INDEX regress_tblspace_test_tbl_idx; -- ok
+REINDEX (TABLESPACE regress_tblspace) TABLE regress_tblspace_test_tbl; -- ok
+REINDEX (TABLESPACE regress_tblspace) TABLE pg_authid; -- fail
+REINDEX (TABLESPACE regress_tblspace) SYSTEM CONCURRENTLY postgres; -- fail
+REINDEX (TABLESPACE regress_tblspace) TABLE CONCURRENTLY pg_am; -- fail
+REINDEX (TABLESPACE pg_global) INDEX regress_tblspace_test_tbl_idx; -- fail
+REINDEX (TABLESPACE regress_tblspace) TABLE pg_am; -- fail
+
+-- check that all relations moved to new tablespace
+SELECT relname FROM pg_class
+WHERE reltablespace=(SELECT oid FROM pg_tablespace WHERE spcname='regress_tblspace')
+ORDER BY relname;
+
+-- move indexes back to pg_default tablespace
+REINDEX (TABLESPACE pg_default) TABLE CONCURRENTLY regress_tblspace_test_tbl; -- ok
+REINDEX (TABLESPACE pg_default) TABLE regress_tblspace_test_pt; -- 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;
@@ -96,6 +170,14 @@ SELECT relname, spcname FROM pg_catalog.pg_tablespace t, pg_catalog.pg_class c
\d testschema.part_a_idx
\d+ testschema.part_a_idx
+-- REINDEX partitioned indexes to new tablespace
+REINDEX (TABLESPACE pg_default) TABLE testschema.part;
+\d testschema.part
+\d testschema.part1
+REINDEX (CONCURRENTLY, TABLESPACE regress_tblspace) INDEX testschema.part_a_idx;
+\d testschema.part
+\d testschema.part1
+
-- partitioned rels cannot specify the default tablespace. These fail:
CREATE TABLE testschema.dflt (a int PRIMARY KEY) PARTITION BY LIST (a) TABLESPACE pg_default;
CREATE TABLE testschema.dflt (a int PRIMARY KEY USING INDEX TABLESPACE pg_default) PARTITION BY LIST (a);
@@ -282,6 +364,9 @@ ALTER TABLE ALL IN TABLESPACE regress_tblspace_renamed SET TABLESPACE pg_default
-- Should succeed
DROP TABLESPACE regress_tblspace_renamed;
+DROP TABLE regress_tblspace_test_tbl;
+DROP TABLE regress_tblspace_test_pt;
+
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 94c5f023c6..436770cc32 100644
--- a/src/test/regress/output/tablespace.source
+++ b/src/test/regress/output/tablespace.source
@@ -20,6 +20,122 @@ 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 (with TOAST) to test REINDEX with TABLESPACE change
+CREATE TABLE regress_tblspace_test_tbl (num1 bigint, num2 double precision, t text);
+INSERT INTO regress_tblspace_test_tbl (num1, num2, t)
+ SELECT round(random()*100), random(), repeat('text', 1000000)
+ FROM generate_series(1, 10) s(i);
+CREATE INDEX regress_tblspace_test_tbl_idx ON regress_tblspace_test_tbl (num1);
+CREATE TABLE regress_tblspace_test_pt(col1 int, col2 int) PARTITION BY HASH (col1);
+CREATE TABLE regress_tblspace_test_pt_1
+ PARTITION OF regress_tblspace_test_pt
+ FOR VALUES WITH (modulus 2, remainder 0)
+ PARTITION BY HASH (col2);
+CREATE INDEX regress_tblspace_test_pt_idx_1 ON regress_tblspace_test_pt (col1);
+CREATE INDEX regress_tblspace_test_pt_idx_2 ON regress_tblspace_test_pt (col2);
+-- first, check a no-op case
+REINDEX (TABLESPACE pg_default) INDEX regress_tblspace_test_tbl_idx;
+REINDEX (TABLESPACE pg_default) TABLE regress_tblspace_test_tbl;
+REINDEX (TABLESPACE pg_default) INDEX regress_tblspace_test_pt_idx_2;
+REINDEX (TABLESPACE pg_default) TABLE regress_tblspace_test_pt;
+-- check that REINDEX with TABLESPACE change is transactional
+BEGIN;
+REINDEX (TABLESPACE regress_tblspace) INDEX regress_tblspace_test_tbl_idx;
+REINDEX (TABLESPACE regress_tblspace) TABLE regress_tblspace_test_tbl;
+ROLLBACK;
+SELECT relname FROM pg_class
+WHERE reltablespace=(SELECT oid FROM pg_tablespace WHERE spcname='regress_tblspace');
+ relname
+---------
+(0 rows)
+
+-- reindex and move the entire database, after that return everything back
+REINDEX (TABLESPACE regress_tblspace) DATABASE regression; -- ok with warning
+WARNING: cannot change tablespace of indexes on system relations, skipping all
+REINDEX (TABLESPACE pg_default) DATABASE regression; -- ok with warning
+WARNING: cannot change tablespace of indexes on system relations, skipping all
+SELECT relname FROM pg_class
+WHERE reltablespace=(SELECT oid FROM pg_tablespace WHERE spcname='regress_tblspace');
+ relname
+---------
+(0 rows)
+
+-- partitioned indexes are not directly reindexed, but we still have to set proper
+-- tablespace in pg_class for them
+REINDEX (TABLESPACE regress_tblspace) TABLE regress_tblspace_test_pt; -- ok
+REINDEX (TABLESPACE regress_tblspace) INDEX regress_tblspace_test_pt_idx_1; -- ok
+REINDEX (TABLESPACE regress_tblspace) INDEX regress_tblspace_test_pt_idx_2; -- ok
+SELECT relname FROM pg_class
+WHERE reltablespace=(SELECT oid FROM pg_tablespace WHERE spcname='regress_tblspace');
+ relname
+-------------------------------------
+ regress_tblspace_test_pt_idx_1
+ regress_tblspace_test_pt_idx_2
+ regress_tblspace_test_pt_1_col1_idx
+ regress_tblspace_test_pt_1_col2_idx
+(4 rows)
+
+-- indexes for new partitions should be created in the new tablespace
+CREATE TABLE regress_tblspace_test_pt_2
+ PARTITION OF regress_tblspace_test_pt
+ FOR VALUES WITH (modulus 2, remainder 1);
+CREATE TABLE regress_tblspace_test_pt_1_1
+ PARTITION OF regress_tblspace_test_pt_1
+ FOR VALUES WITH (modulus 2, remainder 0);
+SELECT relname FROM pg_class
+WHERE reltablespace=(SELECT oid FROM pg_tablespace WHERE spcname='regress_tblspace');
+ relname
+---------------------------------------
+ regress_tblspace_test_pt_idx_1
+ regress_tblspace_test_pt_idx_2
+ regress_tblspace_test_pt_2_col1_idx
+ regress_tblspace_test_pt_2_col2_idx
+ regress_tblspace_test_pt_1_1_col1_idx
+ regress_tblspace_test_pt_1_col1_idx
+ regress_tblspace_test_pt_1_1_col2_idx
+ regress_tblspace_test_pt_1_col2_idx
+(8 rows)
+
+-- check REINDEX with TABLESPACE change
+REINDEX (TABLESPACE regress_tblspace) INDEX regress_tblspace_test_tbl_idx; -- ok
+REINDEX (TABLESPACE regress_tblspace) TABLE regress_tblspace_test_tbl; -- ok
+REINDEX (TABLESPACE regress_tblspace) TABLE pg_authid; -- fail
+ERROR: permission denied: "pg_authid_rolname_index" is a system catalog
+REINDEX (TABLESPACE regress_tblspace) SYSTEM CONCURRENTLY postgres; -- fail
+ERROR: cannot reindex system catalogs concurrently
+REINDEX (TABLESPACE regress_tblspace) TABLE CONCURRENTLY pg_am; -- fail
+ERROR: cannot reindex system catalogs concurrently
+REINDEX (TABLESPACE pg_global) INDEX regress_tblspace_test_tbl_idx; -- fail
+ERROR: cannot move non-shared relation to tablespace "pg_global"
+REINDEX (TABLESPACE regress_tblspace) TABLE pg_am; -- fail
+ERROR: permission denied: "pg_am_name_index" is a system catalog
+-- check that all relations moved to new tablespace
+SELECT relname FROM pg_class
+WHERE reltablespace=(SELECT oid FROM pg_tablespace WHERE spcname='regress_tblspace')
+ORDER BY relname;
+ relname
+---------------------------------------
+ regress_tblspace_test_pt_1_1_col1_idx
+ regress_tblspace_test_pt_1_1_col2_idx
+ regress_tblspace_test_pt_1_col1_idx
+ regress_tblspace_test_pt_1_col2_idx
+ regress_tblspace_test_pt_2_col1_idx
+ regress_tblspace_test_pt_2_col2_idx
+ regress_tblspace_test_pt_idx_1
+ regress_tblspace_test_pt_idx_2
+ regress_tblspace_test_tbl_idx
+(9 rows)
+
+-- move indexes back to pg_default tablespace
+REINDEX (TABLESPACE pg_default) TABLE CONCURRENTLY regress_tblspace_test_tbl; -- ok
+REINDEX (TABLESPACE pg_default) TABLE regress_tblspace_test_pt; -- 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
@@ -199,6 +315,47 @@ Partitions: testschema.part1_a_idx,
testschema.part2_a_idx
Tablespace: "regress_tblspace"
+-- REINDEX partitioned indexes to new tablespace
+REINDEX (TABLESPACE pg_default) TABLE testschema.part;
+\d testschema.part
+ Partitioned table "testschema.part"
+ Column | Type | Collation | Nullable | Default
+--------+---------+-----------+----------+---------
+ a | integer | | |
+Partition key: LIST (a)
+Indexes:
+ "part_a_idx" btree (a)
+Number of partitions: 2 (Use \d+ to list them.)
+
+\d testschema.part1
+ Table "testschema.part1"
+ Column | Type | Collation | Nullable | Default
+--------+---------+-----------+----------+---------
+ a | integer | | |
+Partition of: testschema.part FOR VALUES IN (1)
+Indexes:
+ "part1_a_idx" btree (a)
+
+REINDEX (CONCURRENTLY, TABLESPACE regress_tblspace) INDEX testschema.part_a_idx;
+\d testschema.part
+ Partitioned table "testschema.part"
+ Column | Type | Collation | Nullable | Default
+--------+---------+-----------+----------+---------
+ a | integer | | |
+Partition key: LIST (a)
+Indexes:
+ "part_a_idx" btree (a), tablespace "regress_tblspace"
+Number of partitions: 2 (Use \d+ to list them.)
+
+\d testschema.part1
+ Table "testschema.part1"
+ Column | Type | Collation | Nullable | Default
+--------+---------+-----------+----------+---------
+ a | integer | | |
+Partition of: testschema.part FOR VALUES IN (1)
+Indexes:
+ "part1_a_idx" btree (a), tablespace "regress_tblspace"
+
-- partitioned rels cannot specify the default tablespace. These fail:
CREATE TABLE testschema.dflt (a int PRIMARY KEY) PARTITION BY LIST (a) TABLESPACE pg_default;
ERROR: cannot specify default tablespace for partitioned relations
@@ -741,6 +898,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 TABLE regress_tblspace_test_tbl;
+DROP TABLE regress_tblspace_test_pt;
DROP SCHEMA testschema CASCADE;
NOTICE: drop cascades to 6 other objects
DETAIL: drop cascades to table testschema.foo
--
2.20.1
From db99f3daeb50b063145f257bb9ffa2aae2c931d4 Mon Sep 17 00:00:00 2001
From: Alexey Kondratov <kondratov.alek...@gmail.com>
Date: Wed, 20 Jan 2021 20:21:12 +0300
Subject: [PATCH v4 1/2] Extract common part from ATExecSetTableSpaceNoStorage
for a future usage
---
src/backend/commands/tablecmds.c | 96 +++++++++++++++++++-------------
src/include/commands/tablecmds.h | 2 +
2 files changed, 58 insertions(+), 40 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 8687e9a97c..af386b3ad0 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -13291,6 +13291,58 @@ ATExecSetTableSpace(Oid tableOid, Oid newTableSpace, LOCKMODE lockmode)
list_free(reltoastidxids);
}
+/*
+ * SetRelTableSpace - modify relation tablespace in the pg_class entry.
+ *
+ * 'reloid' is an Oid of relation to be modified.
+ * 'tablespaceOid' is an Oid of new tablespace.
+ *
+ * Catalog modification is done only if tablespaceOid is different from
+ * the currently set. Returned bool value is indicating whether any changes
+ * were made or not.
+ */
+bool
+SetRelTableSpace(Oid reloid, Oid tablespaceOid)
+{
+ Relation pg_class;
+ HeapTuple tuple;
+ Form_pg_class rd_rel;
+ bool changed = false;
+
+ /* Get a modifiable copy of the relation's pg_class row. */
+ pg_class = table_open(RelationRelationId, RowExclusiveLock);
+
+ tuple = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(reloid));
+ if (!HeapTupleIsValid(tuple))
+ elog(ERROR, "cache lookup failed for relation %u", reloid);
+ rd_rel = (Form_pg_class) GETSTRUCT(tuple);
+
+ /* MyDatabaseTableSpace is stored as InvalidOid. */
+ if (tablespaceOid == MyDatabaseTableSpace)
+ tablespaceOid = InvalidOid;
+
+ /* No work if no change in tablespace. */
+ if (tablespaceOid != rd_rel->reltablespace)
+ {
+ /* Update the pg_class row. */
+ rd_rel->reltablespace = tablespaceOid;
+ CatalogTupleUpdate(pg_class, &tuple->t_self, tuple);
+
+ /* Record dependency on tablespace. */
+ changeDependencyOnTablespace(RelationRelationId,
+ reloid, rd_rel->reltablespace);
+
+ changed = true;
+ }
+
+ InvokeObjectPostAlterHook(RelationRelationId, reloid, 0);
+
+ heap_freetuple(tuple);
+ table_close(pg_class, RowExclusiveLock);
+
+ return changed;
+}
+
/*
* Special handling of ALTER TABLE SET TABLESPACE for relations with no
* storage that have an interest in preserving tablespace.
@@ -13301,11 +13353,7 @@ ATExecSetTableSpace(Oid tableOid, Oid newTableSpace, LOCKMODE lockmode)
static void
ATExecSetTableSpaceNoStorage(Relation rel, Oid newTableSpace)
{
- HeapTuple tuple;
- Oid oldTableSpace;
- Relation pg_class;
- Form_pg_class rd_rel;
- Oid reloid = RelationGetRelid(rel);
+ Oid reloid = RelationGetRelid(rel);
/*
* Shouldn't be called on relations having storage; these are processed in
@@ -13319,41 +13367,9 @@ ATExecSetTableSpaceNoStorage(Relation rel, Oid newTableSpace)
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("only shared relations can be placed in pg_global tablespace")));
- /*
- * No work if no change in tablespace.
- */
- oldTableSpace = rel->rd_rel->reltablespace;
- if (newTableSpace == oldTableSpace ||
- (newTableSpace == MyDatabaseTableSpace && oldTableSpace == 0))
- {
- InvokeObjectPostAlterHook(RelationRelationId, reloid, 0);
- return;
- }
-
- /* Get a modifiable copy of the relation's pg_class row */
- pg_class = table_open(RelationRelationId, RowExclusiveLock);
-
- tuple = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(reloid));
- if (!HeapTupleIsValid(tuple))
- elog(ERROR, "cache lookup failed for relation %u", reloid);
- rd_rel = (Form_pg_class) GETSTRUCT(tuple);
-
- /* update the pg_class row */
- rd_rel->reltablespace = (newTableSpace == MyDatabaseTableSpace) ? InvalidOid : newTableSpace;
- CatalogTupleUpdate(pg_class, &tuple->t_self, tuple);
-
- /* Record dependency on tablespace */
- changeDependencyOnTablespace(RelationRelationId,
- reloid, rd_rel->reltablespace);
-
- InvokeObjectPostAlterHook(RelationRelationId, reloid, 0);
-
- heap_freetuple(tuple);
-
- table_close(pg_class, RowExclusiveLock);
-
- /* Make sure the reltablespace change is visible */
- CommandCounterIncrement();
+ if (SetRelTableSpace(reloid, newTableSpace))
+ /* Make sure the reltablespace change is visible */
+ CommandCounterIncrement();
}
/*
diff --git a/src/include/commands/tablecmds.h b/src/include/commands/tablecmds.h
index 08c463d3c4..8e63518485 100644
--- a/src/include/commands/tablecmds.h
+++ b/src/include/commands/tablecmds.h
@@ -61,6 +61,8 @@ extern void ExecuteTruncateGuts(List *explicit_rels, List *relids, List *relids_
extern void SetRelationHasSubclass(Oid relationId, bool relhassubclass);
+extern bool SetRelTableSpace(Oid reloid, Oid tablespaceOid);
+
extern ObjectAddress renameatt(RenameStmt *stmt);
extern ObjectAddress RenameConstraint(RenameStmt *stmt);
base-commit: 920f853dc948b98a5dc96580c4ee011a302e33e4
--
2.20.1
