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: Mon, 22 Sep 2025 15:47:49 +0800
Message-ID: <CACJufxEBvrtcOWyX2NMwOCvb2ZvaRGaGeiMQpUD=yxL6ncBFOg@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <171085360143.2046436.7217841141682511557.pgcf@coridan.postgresql.org>
	<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]>
	<CAPpHfdshkf0C2h09S00TVB8h3x8UT+GZyQ6LK2WGg1hOjnNE4g@mail.gmail.com>
	<CAPpHfdsh_jPZ6-2JtWoFeZravVQYY4cYCcnKXQW6KxEx+F5vxg@mail.gmail.com>
	<[email protected]>
	<CACJufxF1Hfx1PST7pCLrKkGcjHEn2A51znXmYwVccnt5GDORcw@mail.gmail.com>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<CACJufxET9Qe-=zJtjGrKPr7-1QbVcP9y-vJU9CMEDfPGt_dbyg@mail.gmail.com>
	<[email protected]>

On Sat, Sep 20, 2025 at 4:06 AM Dmitry Koval <[email protected]> wrote:
>
> Hi, Jiah He!
>
> Thanks!
>
>  >list_intersection seems not right, how can we be sure it deals with
>  >collation correctly?
>
> list_intersection function replaced by new partitions_lists_intersection
> function.
>

hi, more about v60.

+ /*
+ * If new partition has the same name as split partition then we should
+ * rename split partition for reusing name.
+ */
+ if (isSameName)
+ {
+ /*
+ * We must bump the command counter to make the split partition tuple
+ * visible for renaming.
+ */
+ CommandCounterIncrement();
+ /* Rename partition. */
+ sprintf(tmpRelName, "split-%u-%X-tmp", RelationGetRelid(rel), MyProcPid);
+ RenameRelationInternal(splitRelOid, tmpRelName, true, false);
+
+ /*
+ * We must bump the command counter to make the split partition tuple
+ * visible after renaming.
+ */
+ CommandCounterIncrement();
+ }
duplicated CommandCounterIncrement call?


+
+/*
+ * get_list_partvalue_string
+ * A C string representation of one list partition value
+ */
+char *
+get_list_partvalue_string(Const *val)
+{
+ deparse_context context;
+ StringInfo buf = makeStringInfo();
+
+ memset(&context, 0, sizeof(deparse_context));
+ context.buf = buf;
+
+ get_const_expr(val, &context, -1);
+
+ return buf->data;
+}

+ ereport(ERROR,
+ errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+ errmsg("new partitions do not have value %s but split partition does",
+   searchNull ? "NULL" : get_list_partvalue_string(notFoundVal)));

get_list_partvalue_string seems not quite right, because it won't print Const
node collation info.  However, sometimes we do really need to print the
collation, i think.
give that we can do:
CREATE TABLE x(a text collate "C") PARTITION BY LIST (a collate
case_insensitive);

we can simply call deparse_expression. so I removed the
get_list_partvalue_string.


partitions_lists_intersection
will get all the common Const nodes in two PartitionBoundSpec->listdatums.
but that's expensive, and we only need the first common Const location
for error reporting, so I refactored it too.


I propose to change the function name check_partitions_not_overlap_list
to check_list_partitions_overlap,
what do you think?

I also heavily refactor the test again.
(arrange some error check test code together, combine several tests together,
remove/reduce unnecessary tests).
For example, the below code can be merged together to make it more readable.
SELECT attname, attidentity, attgenerated FROM pg_attribute WHERE
attnum > 0 AND attrelid = 'salespeople2_3'::regclass::oid ORDER BY
attnum;
SELECT attname, attidentity, attgenerated FROM pg_attribute WHERE
attnum > 0 AND attrelid = 'salespeople3_4'::regclass::oid ORDER BY
attnum;
SELECT attname, attidentity, attgenerated FROM pg_attribute WHERE
attnum > 0 AND attrelid = 'salespeople4_5'::regclass::oid ORDER BY
attnum;

please check the attached v60 refactor for SPLIT PARTITION.


Attachments:

  [application/octet-stream] v60-0001-refactor-v60.no-cfbot (38.6K, ../CACJufxEBvrtcOWyX2NMwOCvb2ZvaRGaGeiMQpUD=yxL6ncBFOg@mail.gmail.com/2-v60-0001-refactor-v60.no-cfbot)
  download

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]
  Subject: Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
  In-Reply-To: <CACJufxEBvrtcOWyX2NMwOCvb2ZvaRGaGeiMQpUD=yxL6ncBFOg@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