> On May 21, 2026, at 05:17, Alexander Korotkov <[email protected]> wrote:
>
> On Wed, May 20, 2026 at 11:29 PM Alexander Korotkov
> <[email protected]> wrote:
>> On Wed, May 20, 2026 at 2:46 PM Alexander Korotkov <[email protected]>
>> wrote:
>>> On Wed, May 20, 2026 at 9:29 AM Chao Li <[email protected]> wrote:
>>>>> On May 20, 2026, at 14:19, Alexander Korotkov <[email protected]>
>>>>> wrote:
>>>>> On Wed, May 20, 2026 at 2:37 AM Chao Li <[email protected]> wrote:
>>>>>>> On May 19, 2026, at 19:00, Alexander Korotkov <[email protected]>
>>>>>>> wrote:
>>>>>>> On Tue, May 19, 2026 at 5:50 AM Chao Li <[email protected]> wrote:
>>>>>>>>> On May 18, 2026, at 20:04, Alexander Korotkov <[email protected]>
>>>>>>>>> wrote:
>>>>>>>>>
>>>>>>>>> On Mon, May 18, 2026 at 2:57 PM Chao Li <[email protected]>
>>>>>>>>> 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/
