Tweak choose_bitmap_and() heuristics in the light of example provided in bug

#2075: consider an index redundant if any of its index conditions were already
used, rather than if all of them were.  Also, make the selectivity comparison
a bit fuzzy, so that very small differences in estimated selectivities don't
skew the results.

diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c
index fde89455685e76a5ecd84580f81f32d48ba592a4..c74e530e2b7af8d15a49378604f16a321dc14a9f 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.194 2005/11/25 19:47:49 tgl Exp $
+ *   $PostgreSQL: pgsql/src/backend/optimizer/path/indxpath.c,v 1.195 2005/11/30 17:10:19 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -53,6 +53,7 @@ static List *find_usable_indexes(PlannerInfo *root, RelOptInfo *rel,
 static Path *choose_bitmap_and(PlannerInfo *root, RelOptInfo *rel, List *paths);
 static int bitmap_path_comparator(const void *a, const void *b);
 static Cost bitmap_and_cost_est(PlannerInfo *root, RelOptInfo *rel, List *paths);
+static bool lists_intersect_ptr(List *list1, List *list2);
 static bool match_clause_to_indexcol(IndexOptInfo *index,
                         int indexcol, Oid opclass,
                         RestrictInfo *rinfo,
@@ -562,7 +563,7 @@ choose_bitmap_and(PlannerInfo *root, RelOptInfo *rel, List *paths)
     * In theory we should consider every nonempty subset of the given paths.
     * In practice that seems like overkill, given the crude nature of the
     * estimates, not to mention the possible effects of higher-level AND and
-    * OR clauses.  As a compromise, we sort the paths by selectivity. We
+    * OR clauses.  As a compromise, we sort the paths by selectivity.  We
     * always take the first, and sequentially add on paths that result in a
     * lower estimated cost.
     *
@@ -570,12 +571,20 @@ choose_bitmap_and(PlannerInfo *root, RelOptInfo *rel, List *paths)
     * can happen if there are multiple possibly usable indexes.  For this we
     * look only at plain IndexPath and single-element BitmapOrPath inputs
     * (the latter can arise in the presence of ScalarArrayOpExpr quals).  We
-    * consider an index redundant if all its index conditions were already
+    * consider an index redundant if any of its index conditions were already
     * used by earlier indexes.  (We could use predicate_implied_by to have a
     * more intelligent, but much more expensive, check --- but in most cases
     * simple pointer equality should suffice, since after all the index
     * conditions are all coming from the same RestrictInfo lists.)
     *
+    * You might think the condition for redundancy should be "all index
+    * conditions already used", not "any", but this turns out to be wrong.
+    * For example, if we use an index on A, and then come to an index with
+    * conditions on A and B, the only way that the second index can be later
+    * in the selectivity-order sort is if the condition on B is completely
+    * non-selective.  In any case, we'd surely be drastically misestimating
+    * the selectivity if we count the same condition twice.
+    *
     * XXX is there any risk of throwing away a useful partial index here
     * because we don't explicitly look at indpred?  At least in simple cases,
     * the partial index will sort before competing non-partial indexes and so
@@ -620,7 +629,7 @@ choose_bitmap_and(PlannerInfo *root, RelOptInfo *rel, List *paths)
        if (IsA(newpath, IndexPath))
        {
            newqual = ((IndexPath *) newpath)->indexclauses;
-           if (list_difference_ptr(newqual, qualsofar) == NIL)
+           if (lists_intersect_ptr(newqual, qualsofar))
                continue;       /* redundant */
        }
        else if (IsA(newpath, BitmapOrPath))
@@ -630,7 +639,7 @@ choose_bitmap_and(PlannerInfo *root, RelOptInfo *rel, List *paths)
            if (list_length(orquals) == 1 &&
                IsA(linitial(orquals), IndexPath))
                newqual = ((IndexPath *) linitial(orquals))->indexclauses;
-           if (list_difference_ptr(newqual, qualsofar) == NIL)
+           if (lists_intersect_ptr(newqual, qualsofar))
                continue;       /* redundant */
        }
 
@@ -665,19 +674,27 @@ bitmap_path_comparator(const void *a, const void *b)
    Cost        bcost;
    Selectivity aselec;
    Selectivity bselec;
+   Selectivity diff;
 
    cost_bitmap_tree_node(pa, &acost, &aselec);
    cost_bitmap_tree_node(pb, &bcost, &bselec);
 
-   if (aselec < bselec)
+   /*
+    * Since selectivities are often pretty crude, don't put blind faith
+    * in them; if the selectivities are within 1% of being the same, treat
+    * them as equal and sort by cost instead.
+    */
+   diff = aselec - bselec;
+   if (diff < -0.01)
        return -1;
-   if (aselec > bselec)
+   if (diff > 0.01)
        return 1;
-   /* if identical selectivity, sort by cost */
+
    if (acost < bcost)
        return -1;
    if (acost > bcost)
        return 1;
+
    return 0;
 }
 
@@ -704,6 +721,35 @@ bitmap_and_cost_est(PlannerInfo *root, RelOptInfo *rel, List *paths)
 }
 
 
+/*
+ * lists_intersect_ptr
+ *     Detect whether two lists have a nonempty intersection,
+ *     using pointer equality to compare members.
+ *
+ * This possibly should go into list.c, but it doesn't yet have any use
+ * except in choose_bitmap_and.
+ */
+static bool
+lists_intersect_ptr(List *list1, List *list2)
+{
+   ListCell   *cell1;
+
+   foreach(cell1, list1)
+   {
+       void   *datum1 = lfirst(cell1);
+       ListCell   *cell2;
+
+       foreach(cell2, list2)
+       {
+           if (lfirst(cell2) == datum1)
+               return true;
+       }
+   }
+
+   return false;
+}
+
+
 /****************************************************************************
  *             ----  ROUTINES TO CHECK RESTRICTIONS  ----
  ****************************************************************************/