public inbox for [email protected]  

help / color / mirror / Atom feed

From: jian he <[email protected]>
To: Dmitry Koval <[email protected]>
Cc: [email protected]
Subject: Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
Date: Wed, 4 Jun 2025 18:45:32 +0800
Message-ID: <CACJufxEi_JVg+=_BSS0o+TYUE_Hq+jmga_yxxuKLSGbQiAnZDg@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <171085360143.2046436.7217841141682511557.pgcf@coridan.postgresql.org>
	<CA+TgmobHYix=Nn8D4RUHa6fhUVPR88KGAMq1pBfnGfOfEjRixA@mail.gmail.com>
	<CAPpHfdt_Apbzjx-q2moST3VyNq9Shx8ByGxTF-aqBs3Gaf_n-Q@mail.gmail.com>
	<CA+TgmoY0=bT_xBP8csR=MFE=FxGE2n2-me2-31jBOgEcLvW7ug@mail.gmail.com>
	<CAPpHfdtp3_H_Rp1ubVYGUnnF0fXbam7+OOG_EMnWos1x2a_3Zg@mail.gmail.com>
	<[email protected]>
	<CA+TgmoZux8hzQ0=1-Rvigm9VuojW0aHiGSzL+G_g43qyrLGiSg@mail.gmail.com>
	<[email protected]>
	<CAPpHfdtXDuBaHscJn-wada6Kaf3fxmS4k-1TTVhpS+Kx=uveyw@mail.gmail.com>
	<[email protected]>
	<CAPpHfdv2V2tsvVn5LYyrJkzmPA==vbdCEKiT8j9yaaiDTfaQtA@mail.gmail.com>
	<[email protected]>
	<CALDaNm3V5DQSbZGxhYH+3MMMm-YdgOg+RdJ-ni6D0TZx_=Xd7w@mail.gmail.com>
	<[email protected]>
	<CAPpHfduH83+CW1dBppFVjxauPFGC+-ux+Y7wB0koZJCs029SXA@mail.gmail.com>
	<[email protected]>

On Wed, Jun 4, 2025 at 4:53 AM Dmitry Koval <[email protected]> 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

view thread (178+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected]
  Subject: Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
  In-Reply-To: <CACJufxEi_JVg+=_BSS0o+TYUE_Hq+jmga_yxxuKLSGbQiAnZDg@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox