Re: Support specify tablespace for each merged/split partition

From: Amul Sul <sulamul(at)gmail(dot)com>
To: Junwang Zhao <zhjwpku(at)gmail(dot)com>
Cc: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Support specify tablespace for each merged/split partition
Date: 2024-08-05 12:37:41
Message-ID: CAAJ_b96qe+xNVwqXohpfjTA_1eMLKSBWfciri49ZL5++fd-k6A@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Jul 15, 2024 at 11:19 AM Junwang Zhao <zhjwpku(at)gmail(dot)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-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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Joe Conway 2024-08-05 12:38:00 Re: Cutting support for OpenSSL 1.0.1 and 1.0.2 in 17~?
Previous Message shveta malik 2024-08-05 12:35:13 Re: Logical Replication of sequences