Re: Fix SPLIT PARTITION bound-overlap bug and other improvements - Mailing list pgsql-hackers

On Wed, May 20, 2026 at 11:29 PM Alexander Korotkov
<aekorotkov@gmail.com> wrote:
> On Wed, May 20, 2026 at 2:46 PM Alexander Korotkov <aekorotkov@gmail.com> wrote:
> > On Wed, May 20, 2026 at 9:29 AM Chao Li <li.evan.chao@gmail.com> wrote:
> > > > On May 20, 2026, at 14:19, Alexander Korotkov <aekorotkov@gmail.com> wrote:
> > > > On Wed, May 20, 2026 at 2:37 AM Chao Li <li.evan.chao@gmail.com> wrote:
> > > >>> On May 19, 2026, at 19:00, Alexander Korotkov <aekorotkov@gmail.com> wrote:
> > > >>> On Tue, May 19, 2026 at 5:50 AM Chao Li <li.evan.chao@gmail.com> wrote:
> > > >>>>> On May 18, 2026, at 20:04, Alexander Korotkov <aekorotkov@gmail.com> wrote:
> > > >>>>>
> > > >>>>> On Mon, May 18, 2026 at 2:57 PM Chao Li <li.evan.chao@gmail.com> 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
itmore 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
listlengths 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’sbound is contained within the split partition’s bound, we need to check the reverse direction as well.
Whetherthe 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