Skip to content

Commit

Permalink
Fix incorrect is-this-the-topmost-join tests in parallel planning.
Browse files Browse the repository at this point in the history
Two callers of generate_useful_gather_paths were testing the wrong
thing when deciding whether to call that function: they checked for
being at the top of the current join subproblem, rather than being at
the actual top join.  This'd result in failing to construct parallel
paths for a sub-join for which they might be useful.

While set_rel_pathlist() isn't actively broken, it seems best to
make its identical-in-intention test for this be like the other two.

This has been wrong all along, but given the lack of field complaints
I'm hesitant to back-patch into stable branches; we usually prefer
to avoid non-bug-fix changes in plan choices in minor releases.
It seems not too late for v15 though.

Richard Guo, reviewed by Antonin Houska and Tom Lane

Discussion: https://postgr.es/m/CAMbWs4-mH8Zf87-w+3P2J=nJB+5OyicO28ia9q_9o=Lamf_VHg@mail.gmail.com
  • Loading branch information
tglsfdc committed Jul 30, 2022
1 parent d92f2bc commit a3699c5
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 6 deletions.
2 changes: 1 addition & 1 deletion src/backend/optimizer/geqo/geqo_eval.c
Expand Up @@ -273,7 +273,7 @@ merge_clump(PlannerInfo *root, List *clumps, Clump *new_clump, int num_gene,
* rel once we know the final targetlist (see
* grouping_planner).
*/
if (old_clump->size + new_clump->size < num_gene)
if (!bms_equal(joinrel->relids, root->all_baserels))
generate_useful_gather_paths(root, joinrel, false);

/* Find and save the cheapest paths for this joinrel */
Expand Down
9 changes: 4 additions & 5 deletions src/backend/optimizer/path/allpaths.c
Expand Up @@ -553,12 +553,11 @@ set_rel_pathlist(PlannerInfo *root, RelOptInfo *rel,
* its own pool of workers. Instead, we'll consider gathering partial
* paths for the parent appendrel.
*
* Also, if this is the topmost scan/join rel (that is, the only baserel),
* we postpone gathering until the final scan/join targetlist is available
* (see grouping_planner).
* Also, if this is the topmost scan/join rel, we postpone gathering until
* the final scan/join targetlist is available (see grouping_planner).
*/
if (rel->reloptkind == RELOPT_BASEREL &&
bms_membership(root->all_baserels) != BMS_SINGLETON)
!bms_equal(rel->relids, root->all_baserels))
generate_useful_gather_paths(root, rel, false);

/* Now find the cheapest of the paths for this rel */
Expand Down Expand Up @@ -3402,7 +3401,7 @@ standard_join_search(PlannerInfo *root, int levels_needed, List *initial_rels)
* partial paths. We'll do the same for the topmost scan/join rel
* once we know the final targetlist (see grouping_planner).
*/
if (lev < levels_needed)
if (!bms_equal(rel->relids, root->all_baserels))
generate_useful_gather_paths(root, rel, false);

/* Find and save the cheapest paths for this rel */
Expand Down

0 comments on commit a3699c5

Please sign in to comment.