On Thu, May 21, 2026 at 2:23 AM Chao Li <[email protected]> wrote: > > > 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/
Pushed, thank you. AFAICS, it runs ok on buildfarm now. ------ Regards, Alexander Korotkov Supabase
