Re: pg_plan_advice
| От | Alexandra Wang |
|---|---|
| Тема | Re: pg_plan_advice |
| Дата | |
| Msg-id | CAK98qZ16UG0A8GNp4D_DD9Hect30XbsM190Br6uY-zx3KHg8ew@mail.gmail.com обсуждение исходный текст |
| Ответ на | Re: pg_plan_advice (Robert Haas <robertmhaas@gmail.com>) |
| Список | pgsql-hackers |
Hi Robert,
On Wed, Jan 28, 2026 at 9:14 AM 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’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
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
В списке pgsql-hackers по дате отправления: