Content-Type: text/plain;
	charset=utf-8
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3864.600.51.1.1\))
Subject: Re: Fix SPLIT PARTITION bound-overlap bug and other improvements
From: Chao Li <li.evan.chao@gmail.com>
In-Reply-To: 
 <CAPpHfdtRNFwVBrekh_b6Pg5fmi+QCoyEdAs7h=ikxXoDcOMdHA@mail.gmail.com>
Date: Thu, 21 May 2026 07:22:24 +0800
Cc: Dmitry Koval <d.koval@postgrespro.ru>,
 PostgreSQL-development <pgsql-hackers@postgresql.org>
Content-Transfer-Encoding: quoted-printable
Message-Id: <BE90212C-FC2C-4FE8-AC96-983D142F33FC@gmail.com>
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>
To: Alexander Korotkov <aekorotkov@gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/BE90212C-FC2C-4FE8-AC96-983D142F33FC%40gmail.com>
Precedence: bulk


> On May 21, 2026, at 05:17, Alexander Korotkov <aekorotkov@gmail.com> =
wrote:
>=20
> 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.com> 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.com> wrote:
>>>>>>> On Tue, May 19, 2026 at 5:50=E2=80=AFAM Chao Li =
<li.evan.chao@gmail.com> wrote:
>>>>>>>>> On May 18, 2026, at 20:04, Alexander Korotkov =
<aekorotkov@gmail.com> wrote:
>>>>>>>>>=20
>>>>>>>>> On Mon, May 18, 2026 at 2:57=E2=80=AFPM 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>
>>>>>>>>>>=20
>>>>>>>>>> v3-0001 through v3-0003 look good to me.
>>>>>>>>>>=20
>>>>>>>>>> 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.
>>>>>>>>>=20
>>>>>>>>> Sure, take your time.
>>>>>>>>>=20
>>>>>>>>> ------
>>>>>>>>> Regards,
>>>>>>>>> Alexander Korotkov
>>>>>>>>> Supabase
>>>>>>>>=20
>>>>>>>> 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=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) =
partition 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 =
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.
>>>>>>>> ```
>>>>>>>>=20
>>>>>>>> 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 are both 2, but the two bounds are actually different, because =
'a' and 'A' are considered equal by the collation.
>>>>>>>>=20
>>>>>>>> So, in the LIST path, since =
check_partition_bounds_for_split_list() has already ensured that the new =
partition=E2=80=99s bound is contained within the split partition=E2=80=99=
s bound, we need to check the reverse direction as well. Whether the =
split partition=E2=80=99s bound is also contained in the new =
partition=E2=80=99s bound. If yes, the two bounds are identical.
>>>>>>>>=20
>>>>>>>> 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.
>>>>>>>=20
>>>>>>> I've pushed 0001-0003.
>>>>>>=20
>>>>>> Thanks for pushing them.
>>>>>>=20
>>>>>>> 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()).
>>>>>>=20
>>>>>> Yes, I realized that while reviewing v3. That=E2=80=99s reason I =
didn=E2=80=99t get back v2 and only worked again based on v3.
>>>>>>=20
>>>>>>> I've revised the remaining patch:
>>>>>>> made function header comment a bit more detailed
>>>>>>=20
>>>>>> This part looks good to me.
>>>>>>=20
>>>>>>> and added additional
>>>>>>> regression tests.  Please, check.
>>>>>>>=20
>>>>>>=20
>>>>>> But I don=E2=80=99t see any change for regression test between v4 =
and v5. Maybe you forgot to save your changes?
>>>>>=20
>>>>> Sorry, I just mess up, no changes in tests.
>>>>> I'm going to push this if no objection.
>>>>>=20
>>>>=20
>>>> No worries. Then v5 looks good to me.
>>>=20
>>> Thank you, pushed.
>>=20
>> Uhhh, most of buildfarm animals don't support locales used in our
>> tests.  I've to revert that,
>=20
> The another attempt is attached.  Now use -0.0 and 0.0 as binary
> different but logically equivalent values, no locale dependence.
>=20
> ------
> 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 =
=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=3Dprimary;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/

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/