Support specify tablespace for each merged/split partition

Started by Junwang Zhaoover 1 year ago9 messages

zhjwpku@gmail.com

over 1 year ago

1 attachment(s)

In [1]/messages/by-id/abaf390b-3320-40a5-8815-ef476db5cfe7@oss.nttdata.com, it is suggested that it might be a good idea to support
specifying the tablespace for each merged/split partition.

We can do the following after this feature is supported:

CREATE TABLESPACE tblspc LOCATION '/tmp/tblspc';
CREATE TABLE t (i int PRIMARY KEY) PARTITION BY RANGE (i);
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 TABLESPACE tblspc;

ALTER TABLE t SPLIT PARTITION tp_0_2 INTO
(PARTITION tp_0_1 FOR VALUES FROM (0) TO (1) TABLESPACE tblspc,
PARTITION tp_1_2 FOR VALUES FROM (1) TO (2));

[1]: /messages/by-id/abaf390b-3320-40a5-8815-ef476db5cfe7@oss.nttdata.com

--
Regards
Junwang Zhao

Attachments:

0001-support-specify-tablespace-for-each-merged-split.patchapplication/octet-stream; name=0001-support-specify-tablespace-for-each-merged-split.patchDownload

From 233574d0b9b7ed9258f20e147bb9d0677b4faabf Mon Sep 17 00:00:00 2001
From: Zhao Junwang <zhjwpku@gmail.com>
Date: Fri, 12 Jul 2024 10:04:29 +0000
Subject: [PATCH] support specify tablespace for each merged/split

It might be a good idea that we can specify the tablespace
for each merged/split partition.

Signed-off-by: Zhao Junwang <zhjwpku@gmail.com>
---
 doc/src/sgml/ref/alter_table.sgml             |  6 ++---
 src/backend/commands/tablecmds.c              | 11 ++++----
 src/backend/parser/gram.y                     |  6 +++--
 src/include/nodes/parsenodes.h                |  4 ++-
 src/test/regress/expected/partition_merge.out | 25 +++++++++++++++++
 src/test/regress/expected/partition_split.out | 27 +++++++++++++++++++
 src/test/regress/sql/partition_merge.sql      | 14 ++++++++++
 src/test/regress/sql/partition_split.sql      | 14 ++++++++++
 8 files changed, 96 insertions(+), 11 deletions(-)

diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml
index 6a2822adad7..d7f64b684d4 100644
--- a/doc/src/sgml/ref/alter_table.sgml
+++ b/doc/src/sgml/ref/alter_table.sgml
@@ -39,11 +39,11 @@ ALTER TABLE [ IF EXISTS ] <replaceable class="parameter">name</replaceable>
     DETACH PARTITION <replaceable class="parameter">partition_name</replaceable> [ CONCURRENTLY | FINALIZE ]
 ALTER TABLE [ IF EXISTS ] <replaceable class="parameter">name</replaceable>
     SPLIT PARTITION <replaceable class="parameter">partition_name</replaceable> INTO
-        (PARTITION <replaceable class="parameter">partition_name1</replaceable> { FOR VALUES <replaceable class="parameter">partition_bound_spec</replaceable> | DEFAULT },
-         PARTITION <replaceable class="parameter">partition_name2</replaceable> { FOR VALUES <replaceable class="parameter">partition_bound_spec</replaceable> | DEFAULT } [, ...])
+        (PARTITION <replaceable class="parameter">partition_name1</replaceable> { FOR VALUES <replaceable class="parameter">partition_bound_spec</replaceable> | DEFAULT } [ TABLESPACE tablespace_name ],
+         PARTITION <replaceable class="parameter">partition_name2</replaceable> { FOR VALUES <replaceable class="parameter">partition_bound_spec</replaceable> | DEFAULT } [ TABLESPACE tablespace_name ] [, ...])
 ALTER TABLE [ IF EXISTS ] <replaceable class="parameter">name</replaceable>
     MERGE PARTITIONS (<replaceable class="parameter">partition_name1</replaceable>, <replaceable class="parameter">partition_name2</replaceable> [, ...])
-        INTO <replaceable class="parameter">partition_name</replaceable>
+        INTO <replaceable class="parameter">partition_name</replaceable> [ TABLESPACE tablespace_name ]
 
 <phrase>where <replaceable class="parameter">action</replaceable> is one of:</phrase>
 
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 721d24783b4..b0979a59c7a 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -20315,8 +20315,8 @@ moveSplitTableRows(Relation rel, Relation splitRel, List *partlist, List *newPar
  * Function returns the created relation (locked in AccessExclusiveLock mode).
  */
 static Relation
-createPartitionTable(RangeVar *newPartName, Relation modelRel,
-					 AlterTableUtilityContext *context)
+createPartitionTable(RangeVar *newPartName, char *tablespacename,
+					 Relation modelRel, AlterTableUtilityContext *context)
 {
 	CreateStmt *createStmt;
 	TableLikeClause *tlc;
@@ -20340,7 +20340,8 @@ createPartitionTable(RangeVar *newPartName, Relation modelRel,
 	createStmt->constraints = NIL;
 	createStmt->options = NIL;
 	createStmt->oncommit = ONCOMMIT_NOOP;
-	createStmt->tablespacename = get_tablespace_name(modelRel->rd_rel->reltablespace);
+	createStmt->tablespacename = tablespacename ? tablespacename :
+		get_tablespace_name(modelRel->rd_rel->reltablespace);;
 	createStmt->if_not_exists = false;
 	createStmt->accessMethod = get_am_name(modelRel->rd_rel->relam);
 
@@ -20499,7 +20500,7 @@ ATExecSplitPartition(List **wqueue, AlteredTableInfo *tab, Relation rel,
 		SinglePartitionSpec *sps = (SinglePartitionSpec *) lfirst(listptr);
 		Relation	newPartRel;
 
-		newPartRel = createPartitionTable(sps->name, rel, context);
+		newPartRel = createPartitionTable(sps->name, sps->tablespacename, rel, context);
 		newPartRels = lappend(newPartRels, newPartRel);
 	}
 
@@ -20743,7 +20744,7 @@ ATExecMergePartitions(List **wqueue, AlteredTableInfo *tab, Relation rel,
 	}
 
 	/* Create table for new partition, use partitioned table as model. */
-	newPartRel = createPartitionTable(cmd->name, rel, context);
+	newPartRel = createPartitionTable(cmd->name, cmd->tablespacename, rel, context);
 
 	/* Copy data from merged partitions to new partition. */
 	moveMergedTablesRows(rel, mergingPartitionsList, newPartRel);
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index a043fd4c669..95bfe638ab4 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -2317,12 +2317,13 @@ partitions_list:
 		;
 
 SinglePartitionSpec:
-			PARTITION qualified_name PartitionBoundSpec
+			PARTITION qualified_name PartitionBoundSpec OptTableSpace
 				{
 					SinglePartitionSpec *n = makeNode(SinglePartitionSpec);
 
 					n->name = $2;
 					n->bound = $3;
+					n->tablespacename = $4;
 
 					$$ = n;
 				}
@@ -2387,7 +2388,7 @@ partition_cmd:
 					$$ = (Node *) n;
 				}
 			/* ALTER TABLE <name> MERGE PARTITIONS () INTO <partition_name> */
-			| MERGE PARTITIONS '(' qualified_name_list ')' INTO qualified_name
+			| MERGE PARTITIONS '(' qualified_name_list ')' INTO qualified_name OptTableSpace
 				{
 					AlterTableCmd *n = makeNode(AlterTableCmd);
 					PartitionCmd *cmd = makeNode(PartitionCmd);
@@ -2397,6 +2398,7 @@ partition_cmd:
 					cmd->bound = NULL;
 					cmd->partlist = $4;
 					cmd->concurrent = false;
+					cmd->tablespacename = $8;
 					n->def = (Node *) cmd;
 					$$ = (Node *) n;
 				}
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 85a62b538e5..a0f12d05cc5 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -945,11 +945,12 @@ typedef struct SinglePartitionSpec
 	NodeTag		type;
 
 	RangeVar   *name;			/* name of partition */
+	char	   *tablespacename; /* name of tablespace, or NULL for default */
 	PartitionBoundSpec *bound;	/* FOR VALUES, if attaching */
 } SinglePartitionSpec;
 
 /*
- * PartitionCmd - info for ALTER TABLE/INDEX ATTACH/DETACH PARTITION commands
+ * PartitionCmd - info for ALTER TABLE/INDEX ATTACH/DETACH/MERGE/SPLIT PARTITION commands
  */
 typedef struct PartitionCmd
 {
@@ -959,6 +960,7 @@ typedef struct PartitionCmd
 	List	   *partlist;		/* list of partitions, for MERGE/SPLIT
 								 * PARTITION command */
 	bool		concurrent;
+	char	   *tablespacename; /* name of tablespace, or NULL for default */
 } PartitionCmd;
 
 /****************************************************************************
diff --git a/src/test/regress/expected/partition_merge.out b/src/test/regress/expected/partition_merge.out
index 59836e2d35e..c28836a241c 100644
--- a/src/test/regress/expected/partition_merge.out
+++ b/src/test/regress/expected/partition_merge.out
@@ -886,6 +886,31 @@ SELECT tablename, indexname, tablespace FROM pg_indexes
  tp_0_2    | tp_0_2_pkey | regress_tblspace
 (2 rows)
 
+DROP TABLE t;
+-- Check the merged partition can be set to a different tablespace
+SET search_path = partitions_merge_schema, public;
+CREATE TABLE t (i int PRIMARY KEY) PARTITION BY RANGE (i);
+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 TABLESPACE regress_tblspace;
+SELECT tablename, tablespace FROM pg_tables
+  WHERE tablename IN ('t', 'tp_0_2') AND schemaname = 'partitions_merge_schema'
+  ORDER BY tablename, tablespace;
+ tablename |    tablespace    
+-----------+------------------
+ t         | 
+ tp_0_2    | regress_tblspace
+(2 rows)
+
+SELECT tablename, indexname, tablespace FROM pg_indexes
+  WHERE tablename IN ('t', 'tp_0_2') AND schemaname = 'partitions_merge_schema'
+  ORDER BY tablename, indexname, tablespace;
+ tablename |  indexname  | tablespace 
+-----------+-------------+------------
+ t         | t_pkey      | 
+ tp_0_2    | tp_0_2_pkey | 
+(2 rows)
+
 DROP TABLE t;
 -- Check the new partition inherits parent's table access method
 SET search_path = partitions_merge_schema, public;
diff --git a/src/test/regress/expected/partition_split.out b/src/test/regress/expected/partition_split.out
index dc9a5130ccc..d3853e4befc 100644
--- a/src/test/regress/expected/partition_split.out
+++ b/src/test/regress/expected/partition_split.out
@@ -1521,6 +1521,33 @@ SELECT tablename, indexname, tablespace FROM pg_indexes
  tp_1_2    | tp_1_2_pkey | regress_tblspace
 (3 rows)
 
+DROP TABLE t;
+-- Check the split partitions can be set to a different tablespace
+CREATE TABLE t (i int PRIMARY KEY) PARTITION BY RANGE (i);
+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) TABLESPACE regress_tblspace,
+   PARTITION tp_1_2 FOR VALUES FROM (1) TO (2));
+SELECT tablename, tablespace FROM pg_tables
+  WHERE tablename IN ('t', 'tp_0_1', 'tp_1_2') AND schemaname = 'partition_split_schema'
+  ORDER BY tablename, tablespace;
+ tablename |    tablespace    
+-----------+------------------
+ t         | 
+ tp_0_1    | regress_tblspace
+ tp_1_2    | 
+(3 rows)
+
+SELECT tablename, indexname, tablespace FROM pg_indexes
+  WHERE tablename IN ('t', 'tp_0_1', 'tp_1_2') AND schemaname = 'partition_split_schema'
+  ORDER BY tablename, indexname, tablespace;
+ tablename |  indexname  | tablespace 
+-----------+-------------+------------
+ t         | t_pkey      | 
+ tp_0_1    | tp_0_1_pkey | 
+ tp_1_2    | tp_1_2_pkey | 
+(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 bede819af94..741c9696ec8 100644
--- a/src/test/regress/sql/partition_merge.sql
+++ b/src/test/regress/sql/partition_merge.sql
@@ -551,6 +551,20 @@ SELECT tablename, indexname, tablespace FROM pg_indexes
   ORDER BY tablename, indexname, tablespace;
 DROP TABLE t;
 
+-- Check the merged partition can be set to a different tablespace
+SET search_path = partitions_merge_schema, public;
+CREATE TABLE t (i int PRIMARY KEY) PARTITION BY RANGE (i);
+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 TABLESPACE regress_tblspace;
+SELECT tablename, tablespace FROM pg_tables
+  WHERE tablename IN ('t', 'tp_0_2') AND schemaname = 'partitions_merge_schema'
+  ORDER BY tablename, tablespace;
+SELECT tablename, indexname, tablespace FROM pg_indexes
+  WHERE tablename IN ('t', 'tp_0_2') AND schemaname = 'partitions_merge_schema'
+  ORDER BY tablename, indexname, 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 ef5ea07f740..aaaee1d8769 100644
--- a/src/test/regress/sql/partition_split.sql
+++ b/src/test/regress/sql/partition_split.sql
@@ -895,6 +895,20 @@ SELECT tablename, indexname, tablespace FROM pg_indexes
   ORDER BY tablename, indexname, tablespace;
 DROP TABLE t;
 
+-- Check the split partitions can be set to a different tablespace
+CREATE TABLE t (i int PRIMARY KEY) PARTITION BY RANGE (i);
+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) TABLESPACE regress_tblspace,
+   PARTITION tp_1_2 FOR VALUES FROM (1) TO (2));
+SELECT tablename, tablespace FROM pg_tables
+  WHERE tablename IN ('t', 'tp_0_1', 'tp_1_2') AND schemaname = 'partition_split_schema'
+  ORDER BY tablename, tablespace;
+SELECT tablename, indexname, tablespace FROM pg_indexes
+  WHERE tablename IN ('t', 'tp_0_1', 'tp_1_2') AND schemaname = 'partition_split_schema'
+  ORDER BY tablename, indexname, 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.39.2

Amul Sul

sulamul@gmail.com

over 1 year ago

In reply to: Junwang Zhao (#1)

Re: Support specify tablespace for each merged/split partition

On Mon, Jul 15, 2024 at 11:19 AM Junwang Zhao <zhjwpku@gmail.com> wrote:

In [1], it is suggested that it might be a good idea to support
specifying the tablespace for each merged/split partition.

We can do the following after this feature is supported:

CREATE TABLESPACE tblspc LOCATION '/tmp/tblspc';
CREATE TABLE t (i int PRIMARY KEY) PARTITION BY RANGE (i);
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 TABLESPACE tblspc;

ALTER TABLE t SPLIT PARTITION tp_0_2 INTO
(PARTITION tp_0_1 FOR VALUES FROM (0) TO (1) TABLESPACE tblspc,
PARTITION tp_1_2 FOR VALUES FROM (1) TO (2));

[1] /messages/by-id/abaf390b-3320-40a5-8815-ef476db5cfe7@oss.nttdata.com

+1 for this enhancement. Here are few comments for the patch:

-        INTO <replaceable class="parameter">partition_name</replaceable>
+       INTO <replaceable
class="parameter">partition_name</replaceable> [ TABLESPACE
tablespace_name ]

tablespace_name should be wrapped in the <replaceable> tag, like partition_name.
--

 static Relation
-createPartitionTable(RangeVar *newPartName, Relation modelRel,
-                    AlterTableUtilityContext *context)
+createPartitionTable(RangeVar *newPartName, char *tablespacename,
+

The comment should mention the tablespace setting in the same way it
mentions the access method.
--

 /*
- * PartitionCmd - info for ALTER TABLE/INDEX ATTACH/DETACH PARTITION commands
+ * PartitionCmd - info for ALTER TABLE/INDEX
ATTACH/DETACH/MERGE/SPLIT PARTITION commands
  */

This change should be a separate patch since it makes sense
independently of your patch. Also, the comments for the "name"
variable in the same structure need to be updated.
--

+SELECT tablename, tablespace FROM pg_tables
+  WHERE tablename IN ('t', 'tp_0_2') AND schemaname = 'partitions_merge_schema'
+  ORDER BY tablename, tablespace;
+ tablename |    tablespace
+-----------+------------------
+ t         |
+ tp_0_2    | regress_tblspace
+(2 rows)
+
+SELECT tablename, indexname, tablespace FROM pg_indexes
+  WHERE tablename IN ('t', 'tp_0_2') AND schemaname = 'partitions_merge_schema'
+  ORDER BY tablename, indexname, tablespace;
+ tablename |  indexname  | tablespace
+-----------+-------------+------------
+ t         | t_pkey      |
+ tp_0_2    | tp_0_2_pkey |
+(2 rows)
+

This seems problematic to me. The index should be in the same
tablespace as the table.
--

Please add the commitfest[1] entry if not already done.

1] https://commitfest.postgresql.org/

Regards,
Amul

Junwang Zhao

zhjwpku@gmail.com

over 1 year ago

In reply to: Amul Sul (#2)

Re: Support specify tablespace for each merged/split partition

Hi Amul,

Thanks for your review.

On Mon, Aug 5, 2024 at 8:38 PM Amul Sul <sulamul@gmail.com> wrote:

On Mon, Jul 15, 2024 at 11:19 AM Junwang Zhao <zhjwpku@gmail.com> wrote:

In [1], it is suggested that it might be a good idea to support
specifying the tablespace for each merged/split partition.

We can do the following after this feature is supported:

CREATE TABLESPACE tblspc LOCATION '/tmp/tblspc';
CREATE TABLE t (i int PRIMARY KEY) PARTITION BY RANGE (i);
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 TABLESPACE tblspc;

ALTER TABLE t SPLIT PARTITION tp_0_2 INTO
(PARTITION tp_0_1 FOR VALUES FROM (0) TO (1) TABLESPACE tblspc,
PARTITION tp_1_2 FOR VALUES FROM (1) TO (2));

[1] /messages/by-id/abaf390b-3320-40a5-8815-ef476db5cfe7@oss.nttdata.com

+1 for this enhancement. Here are few comments for the patch:
-        INTO <replaceable class="parameter">partition_name</replaceable>
+       INTO <replaceable
class="parameter">partition_name</replaceable> [ TABLESPACE
tablespace_name ]
tablespace_name should be wrapped in the <replaceable> tag, like partition_name.

Will add in next version.

--
static Relation
-createPartitionTable(RangeVar *newPartName, Relation modelRel,
-                    AlterTableUtilityContext *context)
+createPartitionTable(RangeVar *newPartName, char *tablespacename,
+
The comment should mention the tablespace setting in the same way it
mentions the access method.

I'm not good at wording, can you give some advice?

--
/*
- * PartitionCmd - info for ALTER TABLE/INDEX ATTACH/DETACH PARTITION commands
+ * PartitionCmd - info for ALTER TABLE/INDEX
ATTACH/DETACH/MERGE/SPLIT PARTITION commands
*/
This change should be a separate patch since it makes sense
independently of your patch. Also, the comments for the "name"
variable in the same structure need to be updated.

Will be split into a separate patch in the next version.

+SELECT tablename, tablespace FROM pg_tables
+  WHERE tablename IN ('t', 'tp_0_2') AND schemaname = 'partitions_merge_schema'
+  ORDER BY tablename, tablespace;
+ tablename |    tablespace
+-----------+------------------
+ t         |
+ tp_0_2    | regress_tblspace
+(2 rows)
+
+SELECT tablename, indexname, tablespace FROM pg_indexes
+  WHERE tablename IN ('t', 'tp_0_2') AND schemaname = 'partitions_merge_schema'
+  ORDER BY tablename, indexname, tablespace;
+ tablename |  indexname  | tablespace
+-----------+-------------+------------
+ t         | t_pkey      |
+ tp_0_2    | tp_0_2_pkey |
+(2 rows)
+

This seems problematic to me. The index should be in the same
tablespace as the table.

I'm not sure about this, it seems to me that partition index will alway
inherit the tablespaceId of its parent(see generateClonedIndexStmt),
do you think we should change the signature of this function?

One thing worth mentioning is that for UNIQUE/PRIMARY KEY,
it allows setting *USING INDEX TABLESPACE tablespace_name*,
I don't think we should change the index tablespace in this case,
what do you think?

--

Please add the commitfest[1] entry if not already done.

1] https://commitfest.postgresql.org/

https://commitfest.postgresql.org/49/5157/

I have added you as a reviewer, hope you don't mind.

Regards,
Amul

--
Regards
Junwang Zhao

Amul Sul

sulamul@gmail.com

over 1 year ago

In reply to: Junwang Zhao (#3)

Re: Support specify tablespace for each merged/split partition

On Mon, Aug 5, 2024 at 9:05 PM Junwang Zhao <zhjwpku@gmail.com> wrote:

Hi Amul,

Thanks for your review.

On Mon, Aug 5, 2024 at 8:38 PM Amul Sul <sulamul@gmail.com> wrote:
On Mon, Jul 15, 2024 at 11:19 AM Junwang Zhao <zhjwpku@gmail.com> wrote:
[...]
static Relation
-createPartitionTable(RangeVar *newPartName, Relation modelRel,
-                    AlterTableUtilityContext *context)
+createPartitionTable(RangeVar *newPartName, char *tablespacename,
+
The comment should mention the tablespace setting in the same way it
mentions the access method.
I'm not good at wording, can you give some advice?

My suggestion is to rewrite the third paragraph as follows, but
someone else might have a better version:
---
The new partitions will also be created in the same tablespace as the parent
if not specified. Also, this function sets the new partition access method
same as parent table access methods (similarly to CREATE TABLE ... PARTITION
OF). It checks that parent and child tables have compatible persistence.
---

+SELECT tablename, tablespace FROM pg_tables
+  WHERE tablename IN ('t', 'tp_0_2') AND schemaname = 'partitions_merge_schema'
+  ORDER BY tablename, tablespace;
+ tablename |    tablespace
+-----------+------------------
+ t         |
+ tp_0_2    | regress_tblspace
+(2 rows)
+
+SELECT tablename, indexname, tablespace FROM pg_indexes
+  WHERE tablename IN ('t', 'tp_0_2') AND schemaname = 'partitions_merge_schema'
+  ORDER BY tablename, indexname, tablespace;
+ tablename |  indexname  | tablespace
+-----------+-------------+------------
+ t         | t_pkey      |
+ tp_0_2    | tp_0_2_pkey |
+(2 rows)
+
This seems problematic to me. The index should be in the same
tablespace as the table.
I'm not sure about this, it seems to me that partition index will alway
inherit the tablespaceId of its parent(see generateClonedIndexStmt),
do you think we should change the signature of this function?

One thing worth mentioning is that for UNIQUE/PRIMARY KEY,
it allows setting *USING INDEX TABLESPACE tablespace_name*,
I don't think we should change the index tablespace in this case,
what do you think?

I think you are correct; my understanding is a bit hazy.

I have added you as a reviewer, hope you don't mind.

Thank you.

Regards,
Amul

Junwang Zhao

zhjwpku@gmail.com

over 1 year ago

In reply to: Amul Sul (#4)

2 attachment(s)

Re: Support specify tablespace for each merged/split partition

On Tue, Aug 6, 2024 at 5:34 PM Amul Sul <sulamul@gmail.com> wrote:

On Mon, Aug 5, 2024 at 9:05 PM Junwang Zhao <zhjwpku@gmail.com> wrote:
Hi Amul,

Thanks for your review.

On Mon, Aug 5, 2024 at 8:38 PM Amul Sul <sulamul@gmail.com> wrote:
On Mon, Jul 15, 2024 at 11:19 AM Junwang Zhao <zhjwpku@gmail.com> wrote:
[...]
static Relation
-createPartitionTable(RangeVar *newPartName, Relation modelRel,
-                    AlterTableUtilityContext *context)
+createPartitionTable(RangeVar *newPartName, char *tablespacename,
+
The comment should mention the tablespace setting in the same way it
mentions the access method.
I'm not good at wording, can you give some advice?
My suggestion is to rewrite the third paragraph as follows, but
someone else might have a better version:
---
The new partitions will also be created in the same tablespace as the parent
if not specified. Also, this function sets the new partition access method
same as parent table access methods (similarly to CREATE TABLE ... PARTITION
OF). It checks that parent and child tables have compatible persistence.
---

I changed to this with minor changes.

+SELECT tablename, tablespace FROM pg_tables
+  WHERE tablename IN ('t', 'tp_0_2') AND schemaname = 'partitions_merge_schema'
+  ORDER BY tablename, tablespace;
+ tablename |    tablespace
+-----------+------------------
+ t         |
+ tp_0_2    | regress_tblspace
+(2 rows)
+
+SELECT tablename, indexname, tablespace FROM pg_indexes
+  WHERE tablename IN ('t', 'tp_0_2') AND schemaname = 'partitions_merge_schema'
+  ORDER BY tablename, indexname, tablespace;
+ tablename |  indexname  | tablespace
+-----------+-------------+------------
+ t         | t_pkey      |
+ tp_0_2    | tp_0_2_pkey |
+(2 rows)
+
This seems problematic to me. The index should be in the same
tablespace as the table.
I'm not sure about this, it seems to me that partition index will alway
inherit the tablespaceId of its parent(see generateClonedIndexStmt),
do you think we should change the signature of this function?

One thing worth mentioning is that for UNIQUE/PRIMARY KEY,
it allows setting *USING INDEX TABLESPACE tablespace_name*,
I don't think we should change the index tablespace in this case,
what do you think?
I think you are correct; my understanding is a bit hazy.

Thanks for your confirmation.

Attached v2 addressed all the problems you mentioned, thanks.

I have added you as a reviewer, hope you don't mind.

Thank you.

Regards,
Amul

--
Regards
Junwang Zhao

Attachments:

v2-0001-support-specify-tablespace-for-each-merged-split.patchapplication/octet-stream; name=v2-0001-support-specify-tablespace-for-each-merged-split.patchDownload

From 4c066c98690544460863f85e944c50400013bb7d Mon Sep 17 00:00:00 2001
From: Zhao Junwang <zhjwpku@gmail.com>
Date: Fri, 12 Jul 2024 10:04:29 +0000
Subject: [PATCH v2 1/2] support specify tablespace for each merged/split

It might be a good idea that we can specify the tablespace
for each merged/split partition.

Signed-off-by: Zhao Junwang <zhjwpku@gmail.com>
---
 doc/src/sgml/ref/alter_table.sgml             |  6 ++---
 src/backend/commands/tablecmds.c              | 16 ++++++-----
 src/backend/parser/gram.y                     |  6 +++--
 src/include/nodes/parsenodes.h                |  2 ++
 src/test/regress/expected/partition_merge.out | 25 +++++++++++++++++
 src/test/regress/expected/partition_split.out | 27 +++++++++++++++++++
 src/test/regress/sql/partition_merge.sql      | 14 ++++++++++
 src/test/regress/sql/partition_split.sql      | 14 ++++++++++
 8 files changed, 98 insertions(+), 12 deletions(-)

diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml
index 6a2822adad..259e8d9402 100644
--- a/doc/src/sgml/ref/alter_table.sgml
+++ b/doc/src/sgml/ref/alter_table.sgml
@@ -39,11 +39,11 @@ ALTER TABLE [ IF EXISTS ] <replaceable class="parameter">name</replaceable>
     DETACH PARTITION <replaceable class="parameter">partition_name</replaceable> [ CONCURRENTLY | FINALIZE ]
 ALTER TABLE [ IF EXISTS ] <replaceable class="parameter">name</replaceable>
     SPLIT PARTITION <replaceable class="parameter">partition_name</replaceable> INTO
-        (PARTITION <replaceable class="parameter">partition_name1</replaceable> { FOR VALUES <replaceable class="parameter">partition_bound_spec</replaceable> | DEFAULT },
-         PARTITION <replaceable class="parameter">partition_name2</replaceable> { FOR VALUES <replaceable class="parameter">partition_bound_spec</replaceable> | DEFAULT } [, ...])
+        (PARTITION <replaceable class="parameter">partition_name1</replaceable> { FOR VALUES <replaceable class="parameter">partition_bound_spec</replaceable> | DEFAULT } [ TABLESPACE <replaceable class="parameter">tablespace_name</replaceable> ],
+         PARTITION <replaceable class="parameter">partition_name2</replaceable> { FOR VALUES <replaceable class="parameter">partition_bound_spec</replaceable> | DEFAULT } [ TABLESPACE <replaceable class="parameter">tablespace_name</replaceable> ] [, ...])
 ALTER TABLE [ IF EXISTS ] <replaceable class="parameter">name</replaceable>
     MERGE PARTITIONS (<replaceable class="parameter">partition_name1</replaceable>, <replaceable class="parameter">partition_name2</replaceable> [, ...])
-        INTO <replaceable class="parameter">partition_name</replaceable>
+        INTO <replaceable class="parameter">partition_name</replaceable> [ TABLESPACE <replaceable class="parameter">tablespace_name</replaceable> ]
 
 <phrase>where <replaceable class="parameter">action</replaceable> is one of:</phrase>
 
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 0b2a52463f..d5f11b7601 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -20315,15 +20315,16 @@ moveSplitTableRows(Relation rel, Relation splitRel, List *partlist, List *newPar
  * Emulates command: CREATE [TEMP] TABLE <newPartName> (LIKE <modelRel's name>
  * INCLUDING ALL EXCLUDING INDEXES EXCLUDING IDENTITY EXCLUDING STATISTICS)
  *
- * Also, this function sets the new partition access method same as parent
- * table access methods (similarly to CREATE TABLE ... PARTITION OF).  It
+ * The new partition will be created in the same tablespace as the parent if not
+ * specified.  Also, this function sets the new partition access method same as
+ * parent table access methods (similarly to CREATE TABLE ... PARTITION OF).  It
  * checks that parent and child tables have compatible persistence.
  *
  * Function returns the created relation (locked in AccessExclusiveLock mode).
  */
 static Relation
-createPartitionTable(RangeVar *newPartName, Relation modelRel,
-					 AlterTableUtilityContext *context)
+createPartitionTable(RangeVar *newPartName, char *tablespacename,
+					 Relation modelRel, AlterTableUtilityContext *context)
 {
 	CreateStmt *createStmt;
 	TableLikeClause *tlc;
@@ -20347,7 +20348,8 @@ createPartitionTable(RangeVar *newPartName, Relation modelRel,
 	createStmt->constraints = NIL;
 	createStmt->options = NIL;
 	createStmt->oncommit = ONCOMMIT_NOOP;
-	createStmt->tablespacename = get_tablespace_name(modelRel->rd_rel->reltablespace);
+	createStmt->tablespacename = tablespacename ? tablespacename :
+		get_tablespace_name(modelRel->rd_rel->reltablespace);;
 	createStmt->if_not_exists = false;
 	createStmt->accessMethod = get_am_name(modelRel->rd_rel->relam);
 
@@ -20506,7 +20508,7 @@ ATExecSplitPartition(List **wqueue, AlteredTableInfo *tab, Relation rel,
 		SinglePartitionSpec *sps = (SinglePartitionSpec *) lfirst(listptr);
 		Relation	newPartRel;
 
-		newPartRel = createPartitionTable(sps->name, rel, context);
+		newPartRel = createPartitionTable(sps->name, sps->tablespacename, rel, context);
 		newPartRels = lappend(newPartRels, newPartRel);
 	}
 
@@ -20750,7 +20752,7 @@ ATExecMergePartitions(List **wqueue, AlteredTableInfo *tab, Relation rel,
 	}
 
 	/* Create table for new partition, use partitioned table as model. */
-	newPartRel = createPartitionTable(cmd->name, rel, context);
+	newPartRel = createPartitionTable(cmd->name, cmd->tablespacename, rel, context);
 
 	/* Copy data from merged partitions to new partition. */
 	moveMergedTablesRows(rel, mergingPartitionsList, newPartRel);
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index a043fd4c66..95bfe638ab 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -2317,12 +2317,13 @@ partitions_list:
 		;
 
 SinglePartitionSpec:
-			PARTITION qualified_name PartitionBoundSpec
+			PARTITION qualified_name PartitionBoundSpec OptTableSpace
 				{
 					SinglePartitionSpec *n = makeNode(SinglePartitionSpec);
 
 					n->name = $2;
 					n->bound = $3;
+					n->tablespacename = $4;
 
 					$$ = n;
 				}
@@ -2387,7 +2388,7 @@ partition_cmd:
 					$$ = (Node *) n;
 				}
 			/* ALTER TABLE <name> MERGE PARTITIONS () INTO <partition_name> */
-			| MERGE PARTITIONS '(' qualified_name_list ')' INTO qualified_name
+			| MERGE PARTITIONS '(' qualified_name_list ')' INTO qualified_name OptTableSpace
 				{
 					AlterTableCmd *n = makeNode(AlterTableCmd);
 					PartitionCmd *cmd = makeNode(PartitionCmd);
@@ -2397,6 +2398,7 @@ partition_cmd:
 					cmd->bound = NULL;
 					cmd->partlist = $4;
 					cmd->concurrent = false;
+					cmd->tablespacename = $8;
 					n->def = (Node *) cmd;
 					$$ = (Node *) n;
 				}
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 85a62b538e..9ccdcb9eff 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -945,6 +945,7 @@ typedef struct SinglePartitionSpec
 	NodeTag		type;
 
 	RangeVar   *name;			/* name of partition */
+	char	   *tablespacename; /* name of tablespace, or NULL for default */
 	PartitionBoundSpec *bound;	/* FOR VALUES, if attaching */
 } SinglePartitionSpec;
 
@@ -959,6 +960,7 @@ typedef struct PartitionCmd
 	List	   *partlist;		/* list of partitions, for MERGE/SPLIT
 								 * PARTITION command */
 	bool		concurrent;
+	char	   *tablespacename; /* name of tablespace, or NULL for default */
 } PartitionCmd;
 
 /****************************************************************************
diff --git a/src/test/regress/expected/partition_merge.out b/src/test/regress/expected/partition_merge.out
index 59836e2d35..c28836a241 100644
--- a/src/test/regress/expected/partition_merge.out
+++ b/src/test/regress/expected/partition_merge.out
@@ -886,6 +886,31 @@ SELECT tablename, indexname, tablespace FROM pg_indexes
  tp_0_2    | tp_0_2_pkey | regress_tblspace
 (2 rows)
 
+DROP TABLE t;
+-- Check the merged partition can be set to a different tablespace
+SET search_path = partitions_merge_schema, public;
+CREATE TABLE t (i int PRIMARY KEY) PARTITION BY RANGE (i);
+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 TABLESPACE regress_tblspace;
+SELECT tablename, tablespace FROM pg_tables
+  WHERE tablename IN ('t', 'tp_0_2') AND schemaname = 'partitions_merge_schema'
+  ORDER BY tablename, tablespace;
+ tablename |    tablespace    
+-----------+------------------
+ t         | 
+ tp_0_2    | regress_tblspace
+(2 rows)
+
+SELECT tablename, indexname, tablespace FROM pg_indexes
+  WHERE tablename IN ('t', 'tp_0_2') AND schemaname = 'partitions_merge_schema'
+  ORDER BY tablename, indexname, tablespace;
+ tablename |  indexname  | tablespace 
+-----------+-------------+------------
+ t         | t_pkey      | 
+ tp_0_2    | tp_0_2_pkey | 
+(2 rows)
+
 DROP TABLE t;
 -- Check the new partition inherits parent's table access method
 SET search_path = partitions_merge_schema, public;
diff --git a/src/test/regress/expected/partition_split.out b/src/test/regress/expected/partition_split.out
index dc9a5130cc..d3853e4bef 100644
--- a/src/test/regress/expected/partition_split.out
+++ b/src/test/regress/expected/partition_split.out
@@ -1521,6 +1521,33 @@ SELECT tablename, indexname, tablespace FROM pg_indexes
  tp_1_2    | tp_1_2_pkey | regress_tblspace
 (3 rows)
 
+DROP TABLE t;
+-- Check the split partitions can be set to a different tablespace
+CREATE TABLE t (i int PRIMARY KEY) PARTITION BY RANGE (i);
+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) TABLESPACE regress_tblspace,
+   PARTITION tp_1_2 FOR VALUES FROM (1) TO (2));
+SELECT tablename, tablespace FROM pg_tables
+  WHERE tablename IN ('t', 'tp_0_1', 'tp_1_2') AND schemaname = 'partition_split_schema'
+  ORDER BY tablename, tablespace;
+ tablename |    tablespace    
+-----------+------------------
+ t         | 
+ tp_0_1    | regress_tblspace
+ tp_1_2    | 
+(3 rows)
+
+SELECT tablename, indexname, tablespace FROM pg_indexes
+  WHERE tablename IN ('t', 'tp_0_1', 'tp_1_2') AND schemaname = 'partition_split_schema'
+  ORDER BY tablename, indexname, tablespace;
+ tablename |  indexname  | tablespace 
+-----------+-------------+------------
+ t         | t_pkey      | 
+ tp_0_1    | tp_0_1_pkey | 
+ tp_1_2    | tp_1_2_pkey | 
+(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 bede819af9..741c9696ec 100644
--- a/src/test/regress/sql/partition_merge.sql
+++ b/src/test/regress/sql/partition_merge.sql
@@ -551,6 +551,20 @@ SELECT tablename, indexname, tablespace FROM pg_indexes
   ORDER BY tablename, indexname, tablespace;
 DROP TABLE t;
 
+-- Check the merged partition can be set to a different tablespace
+SET search_path = partitions_merge_schema, public;
+CREATE TABLE t (i int PRIMARY KEY) PARTITION BY RANGE (i);
+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 TABLESPACE regress_tblspace;
+SELECT tablename, tablespace FROM pg_tables
+  WHERE tablename IN ('t', 'tp_0_2') AND schemaname = 'partitions_merge_schema'
+  ORDER BY tablename, tablespace;
+SELECT tablename, indexname, tablespace FROM pg_indexes
+  WHERE tablename IN ('t', 'tp_0_2') AND schemaname = 'partitions_merge_schema'
+  ORDER BY tablename, indexname, 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 ef5ea07f74..aaaee1d876 100644
--- a/src/test/regress/sql/partition_split.sql
+++ b/src/test/regress/sql/partition_split.sql
@@ -895,6 +895,20 @@ SELECT tablename, indexname, tablespace FROM pg_indexes
   ORDER BY tablename, indexname, tablespace;
 DROP TABLE t;
 
+-- Check the split partitions can be set to a different tablespace
+CREATE TABLE t (i int PRIMARY KEY) PARTITION BY RANGE (i);
+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) TABLESPACE regress_tblspace,
+   PARTITION tp_1_2 FOR VALUES FROM (1) TO (2));
+SELECT tablename, tablespace FROM pg_tables
+  WHERE tablename IN ('t', 'tp_0_1', 'tp_1_2') AND schemaname = 'partition_split_schema'
+  ORDER BY tablename, tablespace;
+SELECT tablename, indexname, tablespace FROM pg_indexes
+  WHERE tablename IN ('t', 'tp_0_1', 'tp_1_2') AND schemaname = 'partition_split_schema'
+  ORDER BY tablename, indexname, 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.39.2

v2-0002-fix-stale-comments.patchapplication/octet-stream; name=v2-0002-fix-stale-comments.patchDownload

From b9f43055f24ecd80ce470024915a30ed5b0aa8ab Mon Sep 17 00:00:00 2001
From: Zhao Junwang <zhjwpku@gmail.com>
Date: Tue, 6 Aug 2024 10:08:34 +0000
Subject: [PATCH v2 2/2] fix stale comments

PartitionCmd is used by ALTER TABLE/INDEX ATTACH/DETACH/MERGE/SPLIT,
the comments do not reflect that.

Signed-off-by: Zhao Junwang <zhjwpku@gmail.com>
---
 src/include/nodes/parsenodes.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 9ccdcb9eff..f9a2a50e8d 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -950,12 +950,13 @@ typedef struct SinglePartitionSpec
 } SinglePartitionSpec;
 
 /*
- * PartitionCmd - info for ALTER TABLE/INDEX ATTACH/DETACH PARTITION commands
+ * PartitionCmd - info for ALTER TABLE/INDEX ATTACH/DETACH/MERGE/SPLIT PARTITION commands
  */
 typedef struct PartitionCmd
 {
 	NodeTag		type;
-	RangeVar   *name;			/* name of partition to attach/detach */
+	RangeVar   *name;			/* name of partition to
+								 * attach/detach/merge/split */
 	PartitionBoundSpec *bound;	/* FOR VALUES, if attaching */
 	List	   *partlist;		/* list of partitions, for MERGE/SPLIT
 								 * PARTITION command */
-- 
2.39.2

Fujii Masao

masao.fujii@oss.nttdata.com

over 1 year ago

In reply to: Junwang Zhao (#5)

Re: Support specify tablespace for each merged/split partition

On 2024/08/06 19:28, Junwang Zhao wrote:

Attached v2 addressed all the problems you mentioned, thanks.

Thanks for updating the patches!

In the ALTER TABLE documentation, v1 patch updated the syntax, but the descriptions for MERGE and SPLIT should also be updated to explain the tablespace of new partitions.

+ char *tablespacename; /* name of tablespace, or NULL for default */
PartitionBoundSpec *bound; /* FOR VALUES, if attaching */

This is not the fault of v1 patch, but the comment "if attaching" seems incorrect.

I also noticed the comment for SinglePartitionSpec refers to a different struct name, PartitionDesc. This should be corrected, and it might be better to include this fix in the v2 patch.

- * PartitionCmd - info for ALTER TABLE/INDEX ATTACH/DETACH PARTITION commands
+ * PartitionCmd - info for ALTER TABLE/INDEX ATTACH/DETACH/MERGE/SPLIT PARTITION commands

How about changing it to "info for ALTER TABLE ATTACH/DETACH/MERGE/SPLIT and ALTER INDEX ATTACH commands" for more precision?

-	RangeVar   *name;			/* name of partition to attach/detach */
+	RangeVar   *name;			/* name of partition to
+								 * attach/detach/merge/split */

In the case of MERGE, it refers to the name of the partition that the command merges into. So, would "merge into" be more appropriate than just "merge" in this comment?

Regards,

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

Junwang Zhao

zhjwpku@gmail.com

over 1 year ago

In reply to: Fujii Masao (#6)

2 attachment(s)

Re: Support specify tablespace for each merged/split partition

Hi Fujii,

Thanks for your review.

On Wed, Aug 7, 2024 at 9:54 PM Fujii Masao <masao.fujii@oss.nttdata.com> wrote:

On 2024/08/06 19:28, Junwang Zhao wrote:

Attached v2 addressed all the problems you mentioned, thanks.

Thanks for updating the patches!

In the ALTER TABLE documentation, v1 patch updated the syntax, but the descriptions for MERGE and SPLIT should also be updated to explain the tablespace of new partitions.

Updated.

+ char *tablespacename; /* name of tablespace, or NULL for default */
PartitionBoundSpec *bound; /* FOR VALUES, if attaching */

This is not the fault of v1 patch, but the comment "if attaching" seems incorrect.

I checked the gram.y, bound can be DEFAULT, so I think a simple comment like
/* a partition bound specification */ may be more proper?

I also noticed the comment for SinglePartitionSpec refers to a different struct name, PartitionDesc. This should be corrected, and it might be better to include this fix in the v2 patch.

Fixed.

- * PartitionCmd - info for ALTER TABLE/INDEX ATTACH/DETACH PARTITION commands
+ * PartitionCmd - info for ALTER TABLE/INDEX ATTACH/DETACH/MERGE/SPLIT PARTITION commands
How about changing it to "info for ALTER TABLE ATTACH/DETACH/MERGE/SPLIT and ALTER INDEX ATTACH commands" for more precision?

Yeah, this is more precise, updated.

-       RangeVar   *name;                       /* name of partition to attach/detach */
+       RangeVar   *name;                       /* name of partition to
+                                                                * attach/detach/merge/split */
In the case of MERGE, it refers to the name of the partition that the command merges into. So, would "merge into" be more appropriate than just "merge" in this comment?

Agree, changing *merge* to *merge into* directly will unhappy pgindent,
so I use comma instead of slash instead.

Regards,

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

--
Regards
Junwang Zhao

Attachments:

v3-0002-fix-stale-comments.patchapplication/octet-stream; name=v3-0002-fix-stale-comments.patchDownload

From bb8e543439756bddcac19e6bf215060633b34e21 Mon Sep 17 00:00:00 2001
From: Zhao Junwang <zhjwpku@gmail.com>
Date: Tue, 6 Aug 2024 10:08:34 +0000
Subject: [PATCH v3 2/2] fix stale comments

PartitionCmd is used by ALTER TABLE/INDEX ATTACH/DETACH/MERGE/SPLIT,
the comments do not reflect that.

Signed-off-by: Zhao Junwang <zhjwpku@gmail.com>
---
 src/include/nodes/parsenodes.h | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 9ccdcb9eff..b8017b0626 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -938,7 +938,7 @@ typedef struct PartitionRangeDatum
 } PartitionRangeDatum;
 
 /*
- * PartitionDesc - info about single partition for ALTER TABLE SPLIT PARTITION command
+ * SinglePartitionSpec - info about single partition for ALTER TABLE SPLIT PARTITION command
  */
 typedef struct SinglePartitionSpec
 {
@@ -946,16 +946,17 @@ typedef struct SinglePartitionSpec
 
 	RangeVar   *name;			/* name of partition */
 	char	   *tablespacename; /* name of tablespace, or NULL for default */
-	PartitionBoundSpec *bound;	/* FOR VALUES, if attaching */
+	PartitionBoundSpec *bound;	/* a partition bound specification */
 } SinglePartitionSpec;
 
 /*
- * PartitionCmd - info for ALTER TABLE/INDEX ATTACH/DETACH PARTITION commands
+ * PartitionCmd - info for ALTER TABLE ATTACH/DETACH/MERGE/SPLIT and ALTER INDEX ATTACH commands
  */
 typedef struct PartitionCmd
 {
 	NodeTag		type;
-	RangeVar   *name;			/* name of partition to attach/detach */
+	RangeVar   *name;			/* name of partition to attach, detach, merge
+								 * into or split */
 	PartitionBoundSpec *bound;	/* FOR VALUES, if attaching */
 	List	   *partlist;		/* list of partitions, for MERGE/SPLIT
 								 * PARTITION command */
-- 
2.39.2

v3-0001-support-specify-tablespace-for-each-merged-split.patchapplication/octet-stream; name=v3-0001-support-specify-tablespace-for-each-merged-split.patchDownload

From 389a93c2fa7b65f0bd63166f46238e8cb0bf1b5a Mon Sep 17 00:00:00 2001
From: Zhao Junwang <zhjwpku@gmail.com>
Date: Fri, 12 Jul 2024 10:04:29 +0000
Subject: [PATCH v3 1/2] support specify tablespace for each merged/split

It might be a good idea that we can specify the tablespace
for each merged/split partition.

Signed-off-by: Zhao Junwang <zhjwpku@gmail.com>
---
 doc/src/sgml/ref/alter_table.sgml             | 27 ++++++++++++-------
 src/backend/commands/tablecmds.c              | 16 ++++++-----
 src/backend/parser/gram.y                     |  6 +++--
 src/include/nodes/parsenodes.h                |  2 ++
 src/test/regress/expected/partition_merge.out | 25 +++++++++++++++++
 src/test/regress/expected/partition_split.out | 27 +++++++++++++++++++
 src/test/regress/sql/partition_merge.sql      | 14 ++++++++++
 src/test/regress/sql/partition_split.sql      | 14 ++++++++++
 8 files changed, 113 insertions(+), 18 deletions(-)

diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml
index 6a2822adad..d3e2f7dbfb 100644
--- a/doc/src/sgml/ref/alter_table.sgml
+++ b/doc/src/sgml/ref/alter_table.sgml
@@ -39,11 +39,11 @@ ALTER TABLE [ IF EXISTS ] <replaceable class="parameter">name</replaceable>
     DETACH PARTITION <replaceable class="parameter">partition_name</replaceable> [ CONCURRENTLY | FINALIZE ]
 ALTER TABLE [ IF EXISTS ] <replaceable class="parameter">name</replaceable>
     SPLIT PARTITION <replaceable class="parameter">partition_name</replaceable> INTO
-        (PARTITION <replaceable class="parameter">partition_name1</replaceable> { FOR VALUES <replaceable class="parameter">partition_bound_spec</replaceable> | DEFAULT },
-         PARTITION <replaceable class="parameter">partition_name2</replaceable> { FOR VALUES <replaceable class="parameter">partition_bound_spec</replaceable> | DEFAULT } [, ...])
+        (PARTITION <replaceable class="parameter">partition_name1</replaceable> { FOR VALUES <replaceable class="parameter">partition_bound_spec</replaceable> | DEFAULT } [ TABLESPACE <replaceable class="parameter">tablespace_name</replaceable> ],
+         PARTITION <replaceable class="parameter">partition_name2</replaceable> { FOR VALUES <replaceable class="parameter">partition_bound_spec</replaceable> | DEFAULT } [ TABLESPACE <replaceable class="parameter">tablespace_name</replaceable> ] [, ...])
 ALTER TABLE [ IF EXISTS ] <replaceable class="parameter">name</replaceable>
     MERGE PARTITIONS (<replaceable class="parameter">partition_name1</replaceable>, <replaceable class="parameter">partition_name2</replaceable> [, ...])
-        INTO <replaceable class="parameter">partition_name</replaceable>
+        INTO <replaceable class="parameter">partition_name</replaceable> [ TABLESPACE <replaceable class="parameter">tablespace_name</replaceable> ]
 
 <phrase>where <replaceable class="parameter">action</replaceable> is one of:</phrase>
 
@@ -1125,7 +1125,7 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
    </varlistentry>
 
    <varlistentry id="sql-altertable-split-partition">
-    <term><literal>SPLIT PARTITION <replaceable class="parameter">partition_name</replaceable> INTO (PARTITION <replaceable class="parameter">partition_name1</replaceable> { FOR VALUES <replaceable class="parameter">partition_bound_spec</replaceable> | DEFAULT }, PARTITION <replaceable class="parameter">partition_name2</replaceable> { FOR VALUES <replaceable class="parameter">partition_bound_spec</replaceable> | DEFAULT } [, ...])</literal></term>
+    <term><literal>SPLIT PARTITION <replaceable class="parameter">partition_name</replaceable> INTO (PARTITION <replaceable class="parameter">partition_name1</replaceable> { FOR VALUES <replaceable class="parameter">partition_bound_spec</replaceable> | DEFAULT } [ TABLESPACE <replaceable class="parameter">tablespace_name</replaceable> ], PARTITION <replaceable class="parameter">partition_name2</replaceable> { FOR VALUES <replaceable class="parameter">partition_bound_spec</replaceable> | DEFAULT } [ TABLESPACE <replaceable class="parameter">tablespace_name</replaceable> ] [, ...])</literal></term>
 
     <listitem>
      <para>
@@ -1163,8 +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.  New partitions will also be created in
-      the same tablespace as the parent.
+      are also created temporary.  New partitions will be created in
+      the same tablespace as the parent if <literal>TABLESPACE</literal> not specified.
      </para>
      <note>
       <para>
@@ -1177,7 +1177,7 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
    </varlistentry>
 
    <varlistentry id="sql-altertable-merge-partitions">
-    <term><literal>MERGE PARTITIONS (<replaceable class="parameter">partition_name1</replaceable>, <replaceable class="parameter">partition_name2</replaceable> [, ...]) INTO <replaceable class="parameter">partition_name</replaceable></literal></term>
+    <term><literal>MERGE PARTITIONS (<replaceable class="parameter">partition_name1</replaceable>, <replaceable class="parameter">partition_name2</replaceable> [, ...]) INTO <replaceable class="parameter">partition_name</replaceable> [ TABLESPACE <replaceable class="parameter">tablespace_name</replaceable> ]</literal></term>
 
     <listitem>
      <para>
@@ -1236,8 +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.  The new partition will also be created in
-      the same tablespace as the parent.
+      is also created temporary.  The new partition will be created in the
+      same tablespace as the parent if <literal>TABLESPACE</literal> not specified.
      </para>
      <note>
       <para>
@@ -1516,6 +1516,15 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
       </listitem>
      </varlistentry>
 
+     <varlistentry id="sql-altertable-parms-tablespace-name">
+      <term><replaceable class="parameter">tablespace_name</replaceable></term>
+      <listitem>
+       <para>
+        The name of the tablespace in which the split or merged partition will be created.
+       </para>
+      </listitem>
+     </varlistentry>
+
     </variablelist>
  </refsect1>
 
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 0b2a52463f..d5f11b7601 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -20315,15 +20315,16 @@ moveSplitTableRows(Relation rel, Relation splitRel, List *partlist, List *newPar
  * Emulates command: CREATE [TEMP] TABLE <newPartName> (LIKE <modelRel's name>
  * INCLUDING ALL EXCLUDING INDEXES EXCLUDING IDENTITY EXCLUDING STATISTICS)
  *
- * Also, this function sets the new partition access method same as parent
- * table access methods (similarly to CREATE TABLE ... PARTITION OF).  It
+ * The new partition will be created in the same tablespace as the parent if not
+ * specified.  Also, this function sets the new partition access method same as
+ * parent table access methods (similarly to CREATE TABLE ... PARTITION OF).  It
  * checks that parent and child tables have compatible persistence.
  *
  * Function returns the created relation (locked in AccessExclusiveLock mode).
  */
 static Relation
-createPartitionTable(RangeVar *newPartName, Relation modelRel,
-					 AlterTableUtilityContext *context)
+createPartitionTable(RangeVar *newPartName, char *tablespacename,
+					 Relation modelRel, AlterTableUtilityContext *context)
 {
 	CreateStmt *createStmt;
 	TableLikeClause *tlc;
@@ -20347,7 +20348,8 @@ createPartitionTable(RangeVar *newPartName, Relation modelRel,
 	createStmt->constraints = NIL;
 	createStmt->options = NIL;
 	createStmt->oncommit = ONCOMMIT_NOOP;
-	createStmt->tablespacename = get_tablespace_name(modelRel->rd_rel->reltablespace);
+	createStmt->tablespacename = tablespacename ? tablespacename :
+		get_tablespace_name(modelRel->rd_rel->reltablespace);;
 	createStmt->if_not_exists = false;
 	createStmt->accessMethod = get_am_name(modelRel->rd_rel->relam);
 
@@ -20506,7 +20508,7 @@ ATExecSplitPartition(List **wqueue, AlteredTableInfo *tab, Relation rel,
 		SinglePartitionSpec *sps = (SinglePartitionSpec *) lfirst(listptr);
 		Relation	newPartRel;
 
-		newPartRel = createPartitionTable(sps->name, rel, context);
+		newPartRel = createPartitionTable(sps->name, sps->tablespacename, rel, context);
 		newPartRels = lappend(newPartRels, newPartRel);
 	}
 
@@ -20750,7 +20752,7 @@ ATExecMergePartitions(List **wqueue, AlteredTableInfo *tab, Relation rel,
 	}
 
 	/* Create table for new partition, use partitioned table as model. */
-	newPartRel = createPartitionTable(cmd->name, rel, context);
+	newPartRel = createPartitionTable(cmd->name, cmd->tablespacename, rel, context);
 
 	/* Copy data from merged partitions to new partition. */
 	moveMergedTablesRows(rel, mergingPartitionsList, newPartRel);
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index a043fd4c66..95bfe638ab 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -2317,12 +2317,13 @@ partitions_list:
 		;
 
 SinglePartitionSpec:
-			PARTITION qualified_name PartitionBoundSpec
+			PARTITION qualified_name PartitionBoundSpec OptTableSpace
 				{
 					SinglePartitionSpec *n = makeNode(SinglePartitionSpec);
 
 					n->name = $2;
 					n->bound = $3;
+					n->tablespacename = $4;
 
 					$$ = n;
 				}
@@ -2387,7 +2388,7 @@ partition_cmd:
 					$$ = (Node *) n;
 				}
 			/* ALTER TABLE <name> MERGE PARTITIONS () INTO <partition_name> */
-			| MERGE PARTITIONS '(' qualified_name_list ')' INTO qualified_name
+			| MERGE PARTITIONS '(' qualified_name_list ')' INTO qualified_name OptTableSpace
 				{
 					AlterTableCmd *n = makeNode(AlterTableCmd);
 					PartitionCmd *cmd = makeNode(PartitionCmd);
@@ -2397,6 +2398,7 @@ partition_cmd:
 					cmd->bound = NULL;
 					cmd->partlist = $4;
 					cmd->concurrent = false;
+					cmd->tablespacename = $8;
 					n->def = (Node *) cmd;
 					$$ = (Node *) n;
 				}
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 85a62b538e..9ccdcb9eff 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -945,6 +945,7 @@ typedef struct SinglePartitionSpec
 	NodeTag		type;
 
 	RangeVar   *name;			/* name of partition */
+	char	   *tablespacename; /* name of tablespace, or NULL for default */
 	PartitionBoundSpec *bound;	/* FOR VALUES, if attaching */
 } SinglePartitionSpec;
 
@@ -959,6 +960,7 @@ typedef struct PartitionCmd
 	List	   *partlist;		/* list of partitions, for MERGE/SPLIT
 								 * PARTITION command */
 	bool		concurrent;
+	char	   *tablespacename; /* name of tablespace, or NULL for default */
 } PartitionCmd;
 
 /****************************************************************************
diff --git a/src/test/regress/expected/partition_merge.out b/src/test/regress/expected/partition_merge.out
index 59836e2d35..c28836a241 100644
--- a/src/test/regress/expected/partition_merge.out
+++ b/src/test/regress/expected/partition_merge.out
@@ -886,6 +886,31 @@ SELECT tablename, indexname, tablespace FROM pg_indexes
  tp_0_2    | tp_0_2_pkey | regress_tblspace
 (2 rows)
 
+DROP TABLE t;
+-- Check the merged partition can be set to a different tablespace
+SET search_path = partitions_merge_schema, public;
+CREATE TABLE t (i int PRIMARY KEY) PARTITION BY RANGE (i);
+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 TABLESPACE regress_tblspace;
+SELECT tablename, tablespace FROM pg_tables
+  WHERE tablename IN ('t', 'tp_0_2') AND schemaname = 'partitions_merge_schema'
+  ORDER BY tablename, tablespace;
+ tablename |    tablespace    
+-----------+------------------
+ t         | 
+ tp_0_2    | regress_tblspace
+(2 rows)
+
+SELECT tablename, indexname, tablespace FROM pg_indexes
+  WHERE tablename IN ('t', 'tp_0_2') AND schemaname = 'partitions_merge_schema'
+  ORDER BY tablename, indexname, tablespace;
+ tablename |  indexname  | tablespace 
+-----------+-------------+------------
+ t         | t_pkey      | 
+ tp_0_2    | tp_0_2_pkey | 
+(2 rows)
+
 DROP TABLE t;
 -- Check the new partition inherits parent's table access method
 SET search_path = partitions_merge_schema, public;
diff --git a/src/test/regress/expected/partition_split.out b/src/test/regress/expected/partition_split.out
index dc9a5130cc..d3853e4bef 100644
--- a/src/test/regress/expected/partition_split.out
+++ b/src/test/regress/expected/partition_split.out
@@ -1521,6 +1521,33 @@ SELECT tablename, indexname, tablespace FROM pg_indexes
  tp_1_2    | tp_1_2_pkey | regress_tblspace
 (3 rows)
 
+DROP TABLE t;
+-- Check the split partitions can be set to a different tablespace
+CREATE TABLE t (i int PRIMARY KEY) PARTITION BY RANGE (i);
+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) TABLESPACE regress_tblspace,
+   PARTITION tp_1_2 FOR VALUES FROM (1) TO (2));
+SELECT tablename, tablespace FROM pg_tables
+  WHERE tablename IN ('t', 'tp_0_1', 'tp_1_2') AND schemaname = 'partition_split_schema'
+  ORDER BY tablename, tablespace;
+ tablename |    tablespace    
+-----------+------------------
+ t         | 
+ tp_0_1    | regress_tblspace
+ tp_1_2    | 
+(3 rows)
+
+SELECT tablename, indexname, tablespace FROM pg_indexes
+  WHERE tablename IN ('t', 'tp_0_1', 'tp_1_2') AND schemaname = 'partition_split_schema'
+  ORDER BY tablename, indexname, tablespace;
+ tablename |  indexname  | tablespace 
+-----------+-------------+------------
+ t         | t_pkey      | 
+ tp_0_1    | tp_0_1_pkey | 
+ tp_1_2    | tp_1_2_pkey | 
+(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 bede819af9..741c9696ec 100644
--- a/src/test/regress/sql/partition_merge.sql
+++ b/src/test/regress/sql/partition_merge.sql
@@ -551,6 +551,20 @@ SELECT tablename, indexname, tablespace FROM pg_indexes
   ORDER BY tablename, indexname, tablespace;
 DROP TABLE t;
 
+-- Check the merged partition can be set to a different tablespace
+SET search_path = partitions_merge_schema, public;
+CREATE TABLE t (i int PRIMARY KEY) PARTITION BY RANGE (i);
+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 TABLESPACE regress_tblspace;
+SELECT tablename, tablespace FROM pg_tables
+  WHERE tablename IN ('t', 'tp_0_2') AND schemaname = 'partitions_merge_schema'
+  ORDER BY tablename, tablespace;
+SELECT tablename, indexname, tablespace FROM pg_indexes
+  WHERE tablename IN ('t', 'tp_0_2') AND schemaname = 'partitions_merge_schema'
+  ORDER BY tablename, indexname, 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 ef5ea07f74..aaaee1d876 100644
--- a/src/test/regress/sql/partition_split.sql
+++ b/src/test/regress/sql/partition_split.sql
@@ -895,6 +895,20 @@ SELECT tablename, indexname, tablespace FROM pg_indexes
   ORDER BY tablename, indexname, tablespace;
 DROP TABLE t;
 
+-- Check the split partitions can be set to a different tablespace
+CREATE TABLE t (i int PRIMARY KEY) PARTITION BY RANGE (i);
+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) TABLESPACE regress_tblspace,
+   PARTITION tp_1_2 FOR VALUES FROM (1) TO (2));
+SELECT tablename, tablespace FROM pg_tables
+  WHERE tablename IN ('t', 'tp_0_1', 'tp_1_2') AND schemaname = 'partition_split_schema'
+  ORDER BY tablename, tablespace;
+SELECT tablename, indexname, tablespace FROM pg_indexes
+  WHERE tablename IN ('t', 'tp_0_1', 'tp_1_2') AND schemaname = 'partition_split_schema'
+  ORDER BY tablename, indexname, 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.39.2

Michael Paquier

michael@paquier.xyz

over 1 year ago

In reply to: Junwang Zhao (#7)

Re: Support specify tablespace for each merged/split partition

On Thu, Aug 08, 2024 at 09:47:20AM +0800, Junwang Zhao wrote:

Thanks for your review.

The SPLIT/MERGE grammar has been reverted in 3890d90c1508, so this
patch concept does not apply anymore.
--
Michael

Junwang Zhao

zhjwpku@gmail.com

over 1 year ago

In reply to: Michael Paquier (#8)

Re: Support specify tablespace for each merged/split partition

Hi Michael,

On Tue, Oct 8, 2024 at 11:21 AM Michael Paquier <michael@paquier.xyz> wrote:

On Thu, Aug 08, 2024 at 09:47:20AM +0800, Junwang Zhao wrote:

Thanks for your review.

The SPLIT/MERGE grammar has been reverted in 3890d90c1508, so this
patch concept does not apply anymore.
--
Michael

Yeah, I checked the Commit Fest entry, the status has already been
marked as rejected.

Thanks for the reminder.

--
Regards
Junwang Zhao