Re: Add SPLIT PARTITION/MERGE PARTITIONS commands

From: jian he <jian(dot)universality(at)gmail(dot)com>
To: Dmitry Koval <d(dot)koval(at)postgrespro(dot)ru>
Cc: pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
Date: 2025-09-22 07:47:49
Message-ID: CACJufxEBvrtcOWyX2NMwOCvb2ZvaRGaGeiMQpUD=yxL6ncBFOg@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Sat, Sep 20, 2025 at 4:06 AM Dmitry Koval <d(dot)koval(at)postgrespro(dot)ru> 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.

Attachment Content-Type Size
v60-0001-refactor-v60.no-cfbot application/octet-stream 38.6 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2025-09-22 07:57:05 Re: [PATCH] Add tests for Bitmapset
Previous Message Michael Banck 2025-09-22 07:22:25 Re: GNU/Hurd portability patches