Fix best_inner_indexscan to actually enforce that an "inner indexscan" use

at least one join condition as an indexqual.  Before bitmap indexscans, this
oversight didn't really cost much except for redundantly considering the
same join paths twice; but as of 8.1 it could result in silly bitmap scans
that would do the same BitmapOr twice and then BitmapAnd these together :-(

diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c
index c740a89b9d8a886ba8eacb75d90d35e264cbd6ff..1184a047c1046bc478dbadd599eec9b81601df85 100644 (file)
--- a/src/backend/optimizer/path/indxpath.c
+++ b/src/backend/optimizer/path/indxpath.c
@@ -9,7 +9,7 @@
  *
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/optimizer/path/indxpath.c,v 1.202 2006/03/05 15:58:28 momjian Exp $
+ *   $PostgreSQL: pgsql/src/backend/optimizer/path/indxpath.c,v 1.203 2006/04/08 21:32:17 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -253,6 +253,10 @@ find_usable_indexes(PlannerInfo *root, RelOptInfo *rel,
    List       *all_clauses = NIL;      /* not computed till needed */
    ListCell   *ilist;
 
+   /* quick exit if no available clauses */
+   if (clauses == NIL)
+       return NIL;
+
    foreach(ilist, rel->indexlist)
    {
        IndexOptInfo *index = (IndexOptInfo *) lfirst(ilist);
@@ -1370,16 +1374,20 @@ best_inner_indexscan(PlannerInfo *root, RelOptInfo *rel,
    }
 
    /*
-    * Find all the relevant restriction and join clauses.
+    * Find all the relevant join clauses.
     */
    clause_list = find_clauses_for_join(root, rel, outer_relids, isouterjoin);
 
    /*
     * Find all the index paths that are usable for this join, except for
-    * stuff involving OR and ScalarArrayOpExpr clauses.
+    * stuff involving OR and ScalarArrayOpExpr clauses.  We can use both
+    * join and restriction clauses as indexquals, but we insist the path
+    * use at least one join clause (else it'd not be an "inner indexscan"
+    * but a plain indexscan, and those have already been considered).
     */
    indexpaths = find_usable_indexes(root, rel,
-                                    clause_list, NIL,
+                                    clause_list,
+                                    rel->baserestrictinfo,
                                     false, true,
                                     outer_relids,
                                     SAOP_FORBID);
@@ -1389,7 +1397,8 @@ best_inner_indexscan(PlannerInfo *root, RelOptInfo *rel,
     * clauses present in the clause list.
     */
    bitindexpaths = generate_bitmap_or_paths(root, rel,
-                                            clause_list, NIL,
+                                            clause_list,
+                                            rel->baserestrictinfo,
                                             true,
                                             outer_relids);
 
@@ -1439,13 +1448,12 @@ best_inner_indexscan(PlannerInfo *root, RelOptInfo *rel,
 
 /*
  * find_clauses_for_join
- *   Generate a list of clauses that are potentially useful for
+ *   Generate a list of join clauses that are potentially useful for
  *   scanning rel as the inner side of a nestloop join.
  *
- * We consider both join and restriction clauses.  Any joinclause that uses
- * only otherrels in the specified outer_relids is fair game.  But there must
- * be at least one such joinclause in the final list, otherwise we return NIL
- * indicating that there isn't any potential win here.
+ * Any joinclause that uses only otherrels in the specified outer_relids is
+ * fair game.  Note that restriction clauses on rel can also be used in
+ * forming index conditions, but we do not include those here.
  */
 static List *
 find_clauses_for_join(PlannerInfo *root, RelOptInfo *rel,
@@ -1473,28 +1481,28 @@ find_clauses_for_join(PlannerInfo *root, RelOptInfo *rel,
 
    bms_free(join_relids);
 
-   /* if no join clause was matched then forget it, per comments above */
+   /* quick exit if no join clause was matched */
    if (clause_list == NIL)
        return NIL;
 
-   /*
-    * We can also use any plain restriction clauses for the rel.  We put
-    * these at the front of the clause list for the convenience of
-    * remove_redundant_join_clauses, which can never remove non-join clauses
-    * and hence won't be able to get rid of a non-join clause if it appears
-    * after a join clause it is redundant with.
-    */
-   clause_list = list_concat(list_copy(rel->baserestrictinfo), clause_list);
-
    /*
     * We may now have clauses that are known redundant.  Get rid of 'em.
     */
    if (list_length(clause_list) > 1)
-   {
        clause_list = remove_redundant_join_clauses(root,
                                                    clause_list,
                                                    isouterjoin);
-   }
+
+   /*
+    * We might have found join clauses that are known redundant with
+    * restriction clauses on rel (due to conclusions drawn by implied
+    * equality deduction; without that, this would obviously never happen).
+    * Get rid of them too.
+    */
+   if (rel->baserestrictinfo != NIL)
+       clause_list = select_nonredundant_join_clauses(root, clause_list,
+                                                      rel->baserestrictinfo,
+                                                      isouterjoin);
 
    return clause_list;
 }