MIME-Version: 1.0
References: <CA+TgmoZ-Jh1T6QyWoCODMVQdhTUPYkaZjWztzP1En4=ZHoKPzw@mail.gmail.com>
 <CAKZiRmxtJAFG7e1+Vs9B8ngON=AOzJbuws+1ZeH4LsbJh5AzoQ@mail.gmail.com>
 <CA+TgmoY9Ne_Sh10u6LSPc3wvOQPLp3kF9nBp3nqJEG2JGF2QiA@mail.gmail.com>
 <CA+Tgmoa57S6mP=aTOXH2-gDAL4TMO1WbGgrHSg0s6J4zUH=04g@mail.gmail.com>
 <DEB1SAGVDX7S.JMV96X4R0RCS@gmail.com> <CA+Tgmoaf__2B0BUL+vrg28P+3buX=Ti-kybqkHiLTtFrrCfzuA@mail.gmail.com>
 <CA+TgmoYpcLNOuypOTdgCSLW7FuA=t6BtB3meTARHX2-Dj_81xQ@mail.gmail.com>
 <DEFNP3WX7BLX.1X7DS1P9X6JIF@nmfay.com> <CA+TgmoZjv9OyFu1Gkt78w0vWEti8S33w8trYHmErf-GMmGSi=w@mail.gmail.com>
 <DELPC4B56Y5A.3OBZ3WCH3T4VX@nmfay.com> <CA+TgmoaOSBQD9Ux4eG40w723ZN=c0J7p-+oX4+J8urUeyLMo5w@mail.gmail.com>
 <CAOYmi+=g+MMoOpWkk2weXWKJcKH0eKey8gKHHdH0dF4Tiawrhw@mail.gmail.com>
 <CA+TgmobwaT=PXPDDrgDup+jA8KHBbkxigtziD-zNzAKKkQYVgQ@mail.gmail.com>
 <CAOYmi+mOmEW=amDRQMfw6-Fb3ZmDEQFaJzwk8Bc8W8DzaP85XQ@mail.gmail.com>
 <CA+TgmoaX2AMW4cdFM3OngBJxmxpkdmzF33R7-CWhvRLfucbFMg@mail.gmail.com>
 <CAOYmi+njnRGcomnxTY6vsEW3wWigc0ruB0EyWFpb+PVVE8sWpw@mail.gmail.com>
 <CAOYmi+k4AyWCQHK=XVF99KVDuFkqxcADao61OWGLxu0nRYMONQ@mail.gmail.com>
 <CA+TgmoZ0x3ym_oueXRWzbM_=6ucKoPZVGj3rRMLBDC_FnetXDw@mail.gmail.com>
 <CAP53Pkycc=7N2bLzVT3x+qE1JamvRZWev5tFjdLJ1+-AV3Di+Q@mail.gmail.com>
 <CA+TgmoaKhuD91RnazbRyGkmP7--JdNq8oNDC3UcgTZSWbMxC7w@mail.gmail.com>
 <CAP53Pkw5-wMEeDJXFmqo_RTyL_spzCXb7HHKrbSnQqokVoZcNQ@mail.gmail.com>
 <CA+Tgmob-69bzbdi3U_QtebqAf6u1y8js=5=oNK639csVe1VbhA@mail.gmail.com> <CA+TgmoaZMOikxK=LqS+Jn+835h9S139JLGk-3LyETVXw5W5j=w@mail.gmail.com>
In-Reply-To: <CA+TgmoaZMOikxK=LqS+Jn+835h9S139JLGk-3LyETVXw5W5j=w@mail.gmail.com>
From: Alexandra Wang <alexandra.wang.oss@gmail.com>
Date: Mon, 2 Feb 2026 11:36:47 -0800
Message-ID: <CAK98qZ16UG0A8GNp4D_DD9Hect30XbsM190Br6uY-zx3KHg8ew@mail.gmail.com>
Subject: Re: pg_plan_advice
To: Robert Haas <robertmhaas@gmail.com>
Cc: Lukas Fittl <lukas@fittl.com>, Jacob Champion <jacob.champion@enterprisedb.com>, 
	Dian Fay <di@nmfay.com>, Matheus Alcantara <matheusssilv97@gmail.com>, 
	Jakub Wartak <jakub.wartak@enterprisedb.com>, 
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Content-Type: multipart/alternative; boundary="000000000000b6b7600649dc73b3"
Archived-At: <https://www.postgresql.org/message-id/CAK98qZ16UG0A8GNp4D_DD9Hect30XbsM190Br6uY-zx3KHg8ew%40mail.gmail.com>
Precedence: bulk

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

Hi Robert,

On Wed, Jan 28, 2026 at 9:14=E2=80=AFAM Robert Haas <robertmhaas@gmail.com>=
 wrote:

> Thanks to all who have reviewed so far, and please keep it coming. I
> am especially in need of more code review at this point.
>

Thanks for the patches! I=E2=80=99ve reviewed 0001 - 0003 so far; here are =
my
comments.

0001:
The code looks good to me. However, I feel a bit uneasy about not
seeing a test case for the additional subplan origin display added in
pg_overexplain. Maybe we could add the following test cases to
exercise that code:

-- should show "Subplan: sub"
EXPLAIN (RANGE_TABLE, COSTS OFF)
SELECT * FROM vegetables v,
       (SELECT * FROM vegetables WHERE genus =3D 'daucus' OFFSET 0) sub;

-- should show "Subplan: unnamed_subquery"
EXPLAIN (RANGE_TABLE, COSTS OFF)
SELECT * FROM vegetables v,
       (SELECT * FROM vegetables WHERE genus =3D 'daucus' OFFSET 0);

0002:
Looks good to me.

0003:
I see code like this:

@@ -2232,6 +2251,11 @@ accumulate_append_subpath(Path *path, List
**subpaths, List **special_subpaths)
  if (!apath->path.parallel_aware || apath->first_partial_path =3D=3D 0)
  {
  *subpaths =3D list_concat(*subpaths, apath->subpaths);
+ *child_append_relid_sets =3D
+ lappend(*child_append_relid_sets, path->parent->relids);
+ *child_append_relid_sets =3D
+ list_concat(*child_append_relid_sets,
+ apath->child_append_relid_sets);

in accumulate_append_subpath(), but in get_singleton_append_subpath()
there are only calls to lappend() and no list_concat(). Is that
intentional? Do we also want to concatenate the newly pulled up
child_append_relid_sets with the existing ones in
get_singleton_append_subpath()?

In add_paths_to_append_rel():

@@ -1785,13 +1790,16 @@ add_paths_to_append_rel(PlannerInfo *root,
RelOptInfo *rel,
        {
            Path       *path =3D (Path *) lfirst(l);
            AppendPath *appendpath;
+           AppendPathInput append =3D {0};
+
+           append.partial_subpaths =3D list_make1(path);
+           append.child_append_relid_sets =3D list_make1(rel->relids);

Could you help me understand why we need to populate
append.child_append_relid_sets here? I don=E2=80=99t see this child rel bei=
ng
pulled up at this point.

0004:
I=E2=80=99ve only read through the README and documentation so far; I=E2=80=
=99ll
continue reviewing the code in 0004.

Best,
Alex

--=20
Alexandra Wang
EDB: https://www.enterprisedb.com

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

<div dir=3D"ltr"><div>Hi Robert,</div><br><div class=3D"gmail_quote gmail_q=
uote_container"><div dir=3D"ltr" class=3D"gmail_attr">On Wed, Jan 28, 2026 =
at 9:14=E2=80=AFAM Robert Haas &lt;<a href=3D"mailto:robertmhaas@gmail.com"=
>robertmhaas@gmail.com</a>&gt; wrote:</div><blockquote class=3D"gmail_quote=
" style=3D"margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);=
padding-left:1ex">
Thanks to all who have reviewed so far, and please keep it coming. I<br>
am especially in need of more code review at this point.<br>
</blockquote></div><div><br clear=3D"all"></div><div>Thanks for the patches=
! I=E2=80=99ve reviewed 0001 - 0003 so far; here are my<br>comments.<br><br=
>0001:<br>The code looks good to me. However, I feel a bit uneasy about not=
<br>seeing a test case for the additional subplan origin display added in<b=
r>pg_overexplain. Maybe we could add the following test cases to<br>exercis=
e that code:<br><br>-- should show &quot;Subplan: sub&quot;<br>EXPLAIN (RAN=
GE_TABLE, COSTS OFF)<br>SELECT * FROM vegetables v,<br>=C2=A0 =C2=A0 =C2=A0=
 =C2=A0(SELECT * FROM vegetables WHERE genus =3D &#39;daucus&#39; OFFSET 0)=
 sub;<br><br>-- should show &quot;Subplan: unnamed_subquery&quot;<br>EXPLAI=
N (RANGE_TABLE, COSTS OFF)<br>SELECT * FROM vegetables v,<br>=C2=A0 =C2=A0 =
=C2=A0 =C2=A0(SELECT * FROM vegetables WHERE genus =3D &#39;daucus&#39; OFF=
SET 0);<br><br>0002:<br>Looks good to me.<br><br>0003:<br>I see code like t=
his:<br><br>@@ -2232,6 +2251,11 @@ accumulate_append_subpath(Path *path, Li=
st **subpaths, List **special_subpaths)<br>=C2=A0 if (!apath-&gt;path.paral=
lel_aware || apath-&gt;first_partial_path =3D=3D 0)<br>=C2=A0 {<br>=C2=A0 *=
subpaths =3D list_concat(*subpaths, apath-&gt;subpaths);<br>+ *child_append=
_relid_sets =3D<br>+ lappend(*child_append_relid_sets, path-&gt;parent-&gt;=
relids);<br>+ *child_append_relid_sets =3D<br>+ list_concat(*child_append_r=
elid_sets,<br>+ apath-&gt;child_append_relid_sets);<br><br>in accumulate_ap=
pend_subpath(), but in get_singleton_append_subpath()<br>there are only cal=
ls to lappend() and no list_concat(). Is that<br>intentional? Do we also wa=
nt to concatenate the newly pulled up<br>child_append_relid_sets with the e=
xisting ones in<br>get_singleton_append_subpath()?<br><br>In add_paths_to_a=
ppend_rel():<br><br>@@ -1785,13 +1790,16 @@ add_paths_to_append_rel(Planner=
Info *root, RelOptInfo *rel,<br>=C2=A0 =C2=A0 =C2=A0 =C2=A0 {<br>=C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 Path =C2=A0 =C2=A0 =C2=A0 *path =3D (Path *=
) lfirst(l);<br>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 AppendPath *appen=
dpath;<br>+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 AppendPathInput append =3D {=
0};<br>+<br>+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 append.partial_subpaths =
=3D list_make1(path);<br>+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 append.child_=
append_relid_sets =3D list_make1(rel-&gt;relids);<br><br>Could you help me =
understand why we need to populate<br>append.child_append_relid_sets here? =
I don=E2=80=99t see this child rel being<br>pulled up at this point.<br><br=
>0004:<br>I=E2=80=99ve only read through the README and documentation so fa=
r; I=E2=80=99ll<br>continue reviewing the code in 0004.<br><br>Best,<br>Ale=
x<br></div><div><div><br></div><span class=3D"gmail_signature_prefix">-- </=
span><br><div dir=3D"ltr" class=3D"gmail_signature"><div dir=3D"ltr"><font =
face=3D"monospace">Alexandra Wang</font><div><font face=3D"monospace">EDB: =
<a href=3D"https://www.enterprisedb.com" target=3D"_blank">https://www.ente=
rprisedb.com</a></font></div></div></div></div></div>

--000000000000b6b7600649dc73b3--