MIME-Version: 1.0
References: 
 <171085360143.2046436.7217841141682511557.pgcf@coridan.postgresql.org>
 <CACJufxHRX6xQtzn+KenK3SDR2+4kUpVnO+0axforV_pohwMnFw@mail.gmail.com>
 <1e74c9fd-830e-4544-b0de-e49ea4b04029@postgrespro.ru>
 <CACJufxHNvb6PpeGDEkzZMnKHjZrQGBHRUENKb2AwZ4N3Q8G3fg@mail.gmail.com>
 <9c1b75f1-5c40-4a86-9a61-a4b3f1d7ec60@postgrespro.ru>
 <CACJufxE6mU7Za2X5Z4Cs9jEmgfjTYPy_ChiVXND5ZEZ3xM665g@mail.gmail.com>
 <7e1c67b8-ceba-4a04-964a-853eedeb02cc@postgrespro.ru>
 <CACJufxHakPGgUHfe4uThY2u_Hzq3YOP9m4B3gjZUTUm2mbVryA@mail.gmail.com>
 <CACJufxHo0u=PzgCn88VZEW0mtB=JopchqBTESudUMH+HCyiTpg@mail.gmail.com>
 <0b3907b4-83b9-4873-9feb-2b60b29cda33@postgrespro.ru>
 <CACJufxFTiAiN_27FzAE60Hm5yQ124EJ+b1dpNDstcSdCApFzLw@mail.gmail.com>
 <4f19550b-eb41-4649-8bf5-566dc020a720@postgrespro.ru>
 <CACJufxHjpdeDtOsmz9TyGX7jFU73LBEjX=JQpRQdAvDadT0coA@mail.gmail.com>
 <df72c242-a099-4d11-b02e-8ee8474c0455@postgrespro.ru>
 <CACJufxGSwvUwu2gyZfTzv=3TJ8Zu4=Kq-GB_pg3x5_5dpwYQkA@mail.gmail.com>
 <da035c5a-bd58-45ea-938d-71857c981147@postgrespro.ru>
In-Reply-To: <da035c5a-bd58-45ea-938d-71857c981147@postgrespro.ru>
From: jian he <jian.universality@gmail.com>
Date: Sat, 12 Jul 2025 22:01:22 +0800
Message-ID: 
 <CACJufxHM0sD8opy2hUxXLcdY3CQOCaMfsQtJs7yF3TS2YxSqKg@mail.gmail.com>
Subject: Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
To: Dmitry Koval <d.koval@postgrespro.ru>
Cc: pgsql-hackers@lists.postgresql.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: 
 <https://www.postgresql.org/message-id/CACJufxHM0sD8opy2hUxXLcdY3CQOCaMfsQtJs7yF3TS2YxSqKg%40mail.gmail.com>
Precedence: bulk

On Wed, Jun 25, 2025 at 5:28=E2=80=AFAM Dmitry Koval <d.koval@postgrespro.r=
u> wrote:
>
> Hi!
> Thanks for notes!
>
hi.

+static void
+transformPartitionCmdForMerge(CreateStmtContext *cxt, PartitionCmd *partcm=
d)
+{
+
+ /* Is current partition a DEFAULT partition? */
+ defaultPartOid =3D
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 =3D (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 th=
ough.
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 !=3D RELPERSISTENCE_TEMP &&
        newRel->rd_rel->relpersistence =3D=3D 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 !=3D RELPERSISTENCE_TEMP &&
        parent_rel->rd_rel->relpersistence =3D=3D 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 see=
ms
unintuitive to me.
it can be checked right after RangeVarGetAndCheckCreationNamespace.