Correct bug in best_innerjoin(): it should check all the
authorTom Lane
Tue, 27 Jul 1999 06:23:12 +0000 (06:23 +0000)
committerTom Lane
Tue, 27 Jul 1999 06:23:12 +0000 (06:23 +0000)
rels that the inner path needs to join to, but it was only checking for
the first one.  Failure could only have been observed with an OR-clause
that mentions 3 or more tables, and then only if the bogus path was
actually selected as cheapest ...

src/backend/optimizer/path/joinpath.c
src/backend/optimizer/path/joinrels.c
src/include/optimizer/paths.h

index 9b5c226b55b0566f6d9c396b37cdd52d802d791e..667cdce4f2410f909a19579cccd89ec5a5141304 100644 (file)
@@ -7,7 +7,7 @@
  *
  *
  * IDENTIFICATION
- *   $Header: /cvsroot/pgsql/src/backend/optimizer/path/joinpath.c,v 1.41 1999/07/16 04:59:15 momjian Exp $
+ *   $Header: /cvsroot/pgsql/src/backend/optimizer/path/joinpath.c,v 1.42 1999/07/27 06:23:12 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -147,13 +147,14 @@ update_rels_pathlist_for_joins(Query *root, List *joinrels)
 /*
  * best_innerjoin
  *   Find the cheapest index path that has already been identified by
- *   (indexable_joinclauses) as being a possible inner path for the given
- *   outer relation in a nestloop join.
+ *   indexable_joinclauses() as being a possible inner path for the given
+ *   outer relation(s) in a nestloop join.
  *
- * 'join_paths' is a list of join nodes
- * 'outer_relid' is the relid of the outer join relation
+ * 'join_paths' is a list of potential inner indexscan join paths
+ * 'outer_relids' is the relid list of the outer join relation
  *
- * Returns the pathnode of the selected path.
+ * Returns the pathnode of the best path, or NULL if there's no
+ * usable path.
  */
 static Path *
 best_innerjoin(List *join_paths, Relids outer_relids)
@@ -165,7 +166,11 @@ best_innerjoin(List *join_paths, Relids outer_relids)
    {
        Path       *path = (Path *) lfirst(join_path);
 
-       if (intMember(lfirsti(path->joinid), outer_relids) &&
+       /* path->joinid is the set of base rels that must be part of
+        * outer_relids in order to use this inner path, because those
+        * rels are used in the index join quals of this inner path.
+        */
+       if (is_subset(path->joinid, outer_relids) &&
            (cheapest == NULL ||
             path_is_cheaper(path, cheapest)))
            cheapest = path;
index 9fba28759a0ca835d72e8efcb7a7dcbd08b97753..1eac677074447386e97f10da9afa3402b6f956cb 100644 (file)
@@ -7,7 +7,7 @@
  *
  *
  * IDENTIFICATION
- *   $Header: /cvsroot/pgsql/src/backend/optimizer/path/joinrels.c,v 1.37 1999/07/16 04:59:15 momjian Exp $
+ *   $Header: /cvsroot/pgsql/src/backend/optimizer/path/joinrels.c,v 1.38 1999/07/27 06:23:12 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -21,8 +21,6 @@
 #include "optimizer/tlist.h"
 
 static List *new_joininfo_list(List *joininfo_list, Relids join_relids);
-static bool nonoverlap_sets(List *s1, List *s2);
-static bool is_subset(List *s1, List *s2);
 static void set_joinrel_size(RelOptInfo *joinrel, RelOptInfo *outer_rel,
                 RelOptInfo *inner_rel, JoinInfo *jinfo);
 static RelOptInfo *make_join_rel(RelOptInfo *outer_rel, RelOptInfo *inner_rel,
@@ -373,8 +371,8 @@ new_joininfo_list(List *joininfo_list, Relids join_relids)
 RelOptInfo *
 get_cheapest_complete_rel(List *join_rel_list)
 {
-   List       *xrel = NIL;
    RelOptInfo *final_rel = NULL;
+   List       *xrel;
 
    /*
     * find the relations that have no further joins, i.e., its joininfos
@@ -383,8 +381,8 @@ get_cheapest_complete_rel(List *join_rel_list)
    foreach(xrel, join_rel_list)
    {
        RelOptInfo *rel = (RelOptInfo *) lfirst(xrel);
-       List       *xjoininfo = NIL;
        bool        final = true;
+       List       *xjoininfo;
 
        foreach(xjoininfo, rel->joininfo)
        {
@@ -405,36 +403,6 @@ get_cheapest_complete_rel(List *join_rel_list)
    return final_rel;
 }
 
-static bool
-nonoverlap_sets(List *s1, List *s2)
-{
-   List       *x = NIL;
-
-   foreach(x, s1)
-   {
-       int         e = lfirsti(x);
-
-       if (intMember(e, s2))
-           return false;
-   }
-   return true;
-}
-
-static bool
-is_subset(List *s1, List *s2)
-{
-   List       *x = NIL;
-
-   foreach(x, s1)
-   {
-       int         e = lfirsti(x);
-
-       if (!intMember(e, s2))
-           return false;
-   }
-   return true;
-}
-
 static void
 set_joinrel_size(RelOptInfo *joinrel, RelOptInfo *outer_rel, RelOptInfo *inner_rel, JoinInfo *jinfo)
 {
@@ -466,3 +434,39 @@ set_joinrel_size(RelOptInfo *joinrel, RelOptInfo *outer_rel, RelOptInfo *inner_r
 
    joinrel->tuples = ntuples;
 }
+
+/*
+ * Subset-inclusion tests on integer lists.
+ *
+ * XXX these probably ought to be in nodes/list.c or some such place.
+ */
+
+bool
+nonoverlap_sets(List *s1, List *s2)
+{
+   List       *x;
+
+   foreach(x, s1)
+   {
+       int         e = lfirsti(x);
+
+       if (intMember(e, s2))
+           return false;
+   }
+   return true;
+}
+
+bool
+is_subset(List *s1, List *s2)
+{
+   List       *x;
+
+   foreach(x, s1)
+   {
+       int         e = lfirsti(x);
+
+       if (!intMember(e, s2))
+           return false;
+   }
+   return true;
+}
index 75d9e328458afb81ec7b32d4732e5fa214b4bb8f..f074f1eee1f5a8b7d82d70ddcc2f791cc5001d8f 100644 (file)
@@ -1,13 +1,13 @@
 /*-------------------------------------------------------------------------
  *
  * paths.h
- *   prototypes for various files in optimizer/paths (were separate
- *   header files
+ *   prototypes for various files in optimizer/path (were separate
+ *   header files)
  *
  *
  * Copyright (c) 1994, Regents of the University of California
  *
- * $Id: paths.h,v 1.32 1999/07/27 03:51:01 tgl Exp $
+ * $Id: paths.h,v 1.33 1999/07/27 06:23:11 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
 #include "nodes/relation.h"
 
 /*
- * allpaths.h
+ * allpaths.c
  */
 extern RelOptInfo *make_one_rel(Query *root, List *rels);
 
 /*
- * indxpath.h
+ * indxpath.c
  *   routines to generate index paths
  */
 extern List *create_index_paths(Query *root, RelOptInfo *rel, List *indices,
@@ -31,26 +31,26 @@ extern List *create_index_paths(Query *root, RelOptInfo *rel, List *indices,
 extern List *expand_indexqual_conditions(List *indexquals);
 
 /*
- * joinpath.h
+ * joinpath.c
  *    routines to create join paths
  */
 extern void update_rels_pathlist_for_joins(Query *root, List *joinrels);
 
 
 /*
- * orindxpath.h
+ * orindxpath.c
  */
 extern List *create_or_index_paths(Query *root, RelOptInfo *rel, List *clauses);
 
 /*
- * hashutils.h
+ * hashutils.c
  *   routines to deal with hash keys and clauses
  */
 extern List *group_clauses_by_hashop(List *restrictinfo_list,
                        Relids inner_relids);
 
 /*
- * joinutils.h
+ * joinutils.c
  *   generic join method key/clause routines
  */
 extern bool order_joinkeys_by_pathkeys(List *pathkeys,
@@ -65,7 +65,7 @@ extern List *new_join_pathkeys(List *outer_pathkeys,
                  List *join_rel_tlist, List *joinclauses);
 
 /*
- * mergeutils.h
+ * mergeutils.c
  *   routines to deal with merge keys and clauses
  */
 extern List *group_clauses_by_order(List *restrictinfo_list,
@@ -74,7 +74,7 @@ extern MergeInfo *match_order_mergeinfo(PathOrder *ordering,
                      List *mergeinfo_list);
 
 /*
- * joinrels.h
+ * joinrels.c
  *   routines to determine which relations to join
  */
 extern List *make_rels_by_joins(Query *root, List *old_rels);
@@ -83,6 +83,8 @@ extern List *make_rels_by_clause_joins(Query *root, RelOptInfo *old_rel,
 extern List *make_rels_by_clauseless_joins(RelOptInfo *old_rel,
                              List *inner_rels);
 extern RelOptInfo *get_cheapest_complete_rel(List *join_rel_list);
+extern bool nonoverlap_sets(List *s1, List *s2);
+extern bool is_subset(List *s1, List *s2);
 
 /*
  * prototypes for path/prune.c