Thread

Re: Add SPLIT PARTITION/MERGE PARTITIONS commands

Dmitry Koval <d.koval@postgrespro.ru> — 2025-06-04T19:44:26Z

Hi!
Thank you very much for review.

1.
 >you can put it into one INSERT. like
 >INSERT INTO sales_range VALUES (1,  'May',      1000, '2022-01-31'),
 >(1,  'May',      1000, '2022-01-31');
 >which can make the regress test faster.
 >(apply the logic to other places in 
src/test/regress/sql/partition_merge.sql)

Test changed.


2.
 >+ errmsg("partition of hash-partitioned table cannot be merged")));
 >This error case doesn't seem to have a related test, and adding one
 >would be great.

Added test for hash partitioned table.


3.
 >per
 >https://www.postgresql.org/docs/current/error-message-reporting.html
 >"The extra parentheses were required before PostgreSQL version 12, but
 >are now optional."
 >so now you can remove the extra parentheses.

Extra parentheses removed.


4.
 >we can make the first error message like the second one.
 >errmsg("\"%s\" is not a partition of \"%s\"....)

Error message
errmsg("relation \"%s\" is not a partition of relation \"%s\""
occurs in two more places in the code.
I think it's better to keep this error message (for consistency).


5.
 >+ errmsg("list of new partitions should contain at least two items")));
 >This also seems to have no tests.
 >adding a dummy one should be ok.

Test added.


6.
 >We added List *partlist into PartitionCmd
 >typedef struct PartitionCmd
 >we should use
 >cmd->partlist = NIL;
 >instead of
 >cmd->partlist = NULL;
 >We also need comments explaining PartitionCmd.name
 >meaning for ALTER TABLE MERGE PARTITIONS INTO?

Fixed.


7.

 >transformPartitionCmdForMerge
 >+ partOid = RangeVarGetRelid(name, NoLock, false);
 >here "NoLock" seems not right?

AccessExclusiveLock on partitioned table protects only the DEFAULT 
partition. Fixed.


P.S. Similar changes were made for the second commit with SPLIT PARTITION.

-- 
With best regards,
Dmitry Koval

Postgres Professional: http://postgrespro.com