public inbox for [email protected]
help / color / mirror / Atom feedFrom: jian he <[email protected]>
To: Dmitry Koval <[email protected]>
Cc: Alexander Korotkov <[email protected]>
Cc: [email protected]
Subject: Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
Date: Thu, 21 Aug 2025 14:45:12 +0800
Message-ID: <CACJufxE4o-m9Eu0OX33aUgfTVT3p7b3u242-CCJaOpWUaFhS=A@mail.gmail.com> (raw)
In-Reply-To: <CACJufxGh=L6YPr37Y4t+ZfReBeWdv8aiNEk7VoyhYyP7d6L-4w@mail.gmail.com>
References: <171085360143.2046436.7217841141682511557.pgcf@coridan.postgresql.org>
<CACJufxHakPGgUHfe4uThY2u_Hzq3YOP9m4B3gjZUTUm2mbVryA@mail.gmail.com>
<CACJufxHo0u=PzgCn88VZEW0mtB=JopchqBTESudUMH+HCyiTpg@mail.gmail.com>
<[email protected]>
<CACJufxFTiAiN_27FzAE60Hm5yQ124EJ+b1dpNDstcSdCApFzLw@mail.gmail.com>
<[email protected]>
<CACJufxHjpdeDtOsmz9TyGX7jFU73LBEjX=JQpRQdAvDadT0coA@mail.gmail.com>
<[email protected]>
<CACJufxGSwvUwu2gyZfTzv=3TJ8Zu4=Kq-GB_pg3x5_5dpwYQkA@mail.gmail.com>
<[email protected]>
<CACJufxHM0sD8opy2hUxXLcdY3CQOCaMfsQtJs7yF3TS2YxSqKg@mail.gmail.com>
<[email protected]>
<CACJufxEWtK3utKjd7yMxRTGE54dJ8B+uigqvt1ocA4Q1mQ8qiQ@mail.gmail.com>
<[email protected]>
<CAPpHfdsW5_rFLwTfkYuG42nX5RbJCVhaC2vkn8p5GsT8Lseuyw@mail.gmail.com>
<[email protected]>
<CACJufxGEa6ZxP6d5VjZ6-Rpx=+u9zEE9ioRZRqQ=d_HYY6+daQ@mail.gmail.com>
<CACJufxGh=L6YPr37Y4t+ZfReBeWdv8aiNEk7VoyhYyP7d6L-4w@mail.gmail.com>
On Thu, Aug 21, 2025 at 10:53 AM jian he <[email protected]> wrote:
>
> > this time, I only checked
> > v52-0001-Implement-ALTER-TABLE-.-MERGE-PARTITIONS-.-comma.patch
> >
hi.
we may need to change checkPartition.
+-- ERROR: "sales_apr2022" is not a table
+ALTER TABLE sales_range MERGE PARTITIONS (sales_feb2022,
sales_mar2022, sales_apr2022) INTO sales_feb_mar_apr2022;
+ERROR: "sales_apr2022" is not a table
+HINT: ALTER TABLE ... MERGE PARTITIONS can only merge partitions
don't have sub-partitions
+ERROR: "sales_apr2022" is not a table
the above error message seems not intuitive to me.
IMV, the error message pattern should be something like:
ERROR: can not merge relation \"%s\" with other partitions
DETAIL: "sales_apr2022" is not a table
HINT: ALTER TABLE ... MERGE PARTITIONS can only merge partitions
don't have sub-partition
+/*
+ * checkPartition
+ * Check whether partRelOid is a leaf partition of the parent table (rel).
+ * Partition with OID partRelOid must be locked before function call.
+ */
+static void
+checkPartition(Relation rel, Oid partRelOid)
"Partition with OID partRelOid must be locked before function call."
we can remove this sentence.
otherwise, "function call" seems confusing?
+SET search_path = pg_temp, partitions_merge_schema, public;
+
+BEGIN;
+CREATE TABLE t (i int) PARTITION BY RANGE (i);
+CREATE TABLE tp_0_1 PARTITION OF t FOR VALUES FROM (0) TO (1);
+CREATE TABLE tp_1_2 PARTITION OF t FOR VALUES FROM (1) TO (2);
+SELECT c.oid::pg_catalog.regclass, c.relpersistence FROM
pg_catalog.pg_class c WHERE c.oid = 't'::regclass;
+EXECUTE get_partition_info('{t}');
+DEALLOCATE get_partition_info;
+SET search_path = partitions_merge_schema, pg_temp, public;
+
+-- Can't merge temporary partitions into a persistent partition
+ALTER TABLE t MERGE PARTITIONS (tp_0_1, tp_1_2) INTO tp_0_2;
+ROLLBACK;
"+-- Can't merge temporary partitions into a persistent partition"
means
+ALTER TABLE t MERGE PARTITIONS (tp_0_1, tp_1_2) INTO tp_0_2;
should error out, but it didn't.
then I found out:
+/*
+ * ALTER TABLE <name> MERGE PARTITIONS <partition-list> INTO <partition-name>
+ */
+static void
+ATExecMergePartitions(List **wqueue, AlteredTableInfo *tab, Relation rel,
+ PartitionCmd *cmd, AlterTableUtilityContext *context)
+{
....
+ /*
+ * Look up existing relation by new partition name, check we have
+ * permission to create there, lock it against concurrent drop, and mark
+ * stmt->relation as RELPERSISTENCE_TEMP if a temporary namespace is
+ * selected.
+ */
+ cmd->name->relpersistence = rel->rd_rel->relpersistence;
+ RangeVarGetAndCheckCreationNamespace(cmd->name, NoLock, &existingRelid);
``cmd->name->relpersistence`` will be adjusted in
RangeVarGetAndCheckCreationNamespace->RangeVarAdjustRelationPersistence.
+ cmd->name->relpersistence = rel->rd_rel->relpersistence;
seems wrong?
comments "stmt->relation" not sure what it refers to?
attached patch did following the changes:
* remove line ``cmd->name->relpersistence = rel->rd_rel->relpersistence;``
* refactor relpersistence, temp schema related regress tests.
Attachments:
[application/octet-stream] v52-0001-refactor-relpersistence-related-issue.no-cfbot (9.1K, ../CACJufxE4o-m9Eu0OX33aUgfTVT3p7b3u242-CCJaOpWUaFhS=A@mail.gmail.com/2-v52-0001-refactor-relpersistence-related-issue.no-cfbot)
download
view thread (165+ 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], [email protected]
Subject: Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
In-Reply-To: <CACJufxE4o-m9Eu0OX33aUgfTVT3p7b3u242-CCJaOpWUaFhS=A@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