Fix nasty bug in optimization of multiway joins: optimizer

would sometimes generate a plan that omitted a sort step before merge.

diff --git a/src/backend/optimizer/path/hashutils.c b/src/backend/optimizer/path/hashutils.c
index 78441f041a100da4537fff2d8925cd76137ec62c..f67e4eb8eca87a8fde11b33d82f53e6661d8006f 100644 (file)
--- a/src/backend/optimizer/path/hashutils.c
+++ b/src/backend/optimizer/path/hashutils.c
@@ -7,7 +7,7 @@
  *
  *
  * IDENTIFICATION
- *       $Header: /cvsroot/pgsql/src/backend/optimizer/path/Attic/hashutils.c,v 1.14 1999/02/22 05:26:18 momjian Exp $
+ *       $Header: /cvsroot/pgsql/src/backend/optimizer/path/Attic/hashutils.c,v 1.15 1999/04/03 00:18:27 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -29,19 +29,20 @@ static HashInfo *match_hashop_hashinfo(Oid hashop, List *hashinfo_list);
  *       hash operator.
  *
  * 'restrictinfo_list' is the list of restrictinfo nodes
- * 'inner_relid' is the relid of the inner join relation
+ * 'inner_relids' is the list of relids in the inner join relation
+ *   (used to determine whether a join var is inner or outer)
  *
  * Returns the new list of hashinfo nodes.
  *
  */
 List *
 group_clauses_by_hashop(List *restrictinfo_list,
-                                               int inner_relid)
+                                               Relids inner_relids)
 {
        List       *hashinfo_list = NIL;
-       RestrictInfo *restrictinfo = (RestrictInfo *) NULL;
-       List       *i = NIL;
-       Oid                     hashjoinop = 0;
+       RestrictInfo *restrictinfo;
+       List       *i;
+       Oid                     hashjoinop;
 
        foreach(i, restrictinfo_list)
        {
@@ -54,23 +55,21 @@ group_clauses_by_hashop(List *restrictinfo_list,
                 */
                if (hashjoinop)
                {
-                       HashInfo   *xhashinfo = (HashInfo *) NULL;
                        Expr       *clause = restrictinfo->clause;
                        Var                *leftop = get_leftop(clause);
                        Var                *rightop = get_rightop(clause);
-                       JoinKey    *joinkey = (JoinKey *) NULL;
+                       HashInfo   *xhashinfo;
+                       JoinKey    *joinkey;
 
                        xhashinfo = match_hashop_hashinfo(hashjoinop, hashinfo_list);
-
-                       if (inner_relid == leftop->varno)
+                       joinkey = makeNode(JoinKey);
+                       if (intMember(leftop->varno, inner_relids))
                        {
-                               joinkey = makeNode(JoinKey);
                                joinkey->outer = rightop;
                                joinkey->inner = leftop;
                        }
                        else
                        {
-                               joinkey = makeNode(JoinKey);
                                joinkey->outer = leftop;
                                joinkey->inner = rightop;
                        }
@@ -79,16 +78,15 @@ group_clauses_by_hashop(List *restrictinfo_list,
                        {
                                xhashinfo = makeNode(HashInfo);
                                xhashinfo->hashop = hashjoinop;
-
                                xhashinfo->jmethod.jmkeys = NIL;
                                xhashinfo->jmethod.clauses = NIL;
-
                                hashinfo_list = lcons(xhashinfo, hashinfo_list);
                        }
 
-                       xhashinfo->jmethod.clauses = lcons(clause, xhashinfo->jmethod.clauses);
-
-                       xhashinfo->jmethod.jmkeys = lcons(joinkey, xhashinfo->jmethod.jmkeys);
+                       xhashinfo->jmethod.clauses = lcons(clause,
+                                                                                          xhashinfo->jmethod.clauses);
+                       xhashinfo->jmethod.jmkeys = lcons(joinkey,
+                                                                                         xhashinfo->jmethod.jmkeys);
                }
        }
        return hashinfo_list;

diff --git a/src/backend/optimizer/path/joinpath.c b/src/backend/optimizer/path/joinpath.c
index 47668a69b81c5c335a5c1899ec2867b03acc4420..5edfed65b0e9039a39a32afcfbebfa2723565c00 100644 (file)
--- a/src/backend/optimizer/path/joinpath.c
+++ b/src/backend/optimizer/path/joinpath.c
@@ -7,7 +7,7 @@
  *
  *
  * IDENTIFICATION
- *       $Header: /cvsroot/pgsql/src/backend/optimizer/path/joinpath.c,v 1.32 1999/02/22 05:26:20 momjian Exp $
+ *       $Header: /cvsroot/pgsql/src/backend/optimizer/path/joinpath.c,v 1.33 1999/04/03 00:18:28 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -97,11 +97,11 @@ update_rels_pathlist_for_joins(Query *root, List *joinrels)
 
                if (_enable_mergejoin_)
                        mergeinfo_list = group_clauses_by_order(joinrel->restrictinfo,
-                                                                          lfirsti(innerrel->relids));
+                                                                                                       innerrel->relids);
 
                if (_enable_hashjoin_)
                        hashinfo_list = group_clauses_by_hashop(joinrel->restrictinfo,
-                                                                               lfirsti(innerrel->relids));
+                                                                                                       innerrel->relids);
 
                /* need to flatten the relids list */
                joinrel->relids = nconc(listCopy(outerrelids),
@@ -173,12 +173,10 @@ best_innerjoin(List *join_paths, Relids outer_relids)
        {
                Path       *path = (Path *) lfirst(join_path);
 
-               if (intMember(lfirsti(path->joinid), outer_relids)
-                       && ((cheapest == NULL ||
-                                path_is_cheaper((Path *) lfirst(join_path), cheapest))))
-               {
-                       cheapest = (Path *) lfirst(join_path);
-               }
+               if (intMember(lfirsti(path->joinid), outer_relids) &&
+                       (cheapest == NULL ||
+                        path_is_cheaper(path, cheapest)))
+                       cheapest = path;
        }
        return cheapest;
 }

diff --git a/src/backend/optimizer/path/mergeutils.c b/src/backend/optimizer/path/mergeutils.c
index 5f5a11bc7874406a13deca1c206cf979ed00bfed..a5bc8d96980f6a6d94419d5a5aaff384eb06baff 100644 (file)
--- a/src/backend/optimizer/path/mergeutils.c
+++ b/src/backend/optimizer/path/mergeutils.c
@@ -7,7 +7,7 @@
  *
  *
  * IDENTIFICATION
- *       $Header: /cvsroot/pgsql/src/backend/optimizer/path/Attic/mergeutils.c,v 1.20 1999/03/01 00:10:32 tgl Exp $
+ *       $Header: /cvsroot/pgsql/src/backend/optimizer/path/Attic/mergeutils.c,v 1.21 1999/04/03 00:18:28 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -36,14 +36,15 @@
  * Something to fix next time...
  *
  * 'restrictinfo_list' is the list of restrictinfo nodes
- * 'inner_relid' is the relid of the inner join relation
+ * 'inner_relids' is the list of relids in the inner join relation
+ *   (used to determine whether a join var is inner or outer)
  *
  * Returns the new list of mergeinfo nodes.
  *
  */
 List *
 group_clauses_by_order(List *restrictinfo_list,
-                                          int inner_relid)
+                                          Relids inner_relids)
 {
        List       *mergeinfo_list = NIL;
        List       *xrestrictinfo;
@@ -59,26 +60,25 @@ group_clauses_by_order(List *restrictinfo_list,
                         * Create a new mergeinfo node and add it to 'mergeinfo_list'
                         * if one does not yet exist for this merge ordering.
                         */
-                       PathOrder       *pathorder;
-                       MergeInfo       *xmergeinfo;
                        Expr       *clause = restrictinfo->clause;
                        Var                *leftop = get_leftop(clause);
                        Var                *rightop = get_rightop(clause);
+                       PathOrder       *pathorder;
+                       MergeInfo       *xmergeinfo;
                        JoinKey    *jmkeys;
 
                        pathorder = makeNode(PathOrder);
                        pathorder->ordtype = MERGE_ORDER;
                        pathorder->ord.merge = merge_ordering;
                        xmergeinfo = match_order_mergeinfo(pathorder, mergeinfo_list);
-                       if (inner_relid == leftop->varno)
+                       jmkeys = makeNode(JoinKey);
+                       if (intMember(leftop->varno, inner_relids))
                        {
-                               jmkeys = makeNode(JoinKey);
                                jmkeys->outer = rightop;
                                jmkeys->inner = leftop;
                        }
                        else
                        {
-                               jmkeys = makeNode(JoinKey);
                                jmkeys->outer = leftop;
                                jmkeys->inner = rightop;
                        }
@@ -86,10 +86,8 @@ group_clauses_by_order(List *restrictinfo_list,
                        if (xmergeinfo == NULL)
                        {
                                xmergeinfo = makeNode(MergeInfo);
-
                                xmergeinfo->m_ordering = merge_ordering;
-                               mergeinfo_list = lcons(xmergeinfo,
-                                                                          mergeinfo_list);
+                               mergeinfo_list = lcons(xmergeinfo, mergeinfo_list);
                        }
 
                        xmergeinfo->jmethod.clauses = lcons(clause,

diff --git a/src/include/optimizer/paths.h b/src/include/optimizer/paths.h
index dfd2dfa7871ac3c68a85e27e57b5eb2857dbbda6..603efd7e52c57b6aff65244fb659384fc4d5a11e 100644 (file)
--- a/src/include/optimizer/paths.h
+++ b/src/include/optimizer/paths.h
@@ -7,7 +7,7 @@
  *
  * Copyright (c) 1994, Regents of the University of California
  *
- * $Id: paths.h,v 1.26 1999/02/22 05:26:52 momjian Exp $
+ * $Id: paths.h,v 1.27 1999/04/03 00:18:26 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -48,7 +48,7 @@ extern List *create_or_index_paths(Query *root, RelOptInfo *rel, List *clauses);
  *       routines to deal with hash keys and clauses
  */
 extern List *group_clauses_by_hashop(List *restrictinfo_list,
-                                               int inner_relid);
+                                                                        Relids inner_relids);
 
 /*
  * joinutils.h
@@ -70,9 +70,9 @@ extern List *new_join_pathkeys(List *outer_pathkeys,
  *       routines to deal with merge keys and clauses
  */
 extern List *group_clauses_by_order(List *restrictinfo_list,
-                                          int inner_relid);
+                                                                       Relids inner_relids);
 extern MergeInfo *match_order_mergeinfo(PathOrder *ordering,
-                                         List *mergeinfo_list);
+                                                                               List *mergeinfo_list);
 
 /*
  * joinrels.h