> On May 20, 2026, at 14:19, Alexander Korotkov <[email protected]> wrote:
> 
> Hi, Chao!
> 
> 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.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/






Reply via email to