Thread
Commits
GET /api/v1/messages/:b64id/commits
the thread's linked commits as JSON, with link sources.
API reference →
-
Reject degenerate SPLIT PARTITION with DEFAULT partition
- e64a9ba2b4fc 19 (unreleased) landed
- d8af73010033 19 (unreleased) landed
-
Clarify SPLIT PARTITION bound requirements in docs
- 83df16f1fa5c 19 (unreleased) landed
-
Fix SPLIT PARTITION hint for DEFAULT partition bounds
- 971017c49599 19 (unreleased) landed
-
Fix SPLIT PARTITION range bound validation with DEFAULT
- 9354896920e4 19 (unreleased) landed
-
Fix SPLIT PARTITION bound-overlap bug and other improvements
Chao Li <li.evan.chao@gmail.com> — 2026-05-13T04:38:44Z
Hi, While testing ALTER TABLE ... SPLIT PARTITION, I found a bug and a few behaviors and messages that seem worth improving. 0. A bound-overlap bug I numbered this item as 0 because I found it after finishing items 1, 2, and 3. While doing a final verification before sending this email, I was surprised to find that the partitioned table ended up with two overlapping partitions. Here is a simple repro: ``` evantest=# drop table t; DROP TABLE evantest=# CREATE TABLE t (i int) PARTITION BY RANGE(i); CREATE TABLE evantest=# CREATE TABLE p0a PARTITION OF t FOR VALUES FROM (0) TO (51); CREATE TABLE evantest=# CREATE TABLE p0b PARTITION OF t FOR VALUES FROM (51) TO (100); CREATE TABLE evantest=# \d+ t; Partitioned table "public.t" Column | Type | Collation | Nullable | Default | Storage | Compression | Stats target | Description --------+---------+-----------+----------+---------+---------+-------------+--------------+------------- i | integer | | | | plain | | | Partition key: RANGE (i) Partitions: p0a FOR VALUES FROM (0) TO (51) p0b FOR VALUES FROM (51) TO (100) evantest=# ALTER TABLE t SPLIT PARTITION p0a INTO evantest-# (PARTITION p0a FOR VALUES FROM (0) TO (53), evantest(# PARTITION pdef DEFAULT); ALTER TABLE evantest=# evantest=# evantest=# \d+ t; Partitioned table "public.t" Column | Type | Collation | Nullable | Default | Storage | Compression | Stats target | Description --------+---------+-----------+----------+---------+---------+-------------+--------------+------------- i | integer | | | | plain | | | Partition key: RANGE (i) Partitions: p0a FOR VALUES FROM (0) TO (53) p0b FOR VALUES FROM (51) TO (100) pdef DEFAULT ``` As shown above, p0a and p0b now overlap. I think this is a real bug. The problem seems to be in check_partition_bounds_for_split_range(). If the only non-default replacement partition is both first and last, the function checks only the lower bound because it uses if (first) ... else .... It never checks the upper bound in that case. 1. The documentation about splitting with a DEFAULT partition is a bit unclear Since SPLIT PARTITION allows one of the new partitions to be specified as DEFAULT, I wondered whether that new DEFAULT partition means the remaining part of the split partition's bound, or the partitioned table's global DEFAULT partition. The current documentation says: ``` <para> This form splits a single partition of the target table into new partitions. Hash-partitioned target table is not supported. Only a simple, non-partitioned partition can be split. If the split partition is the <literal>DEFAULT</literal> partition, one of the new partitions must be <literal>DEFAULT</literal>. If the partitioned table does not have a <literal>DEFAULT</literal> partition, a <literal>DEFAULT</literal> partition can be defined as one of the new partitions. </para> <para> The bounds of new partitions should not overlap with those of new or existing partitions (except <replaceable class="parameter">partition_name</replaceable>). The combined bounds of new partitions <literal> <replaceable class="parameter">partition_name1</replaceable>, <replaceable class="parameter">partition_name2</replaceable>[, ...] </literal> should be equal to the bounds of the split partition <replaceable class="parameter">partition_name</replaceable>. One of the new partitions can have the same name as the split partition <replaceable class="parameter">partition_name</replaceable> (this is suitable in case of splitting the <literal>DEFAULT</literal> partition: after the split, the <literal>DEFAULT</literal> partition remains with the same name, but its partition bound changes). </para> ``` From the first paragraph, it seems that the new DEFAULT partition is a table-level default partition. However, the second paragraph says that the combined bounds of the new partitions should be equal to the bounds of the split partition, which can make it look as if the new DEFAULT partition only covers the remaining part of the split partition's bound. My tests show that the new DEFAULT partition is the partitioned table's global DEFAULT partition. So I think the documentation can be improved to make that clearer. 2. I found this hint message confusing: ``` evantest=# ALTER TABLE t SPLIT PARTITION p0a INTO (PARTITION p0a1 FOR VALUES FROM (0) TO (5), PARTITION p0a2 FOR VALUES FROM (6) to (51), PARTITION pdef DEFAULT); ERROR: upper bound of partition "p0a2" is greater than upper bound of split partition "p0a" LINE 1: ...0) TO (5), PARTITION p0a2 FOR VALUES FROM (6) to (51), PARTI... ^ HINT: ALTER TABLE ... SPLIT PARTITION require combined bounds of new partitions must exactly match the bound of the split partition. ``` In this command, one of the new explicit partition bounds exceeds the original partition bound, but the command also specifies a DEFAULT partition. In this case, the combined explicit bounds do not need to exactly match the original partition bound, they only need to stay within it. So the hint is not quite accurate for this case. 3. SPLIT PARTITION currently provides another way to add a DEFAULT partition: ``` evantest=# \d+ t; Partitioned table "public.t" Column | Type | Collation | Nullable | Default | Storage | Compression | Stats target | Description --------+---------+-----------+----------+---------+---------+-------------+--------------+------------- i | integer | | | | plain | | | Partition key: RANGE (i) Partitions: p0a FOR VALUES FROM (0) TO (50) p0b FOR VALUES FROM (51) TO (100) p200 FOR VALUES FROM (200) TO (300) evantest=# ALTER TABLE t SPLIT PARTITION p0a INTO (PARTITION p0a FOR VALUES FROM (0) TO (50), PARTITION pdef DEFAULT); ALTER TABLE ``` Here, the new p0a partition has the same bound as the original p0a partition, so no real split happens. The command effectively only adds a new DEFAULT partition. However, it still goes through the full split-partition path: creating a new partition, moving data, attaching the new partition, and dropping the old partition. Initially, I considered adding a fast path for this case so that it would only add the new DEFAULT partition. But after thinking about it more, I think it is better to reject this degenerate form instead. We already has direct ways to add a DEFAULT partition: ``` CREATE TABLE p PARTITION OF t DEFAULT; or CREATE TABLE p; ALTER TABLE t ATTACH PARTITION p DEFAULT; ``` So I do not think SPLIT PARTITION needs to become another syntax for adding a DEFAULT partition when no actual split is performed. Accepting this form could also raise another question later: if this is allowed, why does the user have to repeat the original bound at all? Why not allow something like this? ``` ALTER TABLE t SPLIT PARTITION p0a INTO (PARTITION pdef DEFAULT); ``` That seems like an awkward direction. The attached patch tries to address all these issues. Best regards, -- Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/ -
Re: Fix SPLIT PARTITION bound-overlap bug and other improvements
Kirill Reshke <reshkekirill@gmail.com> — 2026-05-13T05:07:56Z
On Wed, 13 May 2026 at 09:39, Chao Li <li.evan.chao@gmail.com> wrote: > > Hi, > > While testing ALTER TABLE ... SPLIT PARTITION, I found a bug and a few behaviors and messages that seem worth improving. > > 0. A bound-overlap bug > > I numbered this item as 0 because I found it after finishing items 1, 2, and 3. While doing a final verification before sending this email, I was surprised to find that the partitioned table ended up with two overlapping partitions. > > Here is a simple repro: > ``` > evantest=# drop table t; > DROP TABLE > evantest=# CREATE TABLE t (i int) PARTITION BY RANGE(i); > CREATE TABLE > evantest=# CREATE TABLE p0a PARTITION OF t FOR VALUES FROM (0) TO (51); > CREATE TABLE > evantest=# CREATE TABLE p0b PARTITION OF t FOR VALUES FROM (51) TO (100); > CREATE TABLE > evantest=# \d+ t; > Partitioned table "public.t" > Column | Type | Collation | Nullable | Default | Storage | Compression | Stats target | Description > --------+---------+-----------+----------+---------+---------+-------------+--------------+------------- > i | integer | | | | plain | | | > Partition key: RANGE (i) > Partitions: > p0a FOR VALUES FROM (0) TO (51) > p0b FOR VALUES FROM (51) TO (100) > > evantest=# ALTER TABLE t SPLIT PARTITION p0a INTO > evantest-# (PARTITION p0a FOR VALUES FROM (0) TO (53), > evantest(# PARTITION pdef DEFAULT); > ALTER TABLE > evantest=# > evantest=# > evantest=# \d+ t; > Partitioned table "public.t" > Column | Type | Collation | Nullable | Default | Storage | Compression | Stats target | Description > --------+---------+-----------+----------+---------+---------+-------------+--------------+------------- > i | integer | | | | plain | | | > Partition key: RANGE (i) > Partitions: > p0a FOR VALUES FROM (0) TO (53) > p0b FOR VALUES FROM (51) TO (100) > pdef DEFAULT > ``` > > As shown above, p0a and p0b now overlap. I think this is a real bug. that's 100% real issue because of planner partition pruning ``` reshke=# CREATE TABLE t (i int) PARTITION BY RANGE(i); CREATE TABLE reshke=# CREATE TABLE p0a PARTITION OF t FOR VALUES FROM (0) TO (51); CREATE TABLE reshke=# insert into t values (50); INSERT 0 1 reshke=# CREATE TABLE p0b PARTITION OF t FOR VALUES FROM (51) TO (100); CREATE TABLE reshke=# insert into t values (51); INSERT 0 1 reshke=# insert into t values (51); INSERT 0 1 reshke=# insert into t values (51); INSERT 0 1 reshke=# ALTER TABLE t SPLIT PARTITION p0a INTO(PARTITION p0a FOR VALUES FROM (0) TO (53),PARTITION pdef DEFAULT); ALTER TABLE reshke=# table t; i ---- 50 51 51 51 (4 rows) reshke=# select * from t where i = 51; i --- (0 rows) reshke=# set enable_partition_pruning to off; SET reshke=# select * from t where i = 51; i ---- 51 51 51 (3 rows) reshke=# `` -- Best regards, Kirill Reshke
-
Re: Fix SPLIT PARTITION bound-overlap bug and other improvements
Zhenwei Shang <a934172442@gmail.com> — 2026-05-13T09:32:09Z
Kirill Reshke <reshkekirill@gmail.com> 于2026年5月13日周三 13:08写道: > On Wed, 13 May 2026 at 09:39, Chao Li <li.evan.chao@gmail.com> wrote: > > > > Hi, > > > > While testing ALTER TABLE ... SPLIT PARTITION, I found a bug and a few > behaviors and messages that seem worth improving. > > > > 0. A bound-overlap bug > > > > I numbered this item as 0 because I found it after finishing items 1, 2, > and 3. While doing a final verification before sending this email, I was > surprised to find that the partitioned table ended up with two overlapping > partitions. > > > > Here is a simple repro: > > ``` > > evantest=# drop table t; > > DROP TABLE > > evantest=# CREATE TABLE t (i int) PARTITION BY RANGE(i); > > CREATE TABLE > > evantest=# CREATE TABLE p0a PARTITION OF t FOR VALUES FROM (0) TO (51); > > CREATE TABLE > > evantest=# CREATE TABLE p0b PARTITION OF t FOR VALUES FROM (51) TO (100); > > CREATE TABLE > > evantest=# \d+ t; > > Partitioned table "public.t" > > Column | Type | Collation | Nullable | Default | Storage | > Compression | Stats target | Description > > > --------+---------+-----------+----------+---------+---------+-------------+--------------+------------- > > i | integer | | | | plain | > | | > > Partition key: RANGE (i) > > Partitions: > > p0a FOR VALUES FROM (0) TO (51) > > p0b FOR VALUES FROM (51) TO (100) > > > > evantest=# ALTER TABLE t SPLIT PARTITION p0a INTO > > evantest-# (PARTITION p0a FOR VALUES FROM (0) TO (53), > > evantest(# PARTITION pdef DEFAULT); > > ALTER TABLE > > evantest=# > > evantest=# > > evantest=# \d+ t; > > Partitioned table "public.t" > > Column | Type | Collation | Nullable | Default | Storage | > Compression | Stats target | Description > > > --------+---------+-----------+----------+---------+---------+-------------+--------------+------------- > > i | integer | | | | plain | > | | > > Partition key: RANGE (i) > > Partitions: > > p0a FOR VALUES FROM (0) TO (53) > > p0b FOR VALUES FROM (51) TO (100) > > pdef DEFAULT > > ``` > > > > As shown above, p0a and p0b now overlap. I think this is a real bug. > > > that's 100% real issue because of planner partition pruning > > ``` > reshke=# CREATE TABLE t (i int) PARTITION BY RANGE(i); > CREATE TABLE > reshke=# CREATE TABLE p0a PARTITION OF t FOR VALUES FROM (0) TO (51); > CREATE TABLE > reshke=# insert into t values (50); > INSERT 0 1 > reshke=# CREATE TABLE p0b PARTITION OF t FOR VALUES FROM (51) TO (100); > CREATE TABLE > reshke=# insert into t values (51); > INSERT 0 1 > reshke=# insert into t values (51); > INSERT 0 1 > reshke=# insert into t values (51); > INSERT 0 1 > reshke=# ALTER TABLE t SPLIT PARTITION p0a INTO(PARTITION p0a FOR > VALUES FROM (0) TO (53),PARTITION pdef DEFAULT); > ALTER TABLE > reshke=# table t; > i > ---- > 50 > 51 > 51 > 51 > (4 rows) > reshke=# select * from t where i = 51; > i > --- > (0 rows) > > reshke=# set enable_partition_pruning to off; > SET > reshke=# select * from t where i = 51; > i > ---- > 51 > 51 > 51 > (3 rows) > > reshke=# > `` > > > > -- > Best regards, > Kirill Reshke > > > I agree this is a bug. I applied the patch locally and confirmed the bug is fixed with the patch. Overall, the patch looks good to me. Regards, Zhenwei Shang
-
Re: Fix SPLIT PARTITION bound-overlap bug and other improvements
Dmitry Koval <d.koval@postgrespro.ru> — 2026-05-13T20:47:13Z
Hi, Chao Li! Thank you for the bug report, test script, and fix! >> 0. A bound-overlap bug I think this fix should be applied without much discussion: ------------------------------------------------------------------------ diff --git a/src/backend/partitioning/partbounds.c b/src/backend/partitioning/partbounds.c index 9b4277a4987..8b8f90569fe 100644 --- a/src/backend/partitioning/partbounds.c +++ b/src/backend/partitioning/partbounds.c @@ -5419,7 +5419,8 @@ check_partition_bounds_for_split_range(Relation parent, "ALTER TABLE ... SPLIT PARTITION"), parser_errposition(pstate, exprLocation((Node *) datum))); } - else + + if (last) { PartitionRangeBound *split_upper; ------------------------------------------------------------------------ >> 1. The documentation about splitting with a DEFAULT partition is a bit unclear >> ... >> 2. I found this hint message confusing: >> ... Unfortunately, I cannot comment on these points; it would be good to get the opinion of people who know English well. >> 3. SPLIT PARTITION currently provides another way to add a DEFAULT partition: >> ... Agreed, this is another way to add a DEFAULT partition. But I'm not sure that this way should be disabled (using the special function check_split_partition_not_same_bound)... Maybe it's better to keep it "as is"? -- With best regards, Dmitry Koval Postgres Professional: http://postgrespro.com -
Re: Fix SPLIT PARTITION bound-overlap bug and other improvements
Chao Li <li.evan.chao@gmail.com> — 2026-05-14T06:58:38Z
> On May 14, 2026, at 04:47, Dmitry Koval <d.koval@postgrespro.ru> wrote: > > Hi, Chao Li! > > Thank you for the bug report, test script, and fix! > > >> 0. A bound-overlap bug > > I think this fix should be applied without much discussion: > ------------------------------------------------------------------------ > diff --git a/src/backend/partitioning/partbounds.c b/src/backend/partitioning/partbounds.c > index 9b4277a4987..8b8f90569fe 100644 > --- a/src/backend/partitioning/partbounds.c > +++ b/src/backend/partitioning/partbounds.c > @@ -5419,7 +5419,8 @@ check_partition_bounds_for_split_range(Relation parent, > "ALTER TABLE ... SPLIT PARTITION"), > parser_errposition(pstate, exprLocation((Node *) datum))); > } > - else > + > + if (last) > { > PartitionRangeBound *split_upper; > ------------------------------------------------------------------------ Thanks for your confirmation. > > >> 1. The documentation about splitting with a DEFAULT partition is a bit unclear > >> ... > >> 2. I found this hint message confusing: > >> ... > > Unfortunately, I cannot comment on these points; it would be good to get the opinion of people who know English well. I want to add one more point about these two changes. There is a code comment saying that when a DEFAULT partition is specified, the new partition's lower bound may be greater than the original lower bound: ``` /* * The lower bound of "spec" must equal the lower bound of the * split partition. However, if one of the new partitions is * DEFAULT, then it is ok for the new partition's lower bound to * be greater than that of the split partition. */ ``` This also indicates that the original hint message mentioning “exactly match" is wrong for the DEFAULT case. > > > >> 3. SPLIT PARTITION currently provides another way to add a DEFAULT partition: > >> ... > > Agreed, this is another way to add a DEFAULT partition. But I'm not sure that this way should be disabled (using the special function check_split_partition_not_same_bound)... > Maybe it's better to keep it "as is"? > Yeah, this may be worth more discussion. But I think we should either reject this usage or add a fast path to avoid unnecessary creation of a new partition, data movement, etc. Otherwise, it feels more like a misuse of SPLIT PARTITION rather than a useful new alternative. To make this patch easier to process, I split it into 4 commits: 0001 - Fixes the bound-overlap bug 0002 - Fix the incorrect HINT message for the DEFAULT case 0003 - Fix the incorrect description about combined bound in the SGML doc 0004 - Reject only-create-default-partition usage Best regards, -- Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/ -
Re: Fix SPLIT PARTITION bound-overlap bug and other improvements
Alexander Korotkov <aekorotkov@gmail.com> — 2026-05-17T21:45:17Z
Hi, Chao! On Thu, May 14, 2026 at 9:59 AM Chao Li <li.evan.chao@gmail.com> wrote: > To make this patch easier to process, I split it into 4 commits: > > 0001 - Fixes the bound-overlap bug > 0002 - Fix the incorrect HINT message for the DEFAULT case > 0003 - Fix the incorrect description about combined bound in the SGML doc > 0004 - Reject only-create-default-partition usage Thank you for your work. I've revised the patchset. 0002 - I've also fixed gramma of hints in other branches 0004 - In the check_split_partition_not_same_bound(), calling partition_bounds_create() and partition_bounds_equal() looks a bit heavyweight. It doesn't matter much performance-wise, but it feels like start processing from scratch while we're on quite late stage already. I've replaced that with more lightweight check. Also I removed dealing with memory context. This code implies small non-repetitive memory allocations which only lives during DDL operation, no need to wrap them with memory context as we don't do so in other places. Any objections if I commit this? ------ Regards, Alexander Korotkov Supabase
-
Re: Fix SPLIT PARTITION bound-overlap bug and other improvements
Chao Li <li.evan.chao@gmail.com> — 2026-05-17T23:14:30Z
> On May 18, 2026, at 05:45, Alexander Korotkov <aekorotkov@gmail.com> wrote: > > Hi, Chao! > > On Thu, May 14, 2026 at 9:59 AM Chao Li <li.evan.chao@gmail.com> wrote: >> To make this patch easier to process, I split it into 4 commits: >> >> 0001 - Fixes the bound-overlap bug >> 0002 - Fix the incorrect HINT message for the DEFAULT case >> 0003 - Fix the incorrect description about combined bound in the SGML doc >> 0004 - Reject only-create-default-partition usage > > Thank you for your work. I've revised the patchset. > 0002 - I've also fixed gramma of hints in other branches > 0004 - In the check_split_partition_not_same_bound(), calling > partition_bounds_create() and partition_bounds_equal() looks a bit > heavyweight. It doesn't matter much performance-wise, but it feels > like start processing from scratch while we're on quite late stage > already. I've replaced that with more lightweight check. Also I > removed dealing with memory context. This code implies small > non-repetitive memory allocations which only lives during DDL > operation, no need to wrap them with memory context as we don't do so > in other places. > > Any objections if I commit this? > > ------ > Regards, > Alexander Korotkov > Supabase Hi Alexander, Thanks for the revisions. I think you may have missed the attachments, so I cannot review the changes. Best regards, -- Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/
-
Re: Fix SPLIT PARTITION bound-overlap bug and other improvements
Alexander Korotkov <aekorotkov@gmail.com> — 2026-05-18T09:16:19Z
On Mon, May 18, 2026 at 2:15 AM Chao Li <li.evan.chao@gmail.com> wrote: > > On May 18, 2026, at 05:45, Alexander Korotkov <aekorotkov@gmail.com> wrote: > > > > Hi, Chao! > > > > On Thu, May 14, 2026 at 9:59 AM Chao Li <li.evan.chao@gmail.com> wrote: > >> To make this patch easier to process, I split it into 4 commits: > >> > >> 0001 - Fixes the bound-overlap bug > >> 0002 - Fix the incorrect HINT message for the DEFAULT case > >> 0003 - Fix the incorrect description about combined bound in the SGML doc > >> 0004 - Reject only-create-default-partition usage > > > > Thank you for your work. I've revised the patchset. > > 0002 - I've also fixed gramma of hints in other branches > > 0004 - In the check_split_partition_not_same_bound(), calling > > partition_bounds_create() and partition_bounds_equal() looks a bit > > heavyweight. It doesn't matter much performance-wise, but it feels > > like start processing from scratch while we're on quite late stage > > already. I've replaced that with more lightweight check. Also I > > removed dealing with memory context. This code implies small > > non-repetitive memory allocations which only lives during DDL > > operation, no need to wrap them with memory context as we don't do so > > in other places. > > > > Any objections if I commit this? > > > > ------ > > Regards, > > Alexander Korotkov > > Supabase > > Hi Alexander, > > Thanks for the revisions. I think you may have missed the attachments, so I cannot review the changes. Sorry. Here it is. ------ Regards, Alexander Korotkov Supabase
-
Re: Fix SPLIT PARTITION bound-overlap bug and other improvements
Chao Li <li.evan.chao@gmail.com> — 2026-05-18T11:56:55Z
> On May 18, 2026, at 17:16, Alexander Korotkov <aekorotkov@gmail.com> wrote: > > On Mon, May 18, 2026 at 2:15 AM Chao Li <li.evan.chao@gmail.com> wrote: >>> On May 18, 2026, at 05:45, Alexander Korotkov <aekorotkov@gmail.com> wrote: >>> >>> Hi, Chao! >>> >>> On Thu, May 14, 2026 at 9:59 AM Chao Li <li.evan.chao@gmail.com> wrote: >>>> To make this patch easier to process, I split it into 4 commits: >>>> >>>> 0001 - Fixes the bound-overlap bug >>>> 0002 - Fix the incorrect HINT message for the DEFAULT case >>>> 0003 - Fix the incorrect description about combined bound in the SGML doc >>>> 0004 - Reject only-create-default-partition usage >>> >>> Thank you for your work. I've revised the patchset. >>> 0002 - I've also fixed gramma of hints in other branches >>> 0004 - In the check_split_partition_not_same_bound(), calling >>> partition_bounds_create() and partition_bounds_equal() looks a bit >>> heavyweight. It doesn't matter much performance-wise, but it feels >>> like start processing from scratch while we're on quite late stage >>> already. I've replaced that with more lightweight check. Also I >>> removed dealing with memory context. This code implies small >>> non-repetitive memory allocations which only lives during DDL >>> operation, no need to wrap them with memory context as we don't do so >>> in other places. >>> >>> Any objections if I commit this? >>> >>> ------ >>> Regards, >>> Alexander Korotkov >>> Supabase >> >> Hi Alexander, >> >> Thanks for the revisions. I think you may have missed the attachments, so I cannot review the changes. > > Sorry. Here it is. > > ------ > Regards, > Alexander Korotkov > Supabase > <v3-0003-Clarify-SPLIT-PARTITION-bound-requirements-in-doc.patch><v3-0001-Fix-SPLIT-PARTITION-range-bound-validation-with-D.patch><v3-0002-Fix-SPLIT-PARTITION-hint-for-DEFAULT-partition-bo.patch><v3-0004-Reject-degenerate-SPLIT-PARTITION-with-DEFAULT-pa.patch> v3-0001 through v3-0003 look good to me. For v3-0004, I have a suspicion, but it's late here and my brain is getting slow, so I would like to study it more tomorrow. Best regards, -- Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/
-
Re: Fix SPLIT PARTITION bound-overlap bug and other improvements
Alexander Korotkov <aekorotkov@gmail.com> — 2026-05-18T12:04:22Z
On Mon, May 18, 2026 at 2:57 PM Chao Li <li.evan.chao@gmail.com> wrote: > > <v3-0003-Clarify-SPLIT-PARTITION-bound-requirements-in-doc.patch><v3-0001-Fix-SPLIT-PARTITION-range-bound-validation-with-D.patch><v3-0002-Fix-SPLIT-PARTITION-hint-for-DEFAULT-partition-bo.patch><v3-0004-Reject-degenerate-SPLIT-PARTITION-with-DEFAULT-pa.patch> > > v3-0001 through v3-0003 look good to me. > > For v3-0004, I have a suspicion, but it's late here and my brain is getting slow, so I would like to study it more tomorrow. Sure, take your time. ------ Regards, Alexander Korotkov Supabase
-
Re: Fix SPLIT PARTITION bound-overlap bug and other improvements
Chao Li <li.evan.chao@gmail.com> — 2026-05-19T02:50:03Z
> On May 18, 2026, at 20:04, Alexander Korotkov <aekorotkov@gmail.com> wrote: > > On Mon, May 18, 2026 at 2:57 PM Chao Li <li.evan.chao@gmail.com> wrote: >>> <v3-0003-Clarify-SPLIT-PARTITION-bound-requirements-in-doc.patch><v3-0001-Fix-SPLIT-PARTITION-range-bound-validation-with-D.patch><v3-0002-Fix-SPLIT-PARTITION-hint-for-DEFAULT-partition-bo.patch><v3-0004-Reject-degenerate-SPLIT-PARTITION-with-DEFAULT-pa.patch> >> >> v3-0001 through v3-0003 look good to me. >> >> For v3-0004, I have a suspicion, but it's late here and my brain is getting slow, so I would like to study it more tomorrow. > > Sure, take your time. > > ------ > Regards, > Alexander Korotkov > Supabase My suspicion was that check_split_partition_not_same_bound() now has two paths. The RANGE path honors collation, while the LIST path does not. So I spent some time creating a test that uses a case-insensitive collation: ``` evantest=# create collation case_insensitive (provider=icu, locale='und-u-ks-level2', deterministic = false); CREATE COLLATION evantest=# create table t (b text collate case_insensitive) partition by list (b); CREATE TABLE evantest=# create table tp_ab partition of t for values in ('a', 'b'); CREATE TABLE evantest=# alter table t split partition tp_ab into evantest-# (partition tp_a for values in ('a', 'A'), evantest(# partition tp_default default); ERROR: cannot split partition "tp_ab" only to add a DEFAULT partition LINE 2: (partition tp_a for values in ('a', 'A'), ^ DETAIL: The non-DEFAULT partition would keep the same partition bound. HINT: Use CREATE TABLE ... PARTITION OF ... DEFAULT to add a DEFAULT partition. ``` In this test, the split partition’s bound is ('a', 'b'), and the new partition’s bound is ('a', 'A'). Their list lengths are both 2, but the two bounds are actually different, because 'a' and 'A' are considered equal by the collation. So, in the LIST path, since check_partition_bounds_for_split_list() has already ensured that the new partition’s bound is contained within the split partition’s bound, we need to check the reverse direction as well. Whether the split partition’s bound is also contained in the new partition’s bound. If yes, the two bounds are identical. See the attached v4 for my changes for 0004. 0001-0003 are unchanged. Since 0001 and 0003 are independent of 0004, maybe they can be pushed first. Best regards, -- Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/ -
Re: Fix SPLIT PARTITION bound-overlap bug and other improvements
Alexander Korotkov <aekorotkov@gmail.com> — 2026-05-19T11:00:52Z
Hi, Chao! On Tue, May 19, 2026 at 5:50 AM Chao Li <li.evan.chao@gmail.com> wrote: > > On May 18, 2026, at 20:04, Alexander Korotkov <aekorotkov@gmail.com> wrote: > > > > On Mon, May 18, 2026 at 2:57 PM Chao Li <li.evan.chao@gmail.com> wrote: > >>> <v3-0003-Clarify-SPLIT-PARTITION-bound-requirements-in-doc.patch><v3-0001-Fix-SPLIT-PARTITION-range-bound-validation-with-D.patch><v3-0002-Fix-SPLIT-PARTITION-hint-for-DEFAULT-partition-bo.patch><v3-0004-Reject-degenerate-SPLIT-PARTITION-with-DEFAULT-pa.patch> > >> > >> v3-0001 through v3-0003 look good to me. > >> > >> For v3-0004, I have a suspicion, but it's late here and my brain is getting slow, so I would like to study it more tomorrow. > > > > Sure, take your time. > > > > ------ > > Regards, > > Alexander Korotkov > > Supabase > > My suspicion was that check_split_partition_not_same_bound() now has two paths. The RANGE path honors collation, while the LIST path does not. So I spent some time creating a test that uses a case-insensitive collation: > ``` > evantest=# create collation case_insensitive (provider=icu, locale='und-u-ks-level2', deterministic = false); > CREATE COLLATION > evantest=# create table t (b text collate case_insensitive) partition by list (b); > CREATE TABLE > evantest=# create table tp_ab partition of t for values in ('a', 'b'); > CREATE TABLE > evantest=# alter table t split partition tp_ab into > evantest-# (partition tp_a for values in ('a', 'A'), > evantest(# partition tp_default default); > ERROR: cannot split partition "tp_ab" only to add a DEFAULT partition > LINE 2: (partition tp_a for values in ('a', 'A'), > ^ > DETAIL: The non-DEFAULT partition would keep the same partition bound. > HINT: Use CREATE TABLE ... PARTITION OF ... DEFAULT to add a DEFAULT partition. > ``` > > In this test, the split partition’s bound is ('a', 'b'), and the new partition’s bound is ('a', 'A'). Their list lengths are both 2, but the two bounds are actually different, because 'a' and 'A' are considered equal by the collation. > > So, in the LIST path, since check_partition_bounds_for_split_list() has already ensured that the new partition’s bound is contained within the split partition’s bound, we need to check the reverse direction as well. Whether the split partition’s bound is also contained in the new partition’s bound. If yes, the two bounds are identical. > > See the attached v4 for my changes for 0004. 0001-0003 are unchanged. Since 0001 and 0003 are independent of 0004, maybe they can be pushed first. I've pushed 0001-0003. Thank you for discovering the collation issue in 0004. Note that original approach of using partition_bounds_equal() can't handle different collations too (as it internally uses datumIsEqual()). I've revised the remaining patch: made function header comment a bit more detailed and added additional regression tests. Please, check. ------ Regards, Alexander Korotkov Supabase -
Re: Fix SPLIT PARTITION bound-overlap bug and other improvements
Chao Li <li.evan.chao@gmail.com> — 2026-05-19T23:36:32Z
> On May 19, 2026, at 19:00, Alexander Korotkov <aekorotkov@gmail.com> wrote: > > Hi, Chao! > > On Tue, May 19, 2026 at 5:50 AM Chao Li <li.evan.chao@gmail.com> wrote: >>> On May 18, 2026, at 20:04, Alexander Korotkov <aekorotkov@gmail.com> wrote: >>> >>> On Mon, May 18, 2026 at 2:57 PM Chao Li <li.evan.chao@gmail.com> wrote: >>>>> <v3-0003-Clarify-SPLIT-PARTITION-bound-requirements-in-doc.patch><v3-0001-Fix-SPLIT-PARTITION-range-bound-validation-with-D.patch><v3-0002-Fix-SPLIT-PARTITION-hint-for-DEFAULT-partition-bo.patch><v3-0004-Reject-degenerate-SPLIT-PARTITION-with-DEFAULT-pa.patch> >>>> >>>> v3-0001 through v3-0003 look good to me. >>>> >>>> For v3-0004, I have a suspicion, but it's late here and my brain is getting slow, so I would like to study it more tomorrow. >>> >>> Sure, take your time. >>> >>> ------ >>> Regards, >>> Alexander Korotkov >>> Supabase >> >> My suspicion was that check_split_partition_not_same_bound() now has two paths. The RANGE path honors collation, while the LIST path does not. So I spent some time creating a test that uses a case-insensitive collation: >> ``` >> evantest=# create collation case_insensitive (provider=icu, locale='und-u-ks-level2', deterministic = false); >> CREATE COLLATION >> evantest=# create table t (b text collate case_insensitive) partition by list (b); >> CREATE TABLE >> evantest=# create table tp_ab partition of t for values in ('a', 'b'); >> CREATE TABLE >> evantest=# alter table t split partition tp_ab into >> evantest-# (partition tp_a for values in ('a', 'A'), >> evantest(# partition tp_default default); >> ERROR: cannot split partition "tp_ab" only to add a DEFAULT partition >> LINE 2: (partition tp_a for values in ('a', 'A'), >> ^ >> DETAIL: The non-DEFAULT partition would keep the same partition bound. >> HINT: Use CREATE TABLE ... PARTITION OF ... DEFAULT to add a DEFAULT partition. >> ``` >> >> In this test, the split partition’s bound is ('a', 'b'), and the new partition’s bound is ('a', 'A'). Their list lengths are both 2, but the two bounds are actually different, because 'a' and 'A' are considered equal by the collation. >> >> So, in the LIST path, since check_partition_bounds_for_split_list() has already ensured that the new partition’s bound is contained within the split partition’s bound, we need to check the reverse direction as well. Whether the split partition’s bound is also contained in the new partition’s bound. If yes, the two bounds are identical. >> >> See the attached v4 for my changes for 0004. 0001-0003 are unchanged. Since 0001 and 0003 are independent of 0004, maybe they can be pushed first. > > I've pushed 0001-0003. Thanks for pushing them. > Thank you for discovering the collation issue > in 0004. Note that original approach of using > partition_bounds_equal() can't handle different collations too (as it > internally uses datumIsEqual()). Yes, I realized that while reviewing v3. That’s reason I didn’t get back v2 and only worked again based on v3. > I've revised the remaining patch: > made function header comment a bit more detailed This part looks good to me. > and added additional > regression tests. Please, check. > But I don’t see any change for regression test between v4 and v5. Maybe you forgot to save your changes? Best regards, -- Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/ -
Re: Fix SPLIT PARTITION bound-overlap bug and other improvements
Alexander Korotkov <aekorotkov@gmail.com> — 2026-05-20T06:19:18Z
Hi, Chao! On Wed, May 20, 2026 at 2:37 AM Chao Li <li.evan.chao@gmail.com> wrote: > > On May 19, 2026, at 19:00, Alexander Korotkov <aekorotkov@gmail.com> wrote: > > On Tue, May 19, 2026 at 5:50 AM Chao Li <li.evan.chao@gmail.com> wrote: > >>> On May 18, 2026, at 20:04, Alexander Korotkov <aekorotkov@gmail.com> wrote: > >>> > >>> On Mon, May 18, 2026 at 2:57 PM Chao Li <li.evan.chao@gmail.com> wrote: > >>>>> <v3-0003-Clarify-SPLIT-PARTITION-bound-requirements-in-doc.patch><v3-0001-Fix-SPLIT-PARTITION-range-bound-validation-with-D.patch><v3-0002-Fix-SPLIT-PARTITION-hint-for-DEFAULT-partition-bo.patch><v3-0004-Reject-degenerate-SPLIT-PARTITION-with-DEFAULT-pa.patch> > >>>> > >>>> v3-0001 through v3-0003 look good to me. > >>>> > >>>> For v3-0004, I have a suspicion, but it's late here and my brain is getting slow, so I would like to study it more tomorrow. > >>> > >>> Sure, take your time. > >>> > >>> ------ > >>> Regards, > >>> Alexander Korotkov > >>> Supabase > >> > >> My suspicion was that check_split_partition_not_same_bound() now has two paths. The RANGE path honors collation, while the LIST path does not. So I spent some time creating a test that uses a case-insensitive collation: > >> ``` > >> evantest=# create collation case_insensitive (provider=icu, locale='und-u-ks-level2', deterministic = false); > >> CREATE COLLATION > >> evantest=# create table t (b text collate case_insensitive) partition by list (b); > >> CREATE TABLE > >> evantest=# create table tp_ab partition of t for values in ('a', 'b'); > >> CREATE TABLE > >> evantest=# alter table t split partition tp_ab into > >> evantest-# (partition tp_a for values in ('a', 'A'), > >> evantest(# partition tp_default default); > >> ERROR: cannot split partition "tp_ab" only to add a DEFAULT partition > >> LINE 2: (partition tp_a for values in ('a', 'A'), > >> ^ > >> DETAIL: The non-DEFAULT partition would keep the same partition bound. > >> HINT: Use CREATE TABLE ... PARTITION OF ... DEFAULT to add a DEFAULT partition. > >> ``` > >> > >> In this test, the split partition’s bound is ('a', 'b'), and the new partition’s bound is ('a', 'A'). Their list lengths are both 2, but the two bounds are actually different, because 'a' and 'A' are considered equal by the collation. > >> > >> So, in the LIST path, since check_partition_bounds_for_split_list() has already ensured that the new partition’s bound is contained within the split partition’s bound, we need to check the reverse direction as well. Whether the split partition’s bound is also contained in the new partition’s bound. If yes, the two bounds are identical. > >> > >> See the attached v4 for my changes for 0004. 0001-0003 are unchanged. Since 0001 and 0003 are independent of 0004, maybe they can be pushed first. > > > > I've pushed 0001-0003. > > Thanks for pushing them. > > > Thank you for discovering the collation issue > > in 0004. Note that original approach of using > > partition_bounds_equal() can't handle different collations too (as it > > internally uses datumIsEqual()). > > Yes, I realized that while reviewing v3. That’s reason I didn’t get back v2 and only worked again based on v3. > > > I've revised the remaining patch: > > made function header comment a bit more detailed > > This part looks good to me. > > > and added additional > > regression tests. Please, check. > > > > But I don’t see any change for regression test between v4 and v5. Maybe you forgot to save your changes? Sorry, I just mess up, no changes in tests. I'm going to push this if no objection. ------ Regards, Alexander Korotkov Supabase -
Re: Fix SPLIT PARTITION bound-overlap bug and other improvements
Chao Li <li.evan.chao@gmail.com> — 2026-05-20T06:28:43Z
> On May 20, 2026, at 14:19, Alexander Korotkov <aekorotkov@gmail.com> wrote: > > Hi, Chao! > > On Wed, May 20, 2026 at 2:37 AM Chao Li <li.evan.chao@gmail.com> wrote: >>> On May 19, 2026, at 19:00, Alexander Korotkov <aekorotkov@gmail.com> wrote: >>> On Tue, May 19, 2026 at 5:50 AM Chao Li <li.evan.chao@gmail.com> wrote: >>>>> On May 18, 2026, at 20:04, Alexander Korotkov <aekorotkov@gmail.com> wrote: >>>>> >>>>> On Mon, May 18, 2026 at 2:57 PM Chao Li <li.evan.chao@gmail.com> wrote: >>>>>>> <v3-0003-Clarify-SPLIT-PARTITION-bound-requirements-in-doc.patch><v3-0001-Fix-SPLIT-PARTITION-range-bound-validation-with-D.patch><v3-0002-Fix-SPLIT-PARTITION-hint-for-DEFAULT-partition-bo.patch><v3-0004-Reject-degenerate-SPLIT-PARTITION-with-DEFAULT-pa.patch> >>>>>> >>>>>> v3-0001 through v3-0003 look good to me. >>>>>> >>>>>> For v3-0004, I have a suspicion, but it's late here and my brain is getting slow, so I would like to study it more tomorrow. >>>>> >>>>> Sure, take your time. >>>>> >>>>> ------ >>>>> Regards, >>>>> Alexander Korotkov >>>>> Supabase >>>> >>>> My suspicion was that check_split_partition_not_same_bound() now has two paths. The RANGE path honors collation, while the LIST path does not. So I spent some time creating a test that uses a case-insensitive collation: >>>> ``` >>>> evantest=# create collation case_insensitive (provider=icu, locale='und-u-ks-level2', deterministic = false); >>>> CREATE COLLATION >>>> evantest=# create table t (b text collate case_insensitive) partition by list (b); >>>> CREATE TABLE >>>> evantest=# create table tp_ab partition of t for values in ('a', 'b'); >>>> CREATE TABLE >>>> evantest=# alter table t split partition tp_ab into >>>> evantest-# (partition tp_a for values in ('a', 'A'), >>>> evantest(# partition tp_default default); >>>> ERROR: cannot split partition "tp_ab" only to add a DEFAULT partition >>>> LINE 2: (partition tp_a for values in ('a', 'A'), >>>> ^ >>>> DETAIL: The non-DEFAULT partition would keep the same partition bound. >>>> HINT: Use CREATE TABLE ... PARTITION OF ... DEFAULT to add a DEFAULT partition. >>>> ``` >>>> >>>> In this test, the split partition’s bound is ('a', 'b'), and the new partition’s bound is ('a', 'A'). Their list lengths are both 2, but the two bounds are actually different, because 'a' and 'A' are considered equal by the collation. >>>> >>>> So, in the LIST path, since check_partition_bounds_for_split_list() has already ensured that the new partition’s bound is contained within the split partition’s bound, we need to check the reverse direction as well. Whether the split partition’s bound is also contained in the new partition’s bound. If yes, the two bounds are identical. >>>> >>>> See the attached v4 for my changes for 0004. 0001-0003 are unchanged. Since 0001 and 0003 are independent of 0004, maybe they can be pushed first. >>> >>> I've pushed 0001-0003. >> >> Thanks for pushing them. >> >>> Thank you for discovering the collation issue >>> in 0004. Note that original approach of using >>> partition_bounds_equal() can't handle different collations too (as it >>> internally uses datumIsEqual()). >> >> Yes, I realized that while reviewing v3. That’s reason I didn’t get back v2 and only worked again based on v3. >> >>> I've revised the remaining patch: >>> made function header comment a bit more detailed >> >> This part looks good to me. >> >>> and added additional >>> regression tests. Please, check. >>> >> >> But I don’t see any change for regression test between v4 and v5. Maybe you forgot to save your changes? > > Sorry, I just mess up, no changes in tests. > I'm going to push this if no objection. > No worries. Then v5 looks good to me. Best regards, -- Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/ -
Re: Fix SPLIT PARTITION bound-overlap bug and other improvements
Alexander Korotkov <aekorotkov@gmail.com> — 2026-05-20T11:46:30Z
On Wed, May 20, 2026 at 9:29 AM Chao Li <li.evan.chao@gmail.com> wrote: > > On May 20, 2026, at 14:19, Alexander Korotkov <aekorotkov@gmail.com> wrote: > > > > Hi, Chao! > > > > On Wed, May 20, 2026 at 2:37 AM Chao Li <li.evan.chao@gmail.com> wrote: > >>> On May 19, 2026, at 19:00, Alexander Korotkov <aekorotkov@gmail.com> wrote: > >>> On Tue, May 19, 2026 at 5:50 AM Chao Li <li.evan.chao@gmail.com> wrote: > >>>>> On May 18, 2026, at 20:04, Alexander Korotkov <aekorotkov@gmail.com> wrote: > >>>>> > >>>>> On Mon, May 18, 2026 at 2:57 PM Chao Li <li.evan.chao@gmail.com> wrote: > >>>>>>> <v3-0003-Clarify-SPLIT-PARTITION-bound-requirements-in-doc.patch><v3-0001-Fix-SPLIT-PARTITION-range-bound-validation-with-D.patch><v3-0002-Fix-SPLIT-PARTITION-hint-for-DEFAULT-partition-bo.patch><v3-0004-Reject-degenerate-SPLIT-PARTITION-with-DEFAULT-pa.patch> > >>>>>> > >>>>>> v3-0001 through v3-0003 look good to me. > >>>>>> > >>>>>> For v3-0004, I have a suspicion, but it's late here and my brain is getting slow, so I would like to study it more tomorrow. > >>>>> > >>>>> Sure, take your time. > >>>>> > >>>>> ------ > >>>>> Regards, > >>>>> Alexander Korotkov > >>>>> Supabase > >>>> > >>>> My suspicion was that check_split_partition_not_same_bound() now has two paths. The RANGE path honors collation, while the LIST path does not. So I spent some time creating a test that uses a case-insensitive collation: > >>>> ``` > >>>> evantest=# create collation case_insensitive (provider=icu, locale='und-u-ks-level2', deterministic = false); > >>>> CREATE COLLATION > >>>> evantest=# create table t (b text collate case_insensitive) partition by list (b); > >>>> CREATE TABLE > >>>> evantest=# create table tp_ab partition of t for values in ('a', 'b'); > >>>> CREATE TABLE > >>>> evantest=# alter table t split partition tp_ab into > >>>> evantest-# (partition tp_a for values in ('a', 'A'), > >>>> evantest(# partition tp_default default); > >>>> ERROR: cannot split partition "tp_ab" only to add a DEFAULT partition > >>>> LINE 2: (partition tp_a for values in ('a', 'A'), > >>>> ^ > >>>> DETAIL: The non-DEFAULT partition would keep the same partition bound. > >>>> HINT: Use CREATE TABLE ... PARTITION OF ... DEFAULT to add a DEFAULT partition. > >>>> ``` > >>>> > >>>> In this test, the split partition’s bound is ('a', 'b'), and the new partition’s bound is ('a', 'A'). Their list lengths are both 2, but the two bounds are actually different, because 'a' and 'A' are considered equal by the collation. > >>>> > >>>> So, in the LIST path, since check_partition_bounds_for_split_list() has already ensured that the new partition’s bound is contained within the split partition’s bound, we need to check the reverse direction as well. Whether the split partition’s bound is also contained in the new partition’s bound. If yes, the two bounds are identical. > >>>> > >>>> See the attached v4 for my changes for 0004. 0001-0003 are unchanged. Since 0001 and 0003 are independent of 0004, maybe they can be pushed first. > >>> > >>> I've pushed 0001-0003. > >> > >> Thanks for pushing them. > >> > >>> Thank you for discovering the collation issue > >>> in 0004. Note that original approach of using > >>> partition_bounds_equal() can't handle different collations too (as it > >>> internally uses datumIsEqual()). > >> > >> Yes, I realized that while reviewing v3. That’s reason I didn’t get back v2 and only worked again based on v3. > >> > >>> I've revised the remaining patch: > >>> made function header comment a bit more detailed > >> > >> This part looks good to me. > >> > >>> and added additional > >>> regression tests. Please, check. > >>> > >> > >> But I don’t see any change for regression test between v4 and v5. Maybe you forgot to save your changes? > > > > Sorry, I just mess up, no changes in tests. > > I'm going to push this if no objection. > > > > No worries. Then v5 looks good to me. Thank you, pushed. ------ Regards, Alexander Korotkov Supabase -
Re: Fix SPLIT PARTITION bound-overlap bug and other improvements
Alexander Korotkov <aekorotkov@gmail.com> — 2026-05-20T20:29:07Z
On Wed, May 20, 2026 at 2:46 PM Alexander Korotkov <aekorotkov@gmail.com> wrote: > On Wed, May 20, 2026 at 9:29 AM Chao Li <li.evan.chao@gmail.com> wrote: > > > On May 20, 2026, at 14:19, Alexander Korotkov <aekorotkov@gmail.com> wrote: > > > On Wed, May 20, 2026 at 2:37 AM Chao Li <li.evan.chao@gmail.com> wrote: > > >>> On May 19, 2026, at 19:00, Alexander Korotkov <aekorotkov@gmail.com> wrote: > > >>> On Tue, May 19, 2026 at 5:50 AM Chao Li <li.evan.chao@gmail.com> wrote: > > >>>>> On May 18, 2026, at 20:04, Alexander Korotkov <aekorotkov@gmail.com> wrote: > > >>>>> > > >>>>> On Mon, May 18, 2026 at 2:57 PM Chao Li <li.evan.chao@gmail.com> wrote: > > >>>>>>> <v3-0003-Clarify-SPLIT-PARTITION-bound-requirements-in-doc.patch><v3-0001-Fix-SPLIT-PARTITION-range-bound-validation-with-D.patch><v3-0002-Fix-SPLIT-PARTITION-hint-for-DEFAULT-partition-bo.patch><v3-0004-Reject-degenerate-SPLIT-PARTITION-with-DEFAULT-pa.patch> > > >>>>>> > > >>>>>> v3-0001 through v3-0003 look good to me. > > >>>>>> > > >>>>>> For v3-0004, I have a suspicion, but it's late here and my brain is getting slow, so I would like to study it more tomorrow. > > >>>>> > > >>>>> Sure, take your time. > > >>>>> > > >>>>> ------ > > >>>>> Regards, > > >>>>> Alexander Korotkov > > >>>>> Supabase > > >>>> > > >>>> My suspicion was that check_split_partition_not_same_bound() now has two paths. The RANGE path honors collation, while the LIST path does not. So I spent some time creating a test that uses a case-insensitive collation: > > >>>> ``` > > >>>> evantest=# create collation case_insensitive (provider=icu, locale='und-u-ks-level2', deterministic = false); > > >>>> CREATE COLLATION > > >>>> evantest=# create table t (b text collate case_insensitive) partition by list (b); > > >>>> CREATE TABLE > > >>>> evantest=# create table tp_ab partition of t for values in ('a', 'b'); > > >>>> CREATE TABLE > > >>>> evantest=# alter table t split partition tp_ab into > > >>>> evantest-# (partition tp_a for values in ('a', 'A'), > > >>>> evantest(# partition tp_default default); > > >>>> ERROR: cannot split partition "tp_ab" only to add a DEFAULT partition > > >>>> LINE 2: (partition tp_a for values in ('a', 'A'), > > >>>> ^ > > >>>> DETAIL: The non-DEFAULT partition would keep the same partition bound. > > >>>> HINT: Use CREATE TABLE ... PARTITION OF ... DEFAULT to add a DEFAULT partition. > > >>>> ``` > > >>>> > > >>>> In this test, the split partition’s bound is ('a', 'b'), and the new partition’s bound is ('a', 'A'). Their list lengths are both 2, but the two bounds are actually different, because 'a' and 'A' are considered equal by the collation. > > >>>> > > >>>> So, in the LIST path, since check_partition_bounds_for_split_list() has already ensured that the new partition’s bound is contained within the split partition’s bound, we need to check the reverse direction as well. Whether the split partition’s bound is also contained in the new partition’s bound. If yes, the two bounds are identical. > > >>>> > > >>>> See the attached v4 for my changes for 0004. 0001-0003 are unchanged. Since 0001 and 0003 are independent of 0004, maybe they can be pushed first. > > >>> > > >>> I've pushed 0001-0003. > > >> > > >> Thanks for pushing them. > > >> > > >>> Thank you for discovering the collation issue > > >>> in 0004. Note that original approach of using > > >>> partition_bounds_equal() can't handle different collations too (as it > > >>> internally uses datumIsEqual()). > > >> > > >> Yes, I realized that while reviewing v3. That’s reason I didn’t get back v2 and only worked again based on v3. > > >> > > >>> I've revised the remaining patch: > > >>> made function header comment a bit more detailed > > >> > > >> This part looks good to me. > > >> > > >>> and added additional > > >>> regression tests. Please, check. > > >>> > > >> > > >> But I don’t see any change for regression test between v4 and v5. Maybe you forgot to save your changes? > > > > > > Sorry, I just mess up, no changes in tests. > > > I'm going to push this if no objection. > > > > > > > No worries. Then v5 looks good to me. > > Thank you, pushed. Uhhh, most of buildfarm animals don't support locales used in our tests. I've to revert that, ------ Regards, Alexander Korotkov Supabase -
Re: Fix SPLIT PARTITION bound-overlap bug and other improvements
Alexander Korotkov <aekorotkov@gmail.com> — 2026-05-20T21:17:20Z
On Wed, May 20, 2026 at 11:29 PM Alexander Korotkov <aekorotkov@gmail.com> wrote: > On Wed, May 20, 2026 at 2:46 PM Alexander Korotkov <aekorotkov@gmail.com> wrote: > > On Wed, May 20, 2026 at 9:29 AM Chao Li <li.evan.chao@gmail.com> wrote: > > > > On May 20, 2026, at 14:19, Alexander Korotkov <aekorotkov@gmail.com> wrote: > > > > On Wed, May 20, 2026 at 2:37 AM Chao Li <li.evan.chao@gmail.com> wrote: > > > >>> On May 19, 2026, at 19:00, Alexander Korotkov <aekorotkov@gmail.com> wrote: > > > >>> On Tue, May 19, 2026 at 5:50 AM Chao Li <li.evan.chao@gmail.com> wrote: > > > >>>>> On May 18, 2026, at 20:04, Alexander Korotkov <aekorotkov@gmail.com> wrote: > > > >>>>> > > > >>>>> On Mon, May 18, 2026 at 2:57 PM Chao Li <li.evan.chao@gmail.com> wrote: > > > >>>>>>> <v3-0003-Clarify-SPLIT-PARTITION-bound-requirements-in-doc.patch><v3-0001-Fix-SPLIT-PARTITION-range-bound-validation-with-D.patch><v3-0002-Fix-SPLIT-PARTITION-hint-for-DEFAULT-partition-bo.patch><v3-0004-Reject-degenerate-SPLIT-PARTITION-with-DEFAULT-pa.patch> > > > >>>>>> > > > >>>>>> v3-0001 through v3-0003 look good to me. > > > >>>>>> > > > >>>>>> For v3-0004, I have a suspicion, but it's late here and my brain is getting slow, so I would like to study it more tomorrow. > > > >>>>> > > > >>>>> Sure, take your time. > > > >>>>> > > > >>>>> ------ > > > >>>>> Regards, > > > >>>>> Alexander Korotkov > > > >>>>> Supabase > > > >>>> > > > >>>> My suspicion was that check_split_partition_not_same_bound() now has two paths. The RANGE path honors collation, while the LIST path does not. So I spent some time creating a test that uses a case-insensitive collation: > > > >>>> ``` > > > >>>> evantest=# create collation case_insensitive (provider=icu, locale='und-u-ks-level2', deterministic = false); > > > >>>> CREATE COLLATION > > > >>>> evantest=# create table t (b text collate case_insensitive) partition by list (b); > > > >>>> CREATE TABLE > > > >>>> evantest=# create table tp_ab partition of t for values in ('a', 'b'); > > > >>>> CREATE TABLE > > > >>>> evantest=# alter table t split partition tp_ab into > > > >>>> evantest-# (partition tp_a for values in ('a', 'A'), > > > >>>> evantest(# partition tp_default default); > > > >>>> ERROR: cannot split partition "tp_ab" only to add a DEFAULT partition > > > >>>> LINE 2: (partition tp_a for values in ('a', 'A'), > > > >>>> ^ > > > >>>> DETAIL: The non-DEFAULT partition would keep the same partition bound. > > > >>>> HINT: Use CREATE TABLE ... PARTITION OF ... DEFAULT to add a DEFAULT partition. > > > >>>> ``` > > > >>>> > > > >>>> In this test, the split partition’s bound is ('a', 'b'), and the new partition’s bound is ('a', 'A'). Their list lengths are both 2, but the two bounds are actually different, because 'a' and 'A' are considered equal by the collation. > > > >>>> > > > >>>> So, in the LIST path, since check_partition_bounds_for_split_list() has already ensured that the new partition’s bound is contained within the split partition’s bound, we need to check the reverse direction as well. Whether the split partition’s bound is also contained in the new partition’s bound. If yes, the two bounds are identical. > > > >>>> > > > >>>> See the attached v4 for my changes for 0004. 0001-0003 are unchanged. Since 0001 and 0003 are independent of 0004, maybe they can be pushed first. > > > >>> > > > >>> I've pushed 0001-0003. > > > >> > > > >> Thanks for pushing them. > > > >> > > > >>> Thank you for discovering the collation issue > > > >>> in 0004. Note that original approach of using > > > >>> partition_bounds_equal() can't handle different collations too (as it > > > >>> internally uses datumIsEqual()). > > > >> > > > >> Yes, I realized that while reviewing v3. That’s reason I didn’t get back v2 and only worked again based on v3. > > > >> > > > >>> I've revised the remaining patch: > > > >>> made function header comment a bit more detailed > > > >> > > > >> This part looks good to me. > > > >> > > > >>> and added additional > > > >>> regression tests. Please, check. > > > >>> > > > >> > > > >> But I don’t see any change for regression test between v4 and v5. Maybe you forgot to save your changes? > > > > > > > > Sorry, I just mess up, no changes in tests. > > > > I'm going to push this if no objection. > > > > > > > > > > No worries. Then v5 looks good to me. > > > > Thank you, pushed. > > Uhhh, most of buildfarm animals don't support locales used in our > tests. I've to revert that, The another attempt is attached. Now use -0.0 and 0.0 as binary different but logically equivalent values, no locale dependence. ------ Regards, Alexander Korotkov Supabase -
Re: Fix SPLIT PARTITION bound-overlap bug and other improvements
Chao Li <li.evan.chao@gmail.com> — 2026-05-20T23:22:24Z
> On May 21, 2026, at 05:17, Alexander Korotkov <aekorotkov@gmail.com> wrote: > > On Wed, May 20, 2026 at 11:29 PM Alexander Korotkov > <aekorotkov@gmail.com> wrote: >> On Wed, May 20, 2026 at 2:46 PM Alexander Korotkov <aekorotkov@gmail.com> wrote: >>> On Wed, May 20, 2026 at 9:29 AM Chao Li <li.evan.chao@gmail.com> wrote: >>>>> On May 20, 2026, at 14:19, Alexander Korotkov <aekorotkov@gmail.com> wrote: >>>>> On Wed, May 20, 2026 at 2:37 AM Chao Li <li.evan.chao@gmail.com> wrote: >>>>>>> On May 19, 2026, at 19:00, Alexander Korotkov <aekorotkov@gmail.com> wrote: >>>>>>> On Tue, May 19, 2026 at 5:50 AM Chao Li <li.evan.chao@gmail.com> wrote: >>>>>>>>> On May 18, 2026, at 20:04, Alexander Korotkov <aekorotkov@gmail.com> wrote: >>>>>>>>> >>>>>>>>> On Mon, May 18, 2026 at 2:57 PM Chao Li <li.evan.chao@gmail.com> wrote: >>>>>>>>>>> <v3-0003-Clarify-SPLIT-PARTITION-bound-requirements-in-doc.patch><v3-0001-Fix-SPLIT-PARTITION-range-bound-validation-with-D.patch><v3-0002-Fix-SPLIT-PARTITION-hint-for-DEFAULT-partition-bo.patch><v3-0004-Reject-degenerate-SPLIT-PARTITION-with-DEFAULT-pa.patch> >>>>>>>>>> >>>>>>>>>> v3-0001 through v3-0003 look good to me. >>>>>>>>>> >>>>>>>>>> For v3-0004, I have a suspicion, but it's late here and my brain is getting slow, so I would like to study it more tomorrow. >>>>>>>>> >>>>>>>>> Sure, take your time. >>>>>>>>> >>>>>>>>> ------ >>>>>>>>> Regards, >>>>>>>>> Alexander Korotkov >>>>>>>>> Supabase >>>>>>>> >>>>>>>> My suspicion was that check_split_partition_not_same_bound() now has two paths. The RANGE path honors collation, while the LIST path does not. So I spent some time creating a test that uses a case-insensitive collation: >>>>>>>> ``` >>>>>>>> evantest=# create collation case_insensitive (provider=icu, locale='und-u-ks-level2', deterministic = false); >>>>>>>> CREATE COLLATION >>>>>>>> evantest=# create table t (b text collate case_insensitive) partition by list (b); >>>>>>>> CREATE TABLE >>>>>>>> evantest=# create table tp_ab partition of t for values in ('a', 'b'); >>>>>>>> CREATE TABLE >>>>>>>> evantest=# alter table t split partition tp_ab into >>>>>>>> evantest-# (partition tp_a for values in ('a', 'A'), >>>>>>>> evantest(# partition tp_default default); >>>>>>>> ERROR: cannot split partition "tp_ab" only to add a DEFAULT partition >>>>>>>> LINE 2: (partition tp_a for values in ('a', 'A'), >>>>>>>> ^ >>>>>>>> DETAIL: The non-DEFAULT partition would keep the same partition bound. >>>>>>>> HINT: Use CREATE TABLE ... PARTITION OF ... DEFAULT to add a DEFAULT partition. >>>>>>>> ``` >>>>>>>> >>>>>>>> In this test, the split partition’s bound is ('a', 'b'), and the new partition’s bound is ('a', 'A'). Their list lengths are both 2, but the two bounds are actually different, because 'a' and 'A' are considered equal by the collation. >>>>>>>> >>>>>>>> So, in the LIST path, since check_partition_bounds_for_split_list() has already ensured that the new partition’s bound is contained within the split partition’s bound, we need to check the reverse direction as well. Whether the split partition’s bound is also contained in the new partition’s bound. If yes, the two bounds are identical. >>>>>>>> >>>>>>>> See the attached v4 for my changes for 0004. 0001-0003 are unchanged. Since 0001 and 0003 are independent of 0004, maybe they can be pushed first. >>>>>>> >>>>>>> I've pushed 0001-0003. >>>>>> >>>>>> Thanks for pushing them. >>>>>> >>>>>>> Thank you for discovering the collation issue >>>>>>> in 0004. Note that original approach of using >>>>>>> partition_bounds_equal() can't handle different collations too (as it >>>>>>> internally uses datumIsEqual()). >>>>>> >>>>>> Yes, I realized that while reviewing v3. That’s reason I didn’t get back v2 and only worked again based on v3. >>>>>> >>>>>>> I've revised the remaining patch: >>>>>>> made function header comment a bit more detailed >>>>>> >>>>>> This part looks good to me. >>>>>> >>>>>>> and added additional >>>>>>> regression tests. Please, check. >>>>>>> >>>>>> >>>>>> But I don’t see any change for regression test between v4 and v5. Maybe you forgot to save your changes? >>>>> >>>>> Sorry, I just mess up, no changes in tests. >>>>> I'm going to push this if no objection. >>>>> >>>> >>>> No worries. Then v5 looks good to me. >>> >>> Thank you, pushed. >> >> Uhhh, most of buildfarm animals don't support locales used in our >> tests. I've to revert that, > > The another attempt is attached. Now use -0.0 and 0.0 as binary > different but logically equivalent values, no locale dependence. > > ------ > Regards, > Alexander Korotkov > Supabase > <v6-0001-Reject-degenerate-SPLIT-PARTITION-with-DEFAULT-pa.patch> Thank you very much for taking care of that. This was a lesson learned for me. Actually, when I added the ICU test, I did think about whether regression tests support ICU. So I searched the existing tests for “icu” and found some examples. For instance, src/test/regress/sql/collate.icu.utf8.sql has tests like: ``` RESET icu_validation_level; CREATE COLLATION testx (provider = icu, locale = '@colStrength=primary;nonsense=yes'); DROP COLLATION testx; CREATE COLLATION testx (provider = icu, locale = 'nonsense-nowhere'); DROP COLLATION testx; ``` I added v6 to CF to get a CI run and check whether the buildfarm is happy with v6. The CI test has passed now. See [1]. The new test using “-0.0" also looks good to me. [1] https://commitfest.postgresql.org/patch/6796/ Best regards, -- Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/ -
Re: Fix SPLIT PARTITION bound-overlap bug and other improvements
Alexander Korotkov <aekorotkov@gmail.com> — 2026-05-25T17:41:02Z
On Thu, May 21, 2026 at 2:23 AM Chao Li <li.evan.chao@gmail.com> wrote: > > > On May 21, 2026, at 05:17, Alexander Korotkov <aekorotkov@gmail.com> wrote: > > On Wed, May 20, 2026 at 11:29 PM Alexander Korotkov > > <aekorotkov@gmail.com> wrote: > >> On Wed, May 20, 2026 at 2:46 PM Alexander Korotkov <aekorotkov@gmail.com> wrote: > >>> On Wed, May 20, 2026 at 9:29 AM Chao Li <li.evan.chao@gmail.com> wrote: > >>>>> On May 20, 2026, at 14:19, Alexander Korotkov <aekorotkov@gmail.com> wrote: > >>>>> On Wed, May 20, 2026 at 2:37 AM Chao Li <li.evan.chao@gmail.com> wrote: > >>>>>>> On May 19, 2026, at 19:00, Alexander Korotkov <aekorotkov@gmail.com> wrote: > >>>>>>> On Tue, May 19, 2026 at 5:50 AM Chao Li <li.evan.chao@gmail.com> wrote: > >>>>>>>>> On May 18, 2026, at 20:04, Alexander Korotkov <aekorotkov@gmail.com> wrote: > >>>>>>>>> > >>>>>>>>> On Mon, May 18, 2026 at 2:57 PM Chao Li <li.evan.chao@gmail.com> wrote: > >>>>>>>>>>> <v3-0003-Clarify-SPLIT-PARTITION-bound-requirements-in-doc.patch><v3-0001-Fix-SPLIT-PARTITION-range-bound-validation-with-D.patch><v3-0002-Fix-SPLIT-PARTITION-hint-for-DEFAULT-partition-bo.patch><v3-0004-Reject-degenerate-SPLIT-PARTITION-with-DEFAULT-pa.patch> > >>>>>>>>>> > >>>>>>>>>> v3-0001 through v3-0003 look good to me. > >>>>>>>>>> > >>>>>>>>>> For v3-0004, I have a suspicion, but it's late here and my brain is getting slow, so I would like to study it more tomorrow. > >>>>>>>>> > >>>>>>>>> Sure, take your time. > >>>>>>>>> > >>>>>>>>> ------ > >>>>>>>>> Regards, > >>>>>>>>> Alexander Korotkov > >>>>>>>>> Supabase > >>>>>>>> > >>>>>>>> My suspicion was that check_split_partition_not_same_bound() now has two paths. The RANGE path honors collation, while the LIST path does not. So I spent some time creating a test that uses a case-insensitive collation: > >>>>>>>> ``` > >>>>>>>> evantest=# create collation case_insensitive (provider=icu, locale='und-u-ks-level2', deterministic = false); > >>>>>>>> CREATE COLLATION > >>>>>>>> evantest=# create table t (b text collate case_insensitive) partition by list (b); > >>>>>>>> CREATE TABLE > >>>>>>>> evantest=# create table tp_ab partition of t for values in ('a', 'b'); > >>>>>>>> CREATE TABLE > >>>>>>>> evantest=# alter table t split partition tp_ab into > >>>>>>>> evantest-# (partition tp_a for values in ('a', 'A'), > >>>>>>>> evantest(# partition tp_default default); > >>>>>>>> ERROR: cannot split partition "tp_ab" only to add a DEFAULT partition > >>>>>>>> LINE 2: (partition tp_a for values in ('a', 'A'), > >>>>>>>> ^ > >>>>>>>> DETAIL: The non-DEFAULT partition would keep the same partition bound. > >>>>>>>> HINT: Use CREATE TABLE ... PARTITION OF ... DEFAULT to add a DEFAULT partition. > >>>>>>>> ``` > >>>>>>>> > >>>>>>>> In this test, the split partition’s bound is ('a', 'b'), and the new partition’s bound is ('a', 'A'). Their list lengths are both 2, but the two bounds are actually different, because 'a' and 'A' are considered equal by the collation. > >>>>>>>> > >>>>>>>> So, in the LIST path, since check_partition_bounds_for_split_list() has already ensured that the new partition’s bound is contained within the split partition’s bound, we need to check the reverse direction as well. Whether the split partition’s bound is also contained in the new partition’s bound. If yes, the two bounds are identical. > >>>>>>>> > >>>>>>>> See the attached v4 for my changes for 0004. 0001-0003 are unchanged. Since 0001 and 0003 are independent of 0004, maybe they can be pushed first. > >>>>>>> > >>>>>>> I've pushed 0001-0003. > >>>>>> > >>>>>> Thanks for pushing them. > >>>>>> > >>>>>>> Thank you for discovering the collation issue > >>>>>>> in 0004. Note that original approach of using > >>>>>>> partition_bounds_equal() can't handle different collations too (as it > >>>>>>> internally uses datumIsEqual()). > >>>>>> > >>>>>> Yes, I realized that while reviewing v3. That’s reason I didn’t get back v2 and only worked again based on v3. > >>>>>> > >>>>>>> I've revised the remaining patch: > >>>>>>> made function header comment a bit more detailed > >>>>>> > >>>>>> This part looks good to me. > >>>>>> > >>>>>>> and added additional > >>>>>>> regression tests. Please, check. > >>>>>>> > >>>>>> > >>>>>> But I don’t see any change for regression test between v4 and v5. Maybe you forgot to save your changes? > >>>>> > >>>>> Sorry, I just mess up, no changes in tests. > >>>>> I'm going to push this if no objection. > >>>>> > >>>> > >>>> No worries. Then v5 looks good to me. > >>> > >>> Thank you, pushed. > >> > >> Uhhh, most of buildfarm animals don't support locales used in our > >> tests. I've to revert that, > > > > The another attempt is attached. Now use -0.0 and 0.0 as binary > > different but logically equivalent values, no locale dependence. > > > > ------ > > Regards, > > Alexander Korotkov > > Supabase > > <v6-0001-Reject-degenerate-SPLIT-PARTITION-with-DEFAULT-pa.patch> > > Thank you very much for taking care of that. This was a lesson learned for me. > > Actually, when I added the ICU test, I did think about whether regression tests support ICU. So I searched the existing tests for “icu” and found some examples. For instance, src/test/regress/sql/collate.icu.utf8.sql has tests like: > ``` > RESET icu_validation_level; > CREATE COLLATION testx (provider = icu, locale = '@colStrength=primary;nonsense=yes'); DROP COLLATION testx; > CREATE COLLATION testx (provider = icu, locale = 'nonsense-nowhere'); DROP COLLATION testx; > ``` > > I added v6 to CF to get a CI run and check whether the buildfarm is happy with v6. The CI test has passed now. See [1]. The new test using “-0.0" also looks good to me. > > [1] https://commitfest.postgresql.org/patch/6796/ Pushed, thank you. AFAICS, it runs ok on buildfarm now. ------ Regards, Alexander Korotkov Supabase