From e5b8aa636a63a9446e244fcc8d3a262e3e14049a Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Tue, 29 Nov 2016 19:32:35 -0500 Subject: [PATCH] Fix bogus handling of JOIN_UNIQUE_OUTER/INNER cases for parallel joins. consider_parallel_nestloop passed the wrong jointype down to its subroutines for JOIN_UNIQUE_INNER cases (it should pass JOIN_INNER), and it thought that it could pass paths other than innerrel->cheapest_total_path to create_unique_path, which create_unique_path is not on board with. These bugs would lead to assertion failures or other errors, suggesting that this code path hasn't been tested much. hash_inner_and_outer's code for parallel join effectively treated both JOIN_UNIQUE_OUTER and JOIN_UNIQUE_INNER the same as JOIN_INNER (for different reasons :-(), leading to incorrect plans that treated a semijoin as if it were a plain join. Michael Day submitted a test case demonstrating that hash_inner_and_outer failed for JOIN_UNIQUE_OUTER, and I found the other cases through code review. Report: https://postgr.es/m/D0E8A029-D1AC-42E8-979A-5DE4A77E4413@rcmail.com --- src/backend/optimizer/path/joinpath.c | 33 ++++++++++++++++----------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/src/backend/optimizer/path/joinpath.c b/src/backend/optimizer/path/joinpath.c index cc7384f7e5..d7aac5f69b 100644 --- a/src/backend/optimizer/path/joinpath.c +++ b/src/backend/optimizer/path/joinpath.c @@ -1217,8 +1217,12 @@ consider_parallel_nestloop(PlannerInfo *root, JoinType jointype, JoinPathExtraData *extra) { + JoinType save_jointype = jointype; ListCell *lc1; + if (jointype == JOIN_UNIQUE_INNER) + jointype = JOIN_INNER; + foreach(lc1, outerrel->partial_pathlist) { Path *outerpath = (Path *) lfirst(lc1); @@ -1244,18 +1248,19 @@ consider_parallel_nestloop(PlannerInfo *root, continue; /* - * Like match_unsorted_outer, we only consider a single nestloop - * path when the jointype is JOIN_UNIQUE_INNER. But we have to - * scan cheapest_parameterized_paths to find the one we want to - * consider, because cheapest_total_path might not be - * parallel-safe. + * If we're doing JOIN_UNIQUE_INNER, we can only use the inner's + * cheapest_total_path, and we have to unique-ify it. (We might + * be able to relax this to allow other safe, unparameterized + * inner paths, but right now create_unique_path is not on board + * with that.) */ - if (jointype == JOIN_UNIQUE_INNER) + if (save_jointype == JOIN_UNIQUE_INNER) { - if (!bms_is_empty(PATH_REQ_OUTER(innerpath))) + if (innerpath != innerrel->cheapest_total_path) continue; innerpath = (Path *) create_unique_path(root, innerrel, - innerpath, extra->sjinfo); + innerpath, + extra->sjinfo); Assert(innerpath); } @@ -1284,6 +1289,7 @@ hash_inner_and_outer(PlannerInfo *root, JoinType jointype, JoinPathExtraData *extra) { + JoinType save_jointype = jointype; bool isouterjoin = IS_OUTER_JOIN(jointype); List *hashclauses; ListCell *l; @@ -1450,9 +1456,9 @@ hash_inner_and_outer(PlannerInfo *root, * extended rows. Also, the resulting path must not be parameterized. */ if (joinrel->consider_parallel && - jointype != JOIN_UNIQUE_OUTER && - jointype != JOIN_FULL && - jointype != JOIN_RIGHT && + save_jointype != JOIN_UNIQUE_OUTER && + save_jointype != JOIN_FULL && + save_jointype != JOIN_RIGHT && outerrel->partial_pathlist != NIL && bms_is_empty(joinrel->lateral_relids)) { @@ -1466,11 +1472,12 @@ hash_inner_and_outer(PlannerInfo *root, * Normally, given that the joinrel is parallel-safe, the cheapest * total inner path will also be parallel-safe, but if not, we'll * have to search cheapest_parameterized_paths for the cheapest - * unparameterized inner path. + * safe, unparameterized inner path. If doing JOIN_UNIQUE_INNER, + * we can't use any alternative inner path. */ if (cheapest_total_inner->parallel_safe) cheapest_safe_inner = cheapest_total_inner; - else + else if (save_jointype != JOIN_UNIQUE_INNER) { ListCell *lc; -- 2.40.0