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 CACJufxE4o-m9Eu0OX33aUgfTVT3p7b3u242-CCJaOpWUaFhS=A@mail.gmail.com
Whole thread Raw
In response to Re: Add SPLIT PARTITION/MERGE PARTITIONS commands  (jian he <jian.universality@gmail.com>)
Responses Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
List pgsql-hackers
On Thu, Aug 21, 2025 at 10:53 AM jian he <jian.universality@gmail.com> 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.

Attachment

pgsql-hackers by date:

Previous
From: Andrei Lepikhov
Date:
Subject: Re: Organize working memory under per-PlanState context
Next
From: Chao Li
Date:
Subject: Re: Fix replica identity checks for MERGE command on published table.