Re: Add SPLIT PARTITION/MERGE PARTITIONS commands

On Thu, Aug 21, 2025 at 10:53 AM jian he <jian(dot)universality(at)gmail(dot)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.