public inbox for [email protected]  
help / color / mirror / Atom feed
From: Alexandra Wang <[email protected]>
To: Robert Haas <[email protected]>
Cc: Lukas Fittl <[email protected]>
Cc: Jacob Champion <[email protected]>
Cc: Dian Fay <[email protected]>
Cc: Matheus Alcantara <[email protected]>
Cc: Jakub Wartak <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: pg_plan_advice
Date: Mon, 2 Feb 2026 11:36:47 -0800
Message-ID: <CAK98qZ16UG0A8GNp4D_DD9Hect30XbsM190Br6uY-zx3KHg8ew@mail.gmail.com> (raw)
In-Reply-To: <CA+TgmoaZMOikxK=LqS+Jn+835h9S139JLGk-3LyETVXw5W5j=w@mail.gmail.com>
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>
	<[email protected]>
	<CA+Tgmoaf__2B0BUL+vrg28P+3buX=Ti-kybqkHiLTtFrrCfzuA@mail.gmail.com>
	<CA+TgmoYpcLNOuypOTdgCSLW7FuA=t6BtB3meTARHX2-Dj_81xQ@mail.gmail.com>
	<[email protected]>
	<CA+TgmoZjv9OyFu1Gkt78w0vWEti8S33w8trYHmErf-GMmGSi=w@mail.gmail.com>
	<[email protected]>
	<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>

Hi Robert,

On Wed, Jan 28, 2026 at 9:14 AM Robert Haas <[email protected]> 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’ve 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 = 'daucus' OFFSET 0) sub;

-- should show "Subplan: unnamed_subquery"
EXPLAIN (RANGE_TABLE, COSTS OFF)
SELECT * FROM vegetables v,
       (SELECT * FROM vegetables WHERE genus = '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 == 0)
  {
  *subpaths = list_concat(*subpaths, apath->subpaths);
+ *child_append_relid_sets =
+ lappend(*child_append_relid_sets, path->parent->relids);
+ *child_append_relid_sets =
+ 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 = (Path *) lfirst(l);
            AppendPath *appendpath;
+           AppendPathInput append = {0};
+
+           append.partial_subpaths = list_make1(path);
+           append.child_append_relid_sets = list_make1(rel->relids);

Could you help me understand why we need to populate
append.child_append_relid_sets here? I don’t see this child rel being
pulled up at this point.

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

Best,
Alex

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


view thread (143+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: pg_plan_advice
  In-Reply-To: <CAK98qZ16UG0A8GNp4D_DD9Hect30XbsM190Br6uY-zx3KHg8ew@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox