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, ------ Regards, Alexander Korotkov Supabase
