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

Attachment: v6-0001-Reject-degenerate-SPLIT-PARTITION-with-DEFAULT-pa.patch
Description: Binary data

Reply via email to