Hello
A customer reported to us that partitioned indexes are not working
consistently with tablespaces:
1. When a CREATE INDEX specifies a tablespace, existing partitions get
the index in the correct tablespace; however, the parent index itself
does not record the tablespace. So when new partitions are created
later, they get the index in the default tablespace instead of the
specified tablespace. Fix by saving the tablespace in the pg_class row
for the parent index.
2. ALTER TABLE SET TABLESPACE, applied to the partitioned index, would
raise an error indicating that it's not the correct relation kind. In
order for this to actually work, we need bespoke code for ATExecCmd();
the code for all other relation kinds wants to move storage (and runs in
Phase 3, later), but these indexes do not have that. Therefore, write a
cut-down version which is invoked directly in ATExecCmd instead.
3. ALTER INDEX ALL IN TABLESPACE, identical problem, is also fixed by
the above change.
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>From 819dd64baf04d39f02785cde72b6a4c33fd10f3b Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvhe...@alvh.no-ip.org>
Date: Thu, 1 Nov 2018 21:29:56 -0300
Subject: [PATCH] Fix tablespace handling for partitioned indexes
When creating partitioned indexes, the tablespace was not being saved
for the parent index. This meant that subsequently created partitions
would not use the right tablespace for their indexes.
ALTER INDEX SET TABLESPACE and ALTER INDEX ALL IN TABLESPACE also raised
errors when tried; fix them too.
---
src/backend/catalog/heap.c | 10 ++++-
src/backend/commands/tablecmds.c | 61 +++++++++++++++++++++++++++++--
src/test/regress/input/tablespace.source | 10 +++++
src/test/regress/output/tablespace.source | 19 +++++++++-
4 files changed, 95 insertions(+), 5 deletions(-)
diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index 8c52a1543d..61ce44830b 100644
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -297,7 +297,6 @@ heap_create(const char *relname,
case RELKIND_COMPOSITE_TYPE:
case RELKIND_FOREIGN_TABLE:
case RELKIND_PARTITIONED_TABLE:
- case RELKIND_PARTITIONED_INDEX:
create_storage = false;
/*
@@ -306,6 +305,15 @@ heap_create(const char *relname,
*/
reltablespace = InvalidOid;
break;
+
+ case RELKIND_PARTITIONED_INDEX:
+ /*
+ * Preserve tablespace so that it's used as tablespace for indexes
+ * on future partitions.
+ */
+ create_storage = false;
+ break;
+
case RELKIND_SEQUENCE:
create_storage = true;
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 357c73073d..514c1c9d49 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -446,6 +446,8 @@ static bool ATPrepChangePersistence(Relation rel, bool toLogged);
static void ATPrepSetTableSpace(AlteredTableInfo *tab, Relation rel,
const char *tablespacename, LOCKMODE lockmode);
static void ATExecSetTableSpace(Oid tableOid, Oid newTableSpace, LOCKMODE lockmode);
+static void ATExecPartedIdxSetTableSpace(Relation rel, Oid newTableSpace);
+
static void ATExecSetRelOptions(Relation rel, List *defList,
AlterTableType operation,
LOCKMODE lockmode);
@@ -3845,7 +3847,8 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
pass = AT_PASS_DROP;
break;
case AT_SetTableSpace: /* SET TABLESPACE */
- ATSimplePermissions(rel, ATT_TABLE | ATT_MATVIEW | ATT_INDEX);
+ ATSimplePermissions(rel, ATT_TABLE | ATT_MATVIEW | ATT_INDEX |
+ ATT_PARTITIONED_INDEX);
/* This command never recurses */
ATPrepSetTableSpace(tab, rel, cmd->name, lockmode);
pass = AT_PASS_MISC; /* doesn't actually matter */
@@ -4181,10 +4184,13 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
*/
break;
case AT_SetTableSpace: /* SET TABLESPACE */
-
/*
- * Nothing to do here; Phase 3 does the work
+ * Only do this for partitioned indexes, for which this is just
+ * a catalog change. Other relation types are handled by Phase 3.
*/
+ if (rel->rd_rel->relkind == RELKIND_PARTITIONED_INDEX)
+ ATExecPartedIdxSetTableSpace(rel, tab->newTableSpace);
+
break;
case AT_SetRelOptions: /* SET (...) */
case AT_ResetRelOptions: /* RESET (...) */
@@ -11050,6 +11056,55 @@ ATExecSetTableSpace(Oid tableOid, Oid newTableSpace, LOCKMODE lockmode)
}
/*
+ * Special handling of ALTER TABLE SET TABLESPACE for partitioned indexes,
+ * which have no storage (so not handled in Phase 3 like other relation types)
+ */
+static void
+ATExecPartedIdxSetTableSpace(Relation rel, Oid newTableSpace)
+{
+ HeapTuple tuple;
+ Oid oldTableSpace;
+ Relation pg_class;
+ Form_pg_class rd_rel;
+ Oid indexOid = RelationGetRelid(rel);
+
+ Assert(rel->rd_rel->relkind == RELKIND_PARTITIONED_INDEX);
+
+ /*
+ * No work if no change in tablespace.
+ */
+ oldTableSpace = rel->rd_rel->reltablespace;
+ if (newTableSpace == oldTableSpace ||
+ (newTableSpace == MyDatabaseTableSpace && oldTableSpace == 0))
+ {
+ InvokeObjectPostAlterHook(RelationRelationId,
+ indexOid, 0);
+ return;
+ }
+
+ /* Get a modifiable copy of the relation's pg_class row */
+ pg_class = heap_open(RelationRelationId, RowExclusiveLock);
+
+ tuple = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(indexOid));
+ if (!HeapTupleIsValid(tuple))
+ elog(ERROR, "cache lookup failed for relation %u", indexOid);
+ 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);
+
+ InvokeObjectPostAlterHook(RelationRelationId, indexOid, 0);
+
+ heap_freetuple(tuple);
+
+ heap_close(pg_class, RowExclusiveLock);
+
+ /* Make sure the reltablespace change is visible */
+ CommandCounterIncrement();
+}
+
+/*
* Alter Table ALL ... SET TABLESPACE
*
* Allows a user to move all objects of some type in a given tablespace in the
diff --git a/src/test/regress/input/tablespace.source b/src/test/regress/input/tablespace.source
index 7f7934b745..60c87261db 100644
--- a/src/test/regress/input/tablespace.source
+++ b/src/test/regress/input/tablespace.source
@@ -44,6 +44,14 @@ CREATE INDEX foo_idx on testschema.foo(i) TABLESPACE regress_tblspace;
SELECT relname, spcname FROM pg_catalog.pg_tablespace t, pg_catalog.pg_class c
where c.reltablespace = t.oid AND c.relname = 'foo_idx';
+-- partitioned index
+CREATE TABLE testschema.part (a int) PARTITION BY LIST (a);
+CREATE TABLE testschema.part1 PARTITION OF testschema.part FOR VALUES IN (1);
+CREATE INDEX part_a_idx ON testschema.part (a) TABLESPACE regress_tblspace;
+CREATE TABLE testschema.part2 PARTITION OF testschema.part FOR VALUES IN (2);
+SELECT relname, spcname FROM pg_catalog.pg_tablespace t, pg_catalog.pg_class c
+ where c.reltablespace = t.oid AND c.relname LIKE 'part%_idx';
+
-- check that default_tablespace doesn't affect ALTER TABLE index rebuilds
CREATE TABLE testschema.test_default_tab(id bigint) TABLESPACE regress_tblspace;
INSERT INTO testschema.test_default_tab VALUES (1);
@@ -93,6 +101,8 @@ CREATE UNIQUE INDEX anindex ON testschema.atable(column1);
ALTER TABLE testschema.atable SET TABLESPACE regress_tblspace;
ALTER INDEX testschema.anindex SET TABLESPACE regress_tblspace;
+ALTER INDEX testschema.part_a_idx SET TABLESPACE pg_default;
+ALTER INDEX testschema.part_a_idx SET TABLESPACE regress_tblspace;
INSERT INTO testschema.atable VALUES(3); -- ok
INSERT INTO testschema.atable VALUES(1); -- fail (checks index)
diff --git a/src/test/regress/output/tablespace.source b/src/test/regress/output/tablespace.source
index fe3614cd76..43962e6f01 100644
--- a/src/test/regress/output/tablespace.source
+++ b/src/test/regress/output/tablespace.source
@@ -61,6 +61,20 @@ SELECT relname, spcname FROM pg_catalog.pg_tablespace t, pg_catalog.pg_class c
foo_idx | regress_tblspace
(1 row)
+-- partitioned index
+CREATE TABLE testschema.part (a int) PARTITION BY LIST (a);
+CREATE TABLE testschema.part1 PARTITION OF testschema.part FOR VALUES IN (1);
+CREATE INDEX part_a_idx ON testschema.part (a) TABLESPACE regress_tblspace;
+CREATE TABLE testschema.part2 PARTITION OF testschema.part FOR VALUES IN (2);
+SELECT relname, spcname FROM pg_catalog.pg_tablespace t, pg_catalog.pg_class c
+ where c.reltablespace = t.oid AND c.relname LIKE 'part%_idx';
+ relname | spcname
+-------------+------------------
+ part1_a_idx | regress_tblspace
+ part2_a_idx | regress_tblspace
+ part_a_idx | regress_tblspace
+(3 rows)
+
-- check that default_tablespace doesn't affect ALTER TABLE index rebuilds
CREATE TABLE testschema.test_default_tab(id bigint) TABLESPACE regress_tblspace;
INSERT INTO testschema.test_default_tab VALUES (1);
@@ -200,6 +214,8 @@ CREATE TABLE testschema.atable AS VALUES (1), (2);
CREATE UNIQUE INDEX anindex ON testschema.atable(column1);
ALTER TABLE testschema.atable SET TABLESPACE regress_tblspace;
ALTER INDEX testschema.anindex SET TABLESPACE regress_tblspace;
+ALTER INDEX testschema.part_a_idx SET TABLESPACE pg_default;
+ALTER INDEX testschema.part_a_idx SET TABLESPACE regress_tblspace;
INSERT INTO testschema.atable VALUES(3); -- ok
INSERT INTO testschema.atable VALUES(1); -- fail (checks index)
ERROR: duplicate key value violates unique constraint "anindex"
@@ -241,10 +257,11 @@ NOTICE: no matching relations in tablespace "regress_tblspace_renamed" found
-- Should succeed
DROP TABLESPACE regress_tblspace_renamed;
DROP SCHEMA testschema CASCADE;
-NOTICE: drop cascades to 5 other objects
+NOTICE: drop cascades to 6 other objects
DETAIL: drop cascades to table testschema.foo
drop cascades to table testschema.asselect
drop cascades to table testschema.asexecute
+drop cascades to table testschema.part
drop cascades to table testschema.atable
drop cascades to table testschema.tablespace_acl
DROP ROLE regress_tablespace_user1;
--
2.11.0
