Re: Add SPLIT PARTITION/MERGE PARTITIONS commands

On Fri, Jun 13, 2025 at 4:36 AM Dmitry Koval <d(dot)koval(at)postgrespro(dot)ru> wrote:
>
> Hi, Jian He!
>
> Thanks for the notes and patches (again).
> I read a part of emails, I hope to read the rest emails tomorrow.
>

hi.

in doc/src/sgml/ref/alter_table.sgml
<title>Parameters</title> section,
we also need explain
<replaceable class="parameter">partition_name1</replaceable>
and
<replaceable class="parameter">partition_name2</replaceable>
?

+ <para>
+ This form merges several partitions into the one partition of
the target table.
+ Hash-partitioning is not supported.
maybe we can change to
+ <para>
+ This form merges several partitions of the target table into a new
one partition.
+ Hash-partitioned target table is not supported

+ <para>
+ This command acquires an <literal>ACCESS EXCLUSIVE</literal> lock.
+ This is a significant limitation, which limits the usage of this
+ command with large partitioned tables under a high load.
+ </para>
would be better mentioning that the parent table and to be merged
partition will all take
<literal>ACCESS EXCLUSIVE</literal> lock.
for example, other places we have word like:

<para>
Attaching a partition acquires a
<literal>SHARE UPDATE EXCLUSIVE</literal> lock on the parent table,
in addition to the <literal>ACCESS EXCLUSIVE</literal> locks on the table
being attached and on the default partition (if any).
</para>
---------------------------------------------------------------------------------
+ <para>
+ For range- and list-partitioned tables the ranges and lists of values
+ of the merged partitions can be any.
+ </para>
we can change it to
<para>
For range-partitioned and list-partitioned tables, the partition
bounds specification can be arbitrary.
</para>

+ <para>
+ For range-partitioned tables it is necessary that the ranges
+ of the partitions <replaceable
class="parameter">partition_name1</replaceable>,
+ <replaceable class="parameter">partition_name2</replaceable>
[, ...] can
+ be merged into one range without spaces and overlaps
(otherwise an error
+ will be generated).
"without spaces and overlaps" seems not ideal, I think I found out the
perfect word for it: adjacent.
in [1], we have description like:

anyrange -|- anyrange → boolean
Are the ranges adjacent?
[1] https://www.postgresql.org/docs/current/functions-range.html
so we can change the above to

For range-partitioned tables, the ranges of the partitions partition_name1,
partition_name2, [...] must be adjacent in order to be merged. Otherwise, an
error will be raised. The resulting combined range will be the new
partition bound for the partition partition_name.

+ The new partition will have the same table access method as the parent.
+ If the parent table is persistent then the new partition is created
+ persistent. If the parent table is temporary then the new partition
+ is also created temporary. The new partition will also be created in
+ the same tablespace as the parent.
+ </para>
we can simplify the above as
" The new partition will inherit the same table access method, persistence
type, and tablespace as the parent table.
"

+ <para>
+ If merged partitions have different owners, an error will be generated.
+ The owner of the merged partitions will be the owner of the new
partition.
+ It is the user's responsibility to setup <acronym>ACL</acronym> on the
+ new partition.
+ </para>
I think we also need to mention that individual ACL on the merged
partition will be dropped.

We have function signature
static void RemoveInheritance(Relation child_rel, Relation parent_rel,
bool expect_detached)
I found that "child_rel", "parent_rel" naming improves code
readability a lot for partitioned table contexts.
so we can change
+static void
+detachPartitionTable(Relation partRel, Relation rel, Oid defaultPartOid)
to
+static void
+detachPartitionTable(Relation partRel, Relation rel, Oid defaultPartOid)

We can also rename the ATExecMergePartitions argument (Relation rel)
to (Relation parent_rel), I think it will improve code reliability.

The attached patch is mainly documentation refactoring
and renaming function detachPartitionTable argument,
it is based on v44.