MIME-Version: 1.0
References: 
 <171085360143.2046436.7217841141682511557.pgcf@coridan.postgresql.org>
 <CACJufxEH45Rtkn=n9hjakeKEkH7ou8domh+iQHMp+18Wq_L-4g@mail.gmail.com>
 <e5ecd88e-4087-49cc-8c72-26c6c01f760b@postgrespro.ru>
 <CACJufxGWCVf5r9kE-z6MyR2b+wkaU15Q5m2tKz4cvBhYX3-x1g@mail.gmail.com>
 <f1c96833-6d37-46ac-ae9b-6d48ec55dd4c@postgrespro.ru>
 <CAPpHfdshkf0C2h09S00TVB8h3x8UT+GZyQ6LK2WGg1hOjnNE4g@mail.gmail.com>
 <CAPpHfdsh_jPZ6-2JtWoFeZravVQYY4cYCcnKXQW6KxEx+F5vxg@mail.gmail.com>
 <2643645f-2a02-43de-9321-c995ab1b7ef8@postgrespro.ru>
 <CACJufxF1Hfx1PST7pCLrKkGcjHEn2A51znXmYwVccnt5GDORcw@mail.gmail.com>
 <57a94d8a-d7b6-4c5e-9001-be1cd403b172@postgrespro.ru>
 <54414932-9E82-47AA-902B-2DED18F07F64@gmail.com>
 <e7da0ceb-8a01-4856-8c87-f87f09b4a9ad@postgrespro.ru>
 <CACJufxET9Qe-=zJtjGrKPr7-1QbVcP9y-vJU9CMEDfPGt_dbyg@mail.gmail.com>
 <3280c458-a886-4c58-8e2b-4d220b73161d@postgrespro.ru>
 <CACJufxEBvrtcOWyX2NMwOCvb2ZvaRGaGeiMQpUD=yxL6ncBFOg@mail.gmail.com>
 <dbe8e3ac-0b85-4809-bfaf-1d11f3c9d814@postgrespro.ru>
In-Reply-To: <dbe8e3ac-0b85-4809-bfaf-1d11f3c9d814@postgrespro.ru>
From: Alexander Korotkov <aekorotkov@gmail.com>
Date: Mon, 27 Oct 2025 14:13:36 +0200
Message-ID: 
 <CAPpHfdvSBR+sbsRCAybN949tOGArQvMH2QeDXf2L=7NDMNbJfw@mail.gmail.com>
Subject: Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
To: Dmitry Koval <d.koval@postgrespro.ru>
Cc: jian he <jian.universality@gmail.com>, pgsql-hackers@lists.postgresql.org
Content-Type: multipart/alternative; boundary="000000000000e52324064222d41f"
Archived-At: 
 <https://www.postgresql.org/message-id/CAPpHfdvSBR%2BsbsRCAybN949tOGArQvMH2QeDXf2L%3D7NDMNbJfw%40mail.gmail.com>
Precedence: bulk

--000000000000e52324064222d41f
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Mon, Sep 22, 2025 at 11:12=E2=80=AFPM Dmitry Koval <d.koval@postgrespro.=
ru>
wrote:

> Hi, Jiah He!
>
> 1.
>  >duplicated CommandCounterIncrement call?
>
> Probably this duplication is necessary to rename partition correctly ...
>

The second CommandCounterIncrement() is needed to make the renamed relation
visible within our transaction.  Why do we need the first one?  I see tests
pass without it.

Also, I doubt this is correct in the partitions_listdatum_intersection()
function.

    foreach_node(Const, val1, list1)
    {
        if (val1->constisnull)
        {
            if (isnull2)
            {
                result =3D lappend(result, val1);
                return result;
            }
            isnull1 =3D true;
            continue;
        }

The branch handling null value in the outer loop, uses null2 flag from the
inner loop.  I think for the null value of the outer loop we still need to
run inner loop to search for the matching null value.

------
Regards,
Alexander Korotkov
Supabase

--000000000000e52324064222d41f
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div dir=3D"ltr"><div>On Mon, Sep 22, 2025 at 11:12=E2=80=
=AFPM Dmitry Koval &lt;<a href=3D"mailto:d.koval@postgrespro.ru">d.koval@po=
stgrespro.ru</a>&gt; wrote:</div></div><div class=3D"gmail_quote gmail_quot=
e_container"><blockquote class=3D"gmail_quote" style=3D"margin:0px 0px 0px =
0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi, Jiah He!=
<br>
<br>
1.<br>
=C2=A0&gt;duplicated CommandCounterIncrement call?<br>
<br>
Probably this duplication is necessary to rename partition correctly ...<br=
></blockquote><div><br></div><div>The second CommandCounterIncrement() is n=
eeded to make the renamed relation visible within our transaction.=C2=A0 Wh=
y do we need the first one?=C2=A0 I see tests pass without it.</div><div><b=
r></div><div><div>Also, I doubt this is correct in the=C2=A0partitions_list=
datum_intersection() function.</div><div><br></div><div><font face=3D"monos=
pace">=C2=A0 =C2=A0 foreach_node(Const, val1, list1)<br>=C2=A0 =C2=A0 {<br>=
=C2=A0 =C2=A0 =C2=A0 =C2=A0 if (val1-&gt;constisnull)<br>=C2=A0 =C2=A0 =C2=
=A0 =C2=A0 {<br>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (isnull2)<br>=
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 {<br>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0 =C2=A0 result =3D lappend(result, val1);<br>=C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return result;<br>=C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 }<br>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 isnull1 =3D true;<br>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 continue=
;<br>=C2=A0 =C2=A0 =C2=A0 =C2=A0 }<br></font></div><div><br></div><div>The =
branch handling null value in the outer loop, uses null2 flag from the inne=
r loop.=C2=A0 I think for the null value of the outer loop we still need to=
 run inner loop to search for the matching null value.</div><div><br></div>=
</div><div>------</div><div><div dir=3D"ltr" class=3D"gmail_signature"><div=
 dir=3D"ltr">Regards,<br>Alexander Korotkov<div>Supabase</div></div></div><=
/div></div></div>

--000000000000e52324064222d41f--