]> granicus.if.org Git - postgresql/commitdiff
Revert my best_inner_indexscan patch of yesterday, which turns out to have
authorTom Lane <tgl@sss.pgh.pa.us>
Sun, 9 Apr 2006 18:18:41 +0000 (18:18 +0000)
committerTom Lane <tgl@sss.pgh.pa.us>
Sun, 9 Apr 2006 18:18:41 +0000 (18:18 +0000)
had a bad side-effect: it stopped finding plans that involved BitmapAnd
combinations of indexscans using both join and non-join conditions.  Instead,
make choose_bitmap_and more aggressive about detecting redundancies between
BitmapOr subplans.

src/backend/optimizer/path/indxpath.c

index 1184a047c1046bc478dbadd599eec9b81601df85..f5c4dcf18aabde41a0564f09f24bc0a9d7524492 100644 (file)
@@ -9,7 +9,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/optimizer/path/indxpath.c,v 1.203 2006/04/08 21:32:17 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/optimizer/path/indxpath.c,v 1.204 2006/04/09 18:18:41 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 List *pull_indexpath_quals(Path *bitmapqual);
 static bool lists_intersect_ptr(List *list1, List *list2);
 static bool match_clause_to_indexcol(IndexOptInfo *index,
                                                 int indexcol, Oid opclass,
@@ -253,10 +254,6 @@ 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);
@@ -581,9 +578,10 @@ choose_bitmap_and(PlannerInfo *root, RelOptInfo *rel, List *paths)
         * lower estimated cost.
         *
         * We also make some effort to detect directly redundant input paths, as
-        * 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
+        * can happen if there are multiple possibly usable indexes.  (Another
+        * way it can happen is that best_inner_indexscan will find the same OR
+        * join clauses that create_or_index_quals has pulled OR restriction
+        * clauses out of, and then both versions show up as duplicate paths.)  We
         * 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
@@ -620,53 +618,31 @@ choose_bitmap_and(PlannerInfo *root, RelOptInfo *rel, List *paths)
 
        paths = list_make1(patharray[0]);
        costsofar = bitmap_and_cost_est(root, rel, paths);
-       qualsofar = NIL;
-       if (IsA(patharray[0], IndexPath))
-               qualsofar = list_copy(((IndexPath *) patharray[0])->indexclauses);
-       else if (IsA(patharray[0], BitmapOrPath))
-       {
-               List   *orquals = ((BitmapOrPath *) patharray[0])->bitmapquals;
-
-               if (list_length(orquals) == 1 &&
-                       IsA(linitial(orquals), IndexPath))
-               qualsofar = list_copy(((IndexPath *) linitial(orquals))->indexclauses);
-       }
+       qualsofar = pull_indexpath_quals(patharray[0]);
        lastcell = list_head(paths);    /* for quick deletions */
 
        for (i = 1; i < npaths; i++)
        {
                Path       *newpath = patharray[i];
-               List       *newqual = NIL;
+               List       *newqual;
                Cost            newcost;
 
-               if (IsA(newpath, IndexPath))
-               {
-                       newqual = ((IndexPath *) newpath)->indexclauses;
-                       if (lists_intersect_ptr(newqual, qualsofar))
-                               continue;               /* redundant */
-               }
-               else if (IsA(newpath, BitmapOrPath))
-               {
-                       List   *orquals = ((BitmapOrPath *) newpath)->bitmapquals;
-
-                       if (list_length(orquals) == 1 &&
-                               IsA(linitial(orquals), IndexPath))
-                               newqual = ((IndexPath *) linitial(orquals))->indexclauses;
-                       if (lists_intersect_ptr(newqual, qualsofar))
-                               continue;               /* redundant */
-               }
-
+               newqual = pull_indexpath_quals(newpath);
+               if (lists_intersect_ptr(newqual, qualsofar))
+                       continue;                       /* consider it redundant */
+               /* tentatively add newpath to paths, so we can estimate cost */
                paths = lappend(paths, newpath);
                newcost = bitmap_and_cost_est(root, rel, paths);
                if (newcost < costsofar)
                {
+                       /* keep newpath in paths, update subsidiary variables */
                        costsofar = newcost;
-                       if (newqual)
-                               qualsofar = list_concat(qualsofar, list_copy(newqual));
+                       qualsofar = list_concat(qualsofar, newqual);
                        lastcell = lnext(lastcell);
                }
                else
                {
+                       /* reject newpath, remove it from paths list */
                        paths = list_delete_cell(paths, lnext(lastcell), lastcell);
                }
                Assert(lnext(lastcell) == NULL);
@@ -733,6 +709,62 @@ bitmap_and_cost_est(PlannerInfo *root, RelOptInfo *rel, List *paths)
        return bpath.total_cost;
 }
 
+/*
+ * pull_indexpath_quals
+ *
+ * Given the Path structure for a plain or bitmap indexscan, extract a
+ * list of RestrictInfo nodes for all the indexquals used in the Path.
+ *
+ * This is sort of a simplified version of make_restrictinfo_from_bitmapqual;
+ * here, we are not trying to produce an accurate representation of the AND/OR
+ * semantics of the Path, but just find out all the base conditions used.
+ *
+ * The result list contains pointers to the RestrictInfos used in the Path,
+ * but all the list cells are freshly built, so it's safe to destructively
+ * modify the list (eg, by concat'ing it with other lists).
+ */
+static List *
+pull_indexpath_quals(Path *bitmapqual)
+{
+       List       *result = NIL;
+       ListCell   *l;
+
+       if (IsA(bitmapqual, BitmapAndPath))
+       {
+               BitmapAndPath *apath = (BitmapAndPath *) bitmapqual;
+
+               foreach(l, apath->bitmapquals)
+               {
+                       List       *sublist;
+
+                       sublist = pull_indexpath_quals((Path *) lfirst(l));
+                       result = list_concat(result, sublist);
+               }
+       }
+       else if (IsA(bitmapqual, BitmapOrPath))
+       {
+               BitmapOrPath *opath = (BitmapOrPath *) bitmapqual;
+
+               foreach(l, opath->bitmapquals)
+               {
+                       List       *sublist;
+
+                       sublist = pull_indexpath_quals((Path *) lfirst(l));
+                       result = list_concat(result, sublist);
+               }
+       }
+       else if (IsA(bitmapqual, IndexPath))
+       {
+               IndexPath  *ipath = (IndexPath *) bitmapqual;
+
+               result = list_copy(ipath->indexclauses);
+       }
+       else
+               elog(ERROR, "unrecognized node type: %d", nodeTag(bitmapqual));
+
+       return result;
+}
+
 
 /*
  * lists_intersect_ptr
@@ -1374,20 +1406,24 @@ best_inner_indexscan(PlannerInfo *root, RelOptInfo *rel,
        }
 
        /*
-        * Find all the relevant join clauses.
+        * Find all the relevant restriction and join clauses.
+        *
+        * Note: because we include restriction clauses, we will find indexscans
+        * that could be plain indexscans, ie, they don't require the join context
+        * at all.  This may seem redundant, but we need to include those scans in
+        * the input given to choose_bitmap_and() to be sure we find optimal AND
+        * combinations of join and non-join scans.  The worst case is that we
+        * might return a "best inner indexscan" that's really just a plain
+        * indexscan, causing some redundant effort in joinpath.c.
         */
        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.  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).
+        * stuff involving OR and ScalarArrayOpExpr clauses.
         */
        indexpaths = find_usable_indexes(root, rel,
-                                                                        clause_list,
-                                                                        rel->baserestrictinfo,
+                                                                        clause_list, NIL,
                                                                         false, true,
                                                                         outer_relids,
                                                                         SAOP_FORBID);
@@ -1397,8 +1433,7 @@ best_inner_indexscan(PlannerInfo *root, RelOptInfo *rel,
         * clauses present in the clause list.
         */
        bitindexpaths = generate_bitmap_or_paths(root, rel,
-                                                                                        clause_list,
-                                                                                        rel->baserestrictinfo,
+                                                                                        clause_list, NIL,
                                                                                         true,
                                                                                         outer_relids);
 
@@ -1448,12 +1483,13 @@ best_inner_indexscan(PlannerInfo *root, RelOptInfo *rel,
 
 /*
  * find_clauses_for_join
- *       Generate a list of join clauses that are potentially useful for
+ *       Generate a list of clauses that are potentially useful for
  *       scanning rel as the inner side of a nestloop join.
  *
- * 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.
+ * 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.
  */
 static List *
 find_clauses_for_join(PlannerInfo *root, RelOptInfo *rel,
@@ -1481,28 +1517,28 @@ find_clauses_for_join(PlannerInfo *root, RelOptInfo *rel,
 
        bms_free(join_relids);
 
-       /* quick exit if no join clause was matched */
+       /* if no join clause was matched then forget it, per comments above */
        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;
 }