public inbox for [email protected]
help / color / mirror / Atom feedFrom: Alexander Korotkov <[email protected]>
To: Dmitry Koval <[email protected]>
Cc: jian he <[email protected]>
Cc: [email protected]
Subject: Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
Date: Mon, 15 Sep 2025 11:03:54 +0300
Message-ID: <CAPpHfdshkf0C2h09S00TVB8h3x8UT+GZyQ6LK2WGg1hOjnNE4g@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <171085360143.2046436.7217841141682511557.pgcf@coridan.postgresql.org>
<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>
<CACJufxE4o-m9Eu0OX33aUgfTVT3p7b3u242-CCJaOpWUaFhS=A@mail.gmail.com>
<CACJufxEH45Rtkn=n9hjakeKEkH7ou8domh+iQHMp+18Wq_L-4g@mail.gmail.com>
<[email protected]>
<CACJufxGWCVf5r9kE-z6MyR2b+wkaU15Q5m2tKz4cvBhYX3-x1g@mail.gmail.com>
<[email protected]>
Hi Dmitry.
On Mon, Sep 1, 2025 at 2:04 PM Dmitry Koval <[email protected]> wrote:
> Hi!
> Thank you for the notes and patch!
Some additional notes from me.
1) src/backend/parser/parse_utilcmd.c includes are not alphabetically
ordered here
+#include "partitioning/partdesc.h"
+#include "partitioning/partbounds.h"
2) There is unicode dash in the comment of ATExecMergePartitions() here. I
suggest we should stick to ascii.
+ /*
+ * Check ownership of merged partitions — partitions with different
+ * owners cannot be merged. Also, collect the OIDs of these partitions
+ * during the check.
+ */
3) Regarding 17bcf4f545, I see btnamecmp() is collation-aware. Should we
also specify COLLATE "C" every time we do "ORDER BY relname"?
4) This comment sounds misleading. Probably it should say "are contained".
+/*
+ * check_parent_values_in_new_partitions
+ *
+ * (function for BY LIST partitioning)
+ *
+ * Checks that all values of split partition (with Oid partOid) contains
in new
+ * partitions.
+ *
5) Given what latter items say, I think the 1. should say "The DEFAULT
partition must be at most one."
/*
* check_partitions_for_split
*
* Checks new partitions for SPLIT PARTITIONS command:
* 1. DEFAULT partition should be one.
6) Regarding the isolation tests. I see we are exercising INSERTs and
intra-partition UPDATEs. Should we also try some cross-partition UPDATEs?
------
Regards,
Alexander Korotkov
Supabase
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: <CAPpHfdshkf0C2h09S00TVB8h3x8UT+GZyQ6LK2WGg1hOjnNE4g@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