MIME-Version: 1.0
References: <C18878AB-DEB2-4A61-9995-A035DD644B81@gmail.com>
 <4df20e70-a083-4334-9548-5f8b9025847c@postgrespro.ru>
 <D4A592C8-76F4-41A4-9746-2FED6BED625A@gmail.com>
 <CAPpHfdvdevTMpjQe09aWdK_nbrE4+M1z09H2SUe3bgxY3_QUdA@mail.gmail.com>
 <C0F7F893-D33E-4698-938E-612D08305CF6@gmail.com>
 <CAPpHfdsLnD3NGnPC32oeg4ZWWX441M=SfH0-7bG652G40TpEEQ@mail.gmail.com>
 <4B04275C-E044-4EEE-BE64-6FEEE73DCBB0@gmail.com>
 <CAPpHfduMLJkOSguaKhm4PEWRYHqqvFDX6bu5+E_LSxuy5YjPEw@mail.gmail.com>
 <09307DC2-64A1-4D6A-9EAF-9A86A173A7FC@gmail.com>
 <CAPpHfdvn=hPrtMjtvJU-7=HR69ZWDvy2pTE9Prau-dgp5Ua0Jg@mail.gmail.com>
 <8204A871-4650-4B3F-8260-8D5FD40908B3@gmail.com>
 <CAPpHfduDPA1vqhrSgLwmGY5+wVsrmK2AKsPQ2rZa_=QAn-y_XQ@mail.gmail.com>
 <4AEA11EB-0439-401B-996E-F1835F510E8D@gmail.com>
 <CAPpHfdvqk=nC-_wdK4o5=LyWD53FvJDdH7RSk7cZuWgx-qRCQg@mail.gmail.com>
 <CAPpHfdvht=_VcpDybYvwon96Enh=-S-9wGLGPzbu+6_5QmwmwQ@mail.gmail.com>
 <CAPpHfdtRNFwVBrekh_b6Pg5fmi+QCoyEdAs7h=ikxXoDcOMdHA@mail.gmail.com>
 <BE90212C-FC2C-4FE8-AC96-983D142F33FC@gmail.com>
In-Reply-To: <BE90212C-FC2C-4FE8-AC96-983D142F33FC@gmail.com>
From: Alexander Korotkov <aekorotkov@gmail.com>
Date: Mon, 25 May 2026 20:41:02 +0300
Message-ID: 
 <CAPpHfds2kmPLMwO=h71+f0opCaWJAAhDQSagaKXzCwU4g-JqSQ@mail.gmail.com>
Subject: Re: Fix SPLIT PARTITION bound-overlap bug and other improvements
To: Chao Li <li.evan.chao@gmail.com>
Cc: Dmitry Koval <d.koval@postgrespro.ru>,
	PostgreSQL-development <pgsql-hackers@postgresql.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: 
 <https://www.postgresql.org/message-id/CAPpHfds2kmPLMwO%3Dh71%2Bf0opCaWJAAhDQSagaKXzCwU4g-JqSQ%40mail.gmail.com>
Precedence: bulk

On Thu, May 21, 2026 at 2:23=E2=80=AFAM Chao Li <li.evan.chao@gmail.com> wr=
ote:
>
> > On May 21, 2026, at 05:17, Alexander Korotkov <aekorotkov@gmail.com> wr=
ote:
> > On Wed, May 20, 2026 at 11:29=E2=80=AFPM Alexander Korotkov
> > <aekorotkov@gmail.com> wrote:
> >> On Wed, May 20, 2026 at 2:46=E2=80=AFPM Alexander Korotkov <aekorotkov=
@gmail.com> wrote:
> >>> On Wed, May 20, 2026 at 9:29=E2=80=AFAM Chao Li <li.evan.chao@gmail.c=
om> wrote:
> >>>>> On May 20, 2026, at 14:19, Alexander Korotkov <aekorotkov@gmail.com=
> wrote:
> >>>>> On Wed, May 20, 2026 at 2:37=E2=80=AFAM Chao Li <li.evan.chao@gmail=
.com> wrote:
> >>>>>>> On May 19, 2026, at 19:00, Alexander Korotkov <aekorotkov@gmail.c=
om> wrote:
> >>>>>>> On Tue, May 19, 2026 at 5:50=E2=80=AFAM Chao Li <li.evan.chao@gma=
il.com> wrote:
> >>>>>>>>> On May 18, 2026, at 20:04, Alexander Korotkov <aekorotkov@gmail=
.com> wrote:
> >>>>>>>>>
> >>>>>>>>> On Mon, May 18, 2026 at 2:57=E2=80=AFPM Chao Li <li.evan.chao@g=
mail.com> wrote:
> >>>>>>>>>>> <v3-0003-Clarify-SPLIT-PARTITION-bound-requirements-in-doc.pa=
tch><v3-0001-Fix-SPLIT-PARTITION-range-bound-validation-with-D.patch><v3-00=
02-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 bra=
in 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 n=
ot. So I spent some time creating a test that uses a case-insensitive colla=
tion:
> >>>>>>>> ```
> >>>>>>>> evantest=3D# create collation case_insensitive (provider=3Dicu, =
locale=3D'und-u-ks-level2', deterministic =3D false);
> >>>>>>>> CREATE COLLATION
> >>>>>>>> evantest=3D# create table t (b text collate case_insensitive) pa=
rtition by list (b);
> >>>>>>>> CREATE TABLE
> >>>>>>>> evantest=3D# create table tp_ab partition of t for values in ('a=
', 'b');
> >>>>>>>> CREATE TABLE
> >>>>>>>> evantest=3D# 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 par=
tition
> >>>>>>>> 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 DE=
FAULT partition.
> >>>>>>>> ```
> >>>>>>>>
> >>>>>>>> In this test, the split partition=E2=80=99s bound is ('a', 'b'),=
 and the new partition=E2=80=99s bound is ('a', 'A'). Their list lengths ar=
e both 2, but the two bounds are actually different, because 'a' and 'A' ar=
e considered equal by the collation.
> >>>>>>>>
> >>>>>>>> So, in the LIST path, since check_partition_bounds_for_split_lis=
t() has already ensured that the new partition=E2=80=99s bound is contained=
 within the split partition=E2=80=99s bound, we need to check the reverse d=
irection as well. Whether the split partition=E2=80=99s bound is also conta=
ined in the new partition=E2=80=99s bound. If yes, the two bounds are ident=
ical.
> >>>>>>>>
> >>>>>>>> See the attached v4 for my changes for 0004. 0001-0003 are uncha=
nged. 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 (a=
s it
> >>>>>>> internally uses datumIsEqual()).
> >>>>>>
> >>>>>> Yes, I realized that while reviewing v3. That=E2=80=99s reason I d=
idn=E2=80=99t 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=E2=80=99t 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 fo=
r me.
>
> Actually, when I added the ICU test, I did think about whether regression=
 tests support ICU. So I searched the existing tests for =E2=80=9Cicu=E2=80=
=9D 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 =3D icu, locale =3D '@colStrength=3Dprim=
ary;nonsense=3Dyes'); DROP COLLATION testx;
> CREATE COLLATION testx (provider =3D icu, locale =3D '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 =E2=80=9C=
-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