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: Fri, 13 Jun 2025 14:29:23 +0800
Message-ID: <CACJufxE6mU7Za2X5Z4Cs9jEmgfjTYPy_ChiVXND5ZEZ3xM665g@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <171085360143.2046436.7217841141682511557.pgcf@coridan.postgresql.org>
	<[email protected]>
	<CALDaNm3V5DQSbZGxhYH+3MMMm-YdgOg+RdJ-ni6D0TZx_=Xd7w@mail.gmail.com>
	<[email protected]>
	<CAPpHfduH83+CW1dBppFVjxauPFGC+-ux+Y7wB0koZJCs029SXA@mail.gmail.com>
	<[email protected]>
	<CACJufxEi_JVg+=_BSS0o+TYUE_Hq+jmga_yxxuKLSGbQiAnZDg@mail.gmail.com>
	<[email protected]>
	<CACJufxFGPLTDW55bgaBTH1uCf0=YUZUVKWFrTBvOZNYKc49MxA@mail.gmail.com>
	<CACJufxHHnJm6Jb2YQpuRU1RX__tO=JJNJ5=EUMuzif_KNxGd9A@mail.gmail.com>
	<CACJufxFkscyhak2cSr8t+EPfwv11yccfEZBBnPbX4tRc_mWkXQ@mail.gmail.com>
	<[email protected]>
	<CACJufxHRX6xQtzn+KenK3SDR2+4kUpVnO+0axforV_pohwMnFw@mail.gmail.com>
	<[email protected]>
	<CACJufxHNvb6PpeGDEkzZMnKHjZrQGBHRUENKb2AwZ4N3Q8G3fg@mail.gmail.com>
	<[email protected]>

On Fri, Jun 13, 2025 at 4:36 AM Dmitry Koval <[email protected]> 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.


Attachments:

  [application/octet-stream] v44-0001-documentation-refactoring-based-on-v44.no-cfbot (5.9K, ../CACJufxE6mU7Za2X5Z4Cs9jEmgfjTYPy_ChiVXND5ZEZ3xM665g@mail.gmail.com/2-v44-0001-documentation-refactoring-based-on-v44.no-cfbot)
  download

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: <CACJufxE6mU7Za2X5Z4Cs9jEmgfjTYPy_ChiVXND5ZEZ3xM665g@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