Correct bug in best_innerjoin(): it should check all the

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 ...

Author: tglsfdc

src/backend/optimizer/path/joinpath.c

@@ -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;

src/backend/optimizer/path/joinrels.c

@@ -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;
+}

src/include/optimizer/paths.h

@@ -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 $
  *
  *-------------------------------------------------------------------------
  */
@@ -17,12 +17,12 @@
 #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