Re: Add SPLIT PARTITION/MERGE PARTITIONS commands

On Wed, Jun 25, 2025 at 5:28 AM Dmitry Koval <d(dot)koval(at)postgrespro(dot)ru> wrote:
>
> Hi!
> Thanks for notes!
>
hi.

+static void
+transformPartitionCmdForMerge(CreateStmtContext *cxt, PartitionCmd *partcmd)
+{
+
+ /* Is current partition a DEFAULT partition? */
+ defaultPartOid =
get_default_oid_from_partdesc(RelationGetPartitionDesc(parent, true));
+
this comment seems wrong?
it should be "does this partitioned table (parent) have a default partition?

+ 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 error message is not good, IMHO. I don't really have any good ideas though.
if we polish this message later, now we can add a comment: "FIXME
improve this error message".

in ATExecMergePartitions we have:
RangeVarGetAndCheckCreationNamespace(cmd->name, NoLock, &existingRelid);
it will call RangeVarAdjustRelationPersistence
after it, the cmd->name->relpersistence will be set properly.

so the following in createPartitionTable can be checked way earlier.
/*
* We intended to create the partition with the same persistence as the
* parent table, but we still need to recheck because that might be
* affected by the search_path. If the parent is permanent, so must be
* all of its partitions.
*/
if (parent_rel->rd_rel->relpersistence != RELPERSISTENCE_TEMP &&
newRel->rd_rel->relpersistence == RELPERSISTENCE_TEMP)
ereport(ERROR,
errcode(ERRCODE_WRONG_OBJECT_TYPE),
errmsg("cannot create a temporary relation as
partition of permanent relation \"%s\"",
RelationGetRelationName(parent_rel)));
/* Permanent rels cannot be partitions belonging to temporary parent */
if (newRel->rd_rel->relpersistence != RELPERSISTENCE_TEMP &&
parent_rel->rd_rel->relpersistence == RELPERSISTENCE_TEMP)
ereport(ERROR,
errcode(ERRCODE_WRONG_OBJECT_TYPE),
errmsg("cannot create a permanent relation as
partition of temporary relation \"%s\"",
RelationGetRelationName(parent_rel)));

after createPartitionTable->heap_create_with_catalog do the error check seems
unintuitive to me.
it can be checked right after RangeVarGetAndCheckCreationNamespace.