Re: Add SPLIT PARTITION/MERGE PARTITIONS commands

On Wed, Jun 4, 2025 at 4:53 AM Dmitry Koval <d(dot)koval(at)postgrespro(dot)ru> wrote:
> Added some changes to documentation.
> Patches are attached to the email.
>

hi.
I haven't touched v39-0002 yet.
The following are reviews of v39-0001.

+CREATE INDEX sales_range_sales_date_idx ON sales_range USING btree
(sales_date);
+INSERT INTO sales_range VALUES (1, 'May', 1000, '2022-01-31');
+INSERT INTO sales_range VALUES (2, 'Smirnoff', 500, '2022-02-10');
+INSERT INTO sales_range VALUES (3, 'Ford', 2000, '2022-04-30');
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)

+ key = RelationGetPartitionKey(parent);
+ strategy = get_partition_strategy(key);
+
+ if (strategy == PARTITION_STRATEGY_HASH)
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ 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.

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.
+ ereport(ERROR,
+ errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("partition of hash-partitioned table cannot be merged"));

+ if (!partRel->rd_rel->relispartition)
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("\"%s\" is not a partition",
+ RelationGetRelationName(partRel))));
+
+ if (get_partition_parent(partRelOid, false) != RelationGetRelid(rel))
+ ereport(ERROR,
+ (errcode(ERRCODE_UNDEFINED_TABLE),
+ errmsg("relation \"%s\" is not a partition of relation \"%s\"",
+ RelationGetRelationName(partRel),
+ RelationGetRelationName(rel))));
we can make the first error message like the second one.
errmsg("\"%s\" is not a partition of \"%s\"....)

+ case AT_MergePartitions:
+ {
+ PartitionCmd *partcmd = (PartitionCmd *) cmd->def;
+
+ if (list_length(partcmd->partlist) < 2)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+ 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.

We added List *partlist into PartitionCmd
typedef struct PartitionCmd
{
NodeTag type;
RangeVar *name; /* name of partition to attach/detach */
PartitionBoundSpec *bound; /* FOR VALUES, if attaching */
List *partlist; /* list of partitions, for MERGE/SPLIT
* PARTITION command */
bool concurrent;
} PartitionCmd;
then in src/backend/parser/gram.y
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?

transformPartitionCmdForMerge
+ partOid = RangeVarGetRelid(name, NoLock, false);
here "NoLock" seems not right?
For example, after "RangeVarGetRelid(name, NoLock, false);"
before checkPartition, I drop the table test.t in another session.
i got the following error:
ALTER TABLE sales_range MERGE PARTITIONS (sales_feb2022,
sales_mar2022, test.t) INTO sales_feb_mar_apr2022;
ERROR: could not open relation with OID 18164