Re: Add SPLIT PARTITION/MERGE PARTITIONS commands - Mailing list pgsql-hackers

From jian he
Subject Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
Date
Msg-id CACJufxHM0sD8opy2hUxXLcdY3CQOCaMfsQtJs7yF3TS2YxSqKg@mail.gmail.com
Whole thread Raw
In response to Re: Add SPLIT PARTITION/MERGE PARTITIONS commands  (Dmitry Koval <d.koval@postgrespro.ru>)
Responses Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
List pgsql-hackers
On Wed, Jun 25, 2025 at 5:28 AM Dmitry Koval <d.koval@postgrespro.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.



pgsql-hackers by date:

Previous
From: Фуканчик Сергей
Date:
Subject: [PATCH] replace float8 with int in date2isoweek() and date2isoyear()
Next
From: Greg Sabino Mullane
Date:
Subject: Re: [PATCH] Generate random dates/times in a specified range