Re: Add SPLIT PARTITION/MERGE PARTITIONS commands

From: Dmitry Koval <d(dot)koval(at)postgrespro(dot)ru>
To: jian he <jian(dot)universality(at)gmail(dot)com>
Cc: pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
Date: 2025-06-09 22:48:33
Message-ID: 884bcf9e-d6e3-4b0c-8dcb-7f2110070ac9@postgrespro.ru
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi, Jian He!

Thanks for the suggestions and patches!
This email contains comments to three emails (05/06/2025).
I hope to read two emails (for 06/06/2025) tomorrow.

1.
>What should we do when any to be merged partition has constraints?
>...
>Maybe this is expected, but we need to mention it somewhere and have
>some tests on it saying that MERGE PARTITIONS will effectively drop
>the partitions, so if any object depends on that partition
>then MERGE PARTITIONS can not be done.

Added following phrases to the documentation (I hope this should be
enough?):

If merged partitions have individual constraints, those constraints will
be dropped because command uses partitioned table as a model to create
the constraints.
If merged partitions have some objects dependent on them, the command
can not be done (CASCADE is not used, an error will be returned).

2.
> ... so this error check can be performed as early as the
>transformPartitionCmdForMerge stage?

Function createPartitionTable will be used for various other cases
besides MERGE PARTITIONS: for SPLIT PARTITION, for PARTITION BY
REFERENCE (I hope).
So I think it's better to minimize the amount of code and not move the
same one check into different functions (transformPartitionCmdForMerge,
transformPartitionCmdForSplit, ...).

3.
>i think, we can do the following way:
>if (modelRel->rd_rel->relam)
> elog(ERROR, "error");
>relamId = modelRel->rd_rel->relam;

Can you clarify what is reason to change the current AM-logic for
creating a new partition?

+ /* Look up the access method for new relation. */
+ relamId = (modelRel->rd_rel->relam != InvalidOid) ?
modelRel->rd_rel->relam : HEAP_TABLE_AM_OID;

(If AM is set for a partitioned table, then use it, otherwise use AM for
heap tables.)

4.
> Attached is some refactoring in moveMergedTablesRows, hope it's
straightforward.

Thanks, these changes are useful.

5.
>bug in transformPartitionCmdForMerge "equal(name, name2))"
> ...
>ALTER TABLE sales_range MERGE PARTITIONS (sales_feb2022,
>public.sales_feb2022) INTO sales_feb_mar2022;
>ERROR: lower bound of partition "sales_feb2022" conflicts with upper
>bound of previous partition "sales_feb2022"
>in this context. "sales_feb2022" is the same as "public.sales_feb2022".

Added check and test for this case.

6.
>When using ALTER TABLE ... MERGE PARTITIONS, some of the new
>partition's properties will not be inherited from to be merged
>partitions; instead, they will be directly copied from the root
>partitioned table.
>So we need to test this behavior.
>The attached test file is for test table properties:
>(COMMENTS, COMPRESSION, DEFAULTS, GENERATED, STATISTICS, STORAGE).

Some tests already exist (GENERATED, DEFAULTS) - see
partition_merge.sql, lines after:

+-- Test for:
+-- * composite partition key;
+-- * GENERATED column;
+-- * column with DEFAULT value.
...

But the complex test is probably also interesting.
Test added.

--

Similar changes are made for the second commit (SPLIT PARTITION).

--
With best regards,
Dmitry Koval

Postgres Professional: http://postgrespro.com

Attachment Content-Type Size
v42-0001-Implement-ALTER-TABLE-.-MERGE-PARTITIONS-.-comma.patch text/plain 151.2 KB
v42-0002-Implement-ALTER-TABLE-.-SPLIT-PARTITION-.-comman.patch text/plain 219.1 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Cary Huang 2025-06-09 23:04:16 Re: Support tid range scan in parallel?
Previous Message Matheus Alcantara 2025-06-09 22:24:23 Re: Batch TIDs lookup in ambulkdelete