Skip to content
Snippets Groups Projects
Commit dc0eb137 authored by Tom Lane's avatar Tom Lane
Browse files

Call set_rel_pathlist_hook before generate_gather_paths, not after.

The previous ordering of these steps satisfied the nominal requirement
that set_rel_pathlist_hook could editorialize on the whole set of Paths
constructed for a base relation.  In practice, though, trying to change
the set of partial paths was impossible.  Adding one didn't work because
(a) it was too late to be included in Gather paths made by the core code,
and (b) calling add_partial_path after generate_gather_paths is unsafe,
because it might try to delete a path it thinks is dominated, but that
is already embedded in some Gather path(s).  Nor could the hook safely
remove partial paths, for the same reason that they might already be
embedded in Gathers.

Better to call extensions first, let them add partial paths as desired,
and then gather.  In v11 and up, we already doubled down on that ordering
by postponing gathering even further for single-relation queries; so even
if the hook wished to editorialize on Gather path construction, it could
not.

Report and patch by KaiGai Kohei.  Back-patch to 9.6 where Gather paths
were added.

Discussion: https://postgr.es/m/CAOP8fzahwpKJRTVVTqo2AE=mDTz_efVzV6Get_0=U3SO+-ha1A@mail.gmail.com
parent 0d7d71b6
No related branches found
Tags
No related merge requests found
...@@ -37,8 +37,9 @@ ...@@ -37,8 +37,9 @@
<para> <para>
A custom scan provider will typically add paths for a base relation by A custom scan provider will typically add paths for a base relation by
setting the following hook, which is called after the core code has setting the following hook, which is called after the core code has
generated what it believes to be the complete and correct set of access generated all the access paths it can for the relation (except for
paths for the relation. Gather paths, which are made after this call so that they can use
partial paths added by the hook):
<programlisting> <programlisting>
typedef void (*set_rel_pathlist_hook_type) (PlannerInfo *root, typedef void (*set_rel_pathlist_hook_type) (PlannerInfo *root,
RelOptInfo *rel, RelOptInfo *rel,
......
...@@ -478,24 +478,29 @@ set_rel_pathlist(PlannerInfo *root, RelOptInfo *rel, ...@@ -478,24 +478,29 @@ set_rel_pathlist(PlannerInfo *root, RelOptInfo *rel,
} }
} }
/*
* If this is a baserel, consider gathering any partial paths we may have
* created for it. (If we tried to gather inheritance children, we could
* end up with a very large number of gather nodes, each trying to grab
* its own pool of workers, so don't do this for otherrels. Instead,
* we'll consider gathering partial paths for the parent appendrel.)
*/
if (rel->reloptkind == RELOPT_BASEREL)
generate_gather_paths(root, rel);
/* /*
* Allow a plugin to editorialize on the set of Paths for this base * Allow a plugin to editorialize on the set of Paths for this base
* relation. It could add new paths (such as CustomPaths) by calling * relation. It could add new paths (such as CustomPaths) by calling
* add_path(), or delete or modify paths added by the core code. * add_path(), or add_partial_path() if parallel aware. It could also
* delete or modify paths added by the core code.
*/ */
if (set_rel_pathlist_hook) if (set_rel_pathlist_hook)
(*set_rel_pathlist_hook) (root, rel, rti, rte); (*set_rel_pathlist_hook) (root, rel, rti, rte);
/*
* If this is a baserel, we should normally consider gathering any partial
* paths we may have created for it. We have to do this after calling the
* set_rel_pathlist_hook, else it cannot add partial paths to be included
* here.
*
* However, if this is an inheritance child, skip it. Otherwise, we could
* end up with a very large number of gather nodes, each trying to grab
* its own pool of workers. Instead, we'll consider gathering partial
* paths for the parent appendrel.
*/
if (rel->reloptkind == RELOPT_BASEREL)
generate_gather_paths(root, rel);
/* Now find the cheapest of the paths for this rel */ /* Now find the cheapest of the paths for this rel */
set_cheapest(rel); set_cheapest(rel);
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment