Subject: MERGE/SPLIT partition commands should create new partitions in the parent's tablespace?

From: Fujii Masao <[email-masked]>
Date: Sat, 6 Jul 2024 16:05:56 +0900

Hi,

I noticed that ALTER TABLE MERGE PARTITIONS and SPLIT PARTITION commands
always create new partitions in the default tablespace, regardless of
the parent's tablespace. However, the indexes of these new partitions inherit
the tablespaces of their parent indexes. This inconsistency seems odd.
Is this an oversight or intentional?

Here are the steps I used to test this:

-------------------------------------------------------
CREATE TABLESPACE tblspc LOCATION '/tmp/tblspc';
CREATE TABLE t (i int PRIMARY KEY USING INDEX TABLESPACE tblspc)
   PARTITION BY RANGE (i) TABLESPACE tblspc;

CREATE TABLE tp_0_1 PARTITION OF t FOR VALUES FROM (0) TO (1);
CREATE TABLE tp_1_2 PARTITION OF t FOR VALUES FROM (1) TO (2);

ALTER TABLE t MERGE PARTITIONS (tp_0_1, tp_1_2) INTO tp_0_2;

SELECT tablename, tablespace FROM pg_tables WHERE tablename IN ('t', 'tp_0_2') ORDER BY tablename;
  tablename | tablespace
-----------+------------
  t         | tblspc
  tp_0_2    | (null)
(2 rows)

SELECT indexname, tablespace FROM pg_indexes WHERE tablename IN ('t', 'tp_0_2') ORDER BY indexname;
   indexname  | tablespace
-------------+------------
  t_pkey      | tblspc
  tp_0_2_pkey | tblspc
-------------------------------------------------------

If it's an oversight, I've attached a patch to ensure these commands create
new partitions in the parent's tablespace.

Regards,

-- 
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION

From 1ce901d56bd6f0e7cb14f30570187b36f249fcd7 Mon Sep 17 00:00:00 2001
From: Fujii Masao <[email-masked]>
Date: Sat, 6 Jul 2024 14:18:54 +0900
Subject: [PATCH v1] Ensure MERGE/SPLIT partition commands create new
 partitions in the parent's tablespace.

Previously, ALTER TABLE MERGE PARTITIONS and ALTER TABLE SPLIT PARTITION
commands always created new partitions in the default tablespace,
ignoring the parent's tablespace. This commit fixes that, making sure
new partitions inherit the parent's tablespace.

Backpatch to v17 where those commands were added.
---
 doc/src/sgml/ref/alter_table.sgml             |  6 +++--
 src/backend/commands/tablecmds.c              |  2 +-
 src/test/regress/expected/partition_merge.out | 21 ++++++++++++++++
 src/test/regress/expected/partition_split.out | 24 +++++++++++++++++++
 src/test/regress/sql/partition_merge.sql      | 10 ++++++++
 src/test/regress/sql/partition_split.sql      | 11 +++++++++
 6 files changed, 71 insertions(+), 3 deletions(-)

diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml
index c062a36880..6a2822adad 100644
--- a/doc/src/sgml/ref/alter_table.sgml
+++ b/doc/src/sgml/ref/alter_table.sgml
@@ -1163,7 +1163,8 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
       New partitions will have the same table access method as the parent.
       If the parent table is persistent then new partitions are created
       persistent.  If the parent table is temporary then new partitions
-      are also created temporary.
+      are also created temporary.  New partitions will also be created in
+      the same tablespace as the parent.
      </para>
      <note>
       <para>
@@ -1235,7 +1236,8 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
       The new partition will have the same table access method as the parent.
       If the parent table is persistent then the new partition is created
       persistent.  If the parent table is temporary then the new partition
-      is also created temporary.
+      is also created temporary.  The new partition will also be created in
+      the same tablespace as the parent.
      </para>
      <note>
       <para>
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index dbfe0d6b1c..9e1eff19e0 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -20331,7 +20331,7 @@ createPartitionTable(RangeVar *newPartName, Relation modelRel,
 	createStmt->constraints = NIL;
 	createStmt->options = NIL;
 	createStmt->oncommit = ONCOMMIT_NOOP;
-	createStmt->tablespacename = NULL;
+	createStmt->tablespacename = get_tablespace_name(modelRel->rd_rel->reltablespace);
 	createStmt->if_not_exists = false;
 	createStmt->accessMethod = get_am_name(modelRel->rd_rel->relam);
 
diff --git a/src/test/regress/expected/partition_merge.out b/src/test/regress/expected/partition_merge.out
index 9c67a4a8b1..d19ab87b5a 100644
--- a/src/test/regress/expected/partition_merge.out
+++ b/src/test/regress/expected/partition_merge.out
@@ -861,6 +861,27 @@ SET search_path = partitions_merge_schema, pg_temp, public;
 -- Can't merge temporary partitions into a persistent partition
 ALTER TABLE t MERGE PARTITIONS (tp_0_1, tp_1_2) INTO tp_0_2;
 ROLLBACK;
+-- Check the new partition inherits parent's tablespace
+CREATE TABLE t (i int PRIMARY KEY USING INDEX TABLESPACE regress_tblspace)
+  PARTITION BY RANGE (i) TABLESPACE regress_tblspace;
+CREATE TABLE tp_0_1 PARTITION OF t FOR VALUES FROM (0) TO (1);
+CREATE TABLE tp_1_2 PARTITION OF t FOR VALUES FROM (1) TO (2);
+ALTER TABLE t MERGE PARTITIONS (tp_0_1, tp_1_2) INTO tp_0_2;
+SELECT tablespace FROM pg_tables WHERE tablename IN ('t', 'tp_0_2') ORDER BY tablespace;
+    tablespace    
+------------------
+ regress_tblspace
+ regress_tblspace
+(2 rows)
+
+SELECT tablespace FROM pg_indexes WHERE tablename IN ('t', 'tp_0_2') ORDER BY tablespace;
+    tablespace    
+------------------
+ regress_tblspace
+ regress_tblspace
+(2 rows)
+
+DROP TABLE t;
 -- Check the new partition inherits parent's table access method
 SET search_path = partitions_merge_schema, public;
 CREATE ACCESS METHOD partitions_merge_heap TYPE TABLE HANDLER heap_tableam_handler;
diff --git a/src/test/regress/expected/partition_split.out b/src/test/regress/expected/partition_split.out
index 5fbfc8f805..3f51a0b4f6 100644
--- a/src/test/regress/expected/partition_split.out
+++ b/src/test/regress/expected/partition_split.out
@@ -1493,6 +1493,30 @@ SELECT c.oid::pg_catalog.regclass, pg_catalog.pg_get_expr(c.relpartbound, c.oid)
  tp_1_2 | FOR VALUES FROM (1) TO (2) | t
 (2 rows)
 
+DROP TABLE t;
+-- Check new partitions inherits parent's tablespace
+CREATE TABLE t (i int PRIMARY KEY USING INDEX TABLESPACE regress_tblspace)
+  PARTITION BY RANGE (i) TABLESPACE regress_tblspace;
+CREATE TABLE tp_0_2 PARTITION OF t FOR VALUES FROM (0) TO (2);
+ALTER TABLE t SPLIT PARTITION tp_0_2 INTO
+  (PARTITION tp_0_1 FOR VALUES FROM (0) TO (1),
+   PARTITION tp_1_2 FOR VALUES FROM (1) TO (2));
+SELECT tablespace FROM pg_tables WHERE tablename IN ('t', 'tp_0_1', 'tp_1_2') ORDER BY tablespace;
+    tablespace    
+------------------
+ regress_tblspace
+ regress_tblspace
+ regress_tblspace
+(3 rows)
+
+SELECT tablespace FROM pg_indexes WHERE tablename IN ('t', 'tp_0_1', 'tp_1_2') ORDER BY tablespace;
+    tablespace    
+------------------
+ regress_tblspace
+ regress_tblspace
+ regress_tblspace
+(3 rows)
+
 DROP TABLE t;
 -- Check new partitions inherits parent's table access method
 CREATE ACCESS METHOD partition_split_heap TYPE TABLE HANDLER heap_tableam_handler;
diff --git a/src/test/regress/sql/partition_merge.sql b/src/test/regress/sql/partition_merge.sql
index 5624973200..6c3242ab15 100644
--- a/src/test/regress/sql/partition_merge.sql
+++ b/src/test/regress/sql/partition_merge.sql
@@ -536,6 +536,16 @@ SET search_path = partitions_merge_schema, pg_temp, public;
 ALTER TABLE t MERGE PARTITIONS (tp_0_1, tp_1_2) INTO tp_0_2;
 ROLLBACK;
 
+-- Check the new partition inherits parent's tablespace
+CREATE TABLE t (i int PRIMARY KEY USING INDEX TABLESPACE regress_tblspace)
+  PARTITION BY RANGE (i) TABLESPACE regress_tblspace;
+CREATE TABLE tp_0_1 PARTITION OF t FOR VALUES FROM (0) TO (1);
+CREATE TABLE tp_1_2 PARTITION OF t FOR VALUES FROM (1) TO (2);
+ALTER TABLE t MERGE PARTITIONS (tp_0_1, tp_1_2) INTO tp_0_2;
+SELECT tablespace FROM pg_tables WHERE tablename IN ('t', 'tp_0_2') ORDER BY tablespace;
+SELECT tablespace FROM pg_indexes WHERE tablename IN ('t', 'tp_0_2') ORDER BY tablespace;
+DROP TABLE t;
+
 -- Check the new partition inherits parent's table access method
 SET search_path = partitions_merge_schema, public;
 CREATE ACCESS METHOD partitions_merge_heap TYPE TABLE HANDLER heap_tableam_handler;
diff --git a/src/test/regress/sql/partition_split.sql b/src/test/regress/sql/partition_split.sql
index 9c7d47963c..5f0f93dd07 100644
--- a/src/test/regress/sql/partition_split.sql
+++ b/src/test/regress/sql/partition_split.sql
@@ -880,6 +880,17 @@ SELECT c.oid::pg_catalog.regclass, pg_catalog.pg_get_expr(c.relpartbound, c.oid)
 
 DROP TABLE t;
 
+-- Check new partitions inherits parent's tablespace
+CREATE TABLE t (i int PRIMARY KEY USING INDEX TABLESPACE regress_tblspace)
+  PARTITION BY RANGE (i) TABLESPACE regress_tblspace;
+CREATE TABLE tp_0_2 PARTITION OF t FOR VALUES FROM (0) TO (2);
+ALTER TABLE t SPLIT PARTITION tp_0_2 INTO
+  (PARTITION tp_0_1 FOR VALUES FROM (0) TO (1),
+   PARTITION tp_1_2 FOR VALUES FROM (1) TO (2));
+SELECT tablespace FROM pg_tables WHERE tablename IN ('t', 'tp_0_1', 'tp_1_2') ORDER BY tablespace;
+SELECT tablespace FROM pg_indexes WHERE tablename IN ('t', 'tp_0_1', 'tp_1_2') ORDER BY tablespace;
+DROP TABLE t;
+
 -- Check new partitions inherits parent's table access method
 CREATE ACCESS METHOD partition_split_heap TYPE TABLE HANDLER heap_tableam_handler;
 CREATE TABLE t (i int) PARTITION BY RANGE (i) USING partition_split_heap;
-- 
2.45.2

---

From: David Rowley <[email-masked]>
Date: Wed, 10 Jul 2024 21:58:25 +1200

> Is this an oversight or intentional?

My expectation of this feature is that the tablespace choice would
work the same as what was done in ca4103025 to make it inherit from
the partition table's tablespace. I imagine we might get complaints if
it does not follow the same logic.

I've not looked at your patch, but if the behaviour is as you describe
and the patch changes that to follow ca4103025, then +1 from me.

David