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 |
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 |