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 |
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 |