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

diff --git a/src/backend/optimizer/path/joinpath.c b/src/backend/optimizer/path/joinpath.c
index e47189a339d14c456192b0bc6eef4e71072df6e7..96f00fca5bf5cc9315b18d639cd82fe35e5e120c 100644 (file)
--- 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;