On Mon, Jul 15, 2024 at 11:19 AM Junwang Zhao <zhjw...@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] 
> https://www.postgresql.org/message-id/abaf390b-3320-40a5-8815-ef476db5c...@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


Reply via email to