OSDN Git Service

Fix thinko in recent changes to handle ScalarArrayOpExpr as an indexable
authorTom Lane <tgl@sss.pgh.pa.us>
Thu, 18 May 2006 17:12:10 +0000 (17:12 +0000)
committerTom Lane <tgl@sss.pgh.pa.us>
Thu, 18 May 2006 17:12:10 +0000 (17:12 +0000)
condition: when there are multiple possible index paths involving
ScalarArrayOpExprs, they are logically to be ANDed together not ORed.
This thinko was a direct consequence of trying to put the processing
inside generate_bitmap_or_paths(), which I now see was a bit too cute.
So pull it out and make the callers do it separately (there are only two
that need it anyway).  Partially responds to bug #2441 from Arjen van der Meijden.
There are some additional infelicities exposed by his example, but they
are also in 8.1.x, while this mistake is not.

src/backend/optimizer/path/indxpath.c

index f5c4dcf..935494e 100644 (file)
@@ -9,7 +9,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/optimizer/path/indxpath.c,v 1.204 2006/04/09 18:18:41 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/optimizer/path/indxpath.c,v 1.205 2006/05/18 17:12:10 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -50,6 +50,10 @@ static List *find_usable_indexes(PlannerInfo *root, RelOptInfo *rel,
                                        bool istoplevel, bool isjoininner,
                                        Relids outer_relids,
                                        SaOpControl saop_control);
+static List *find_saop_paths(PlannerInfo *root, RelOptInfo *rel,
+                               List *clauses, List *outer_clauses,
+                               bool istoplevel, bool isjoininner,
+                               Relids outer_relids);
 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);
@@ -190,6 +194,15 @@ create_index_paths(PlannerInfo *root, RelOptInfo *rel)
        bitindexpaths = list_concat(bitindexpaths, indexpaths);
 
        /*
+        * Likewise, generate paths using ScalarArrayOpExpr clauses; these can't
+        * be simple indexscans but they can be used in bitmap scans.
+        */
+       indexpaths = find_saop_paths(root, rel,
+                                                                rel->baserestrictinfo, NIL,
+                                                                true, false, NULL);
+       bitindexpaths = list_concat(bitindexpaths, indexpaths);
+
+       /*
         * If we found anything usable, generate a BitmapHeapPath for the most
         * promising combination of bitmap index paths.
         */
@@ -279,10 +292,6 @@ find_usable_indexes(PlannerInfo *root, RelOptInfo *rel,
                 * predOK index to an arm of an OR, which would be a legal but
                 * pointlessly inefficient plan.  (A better plan will be generated by
                 * just scanning the predOK index alone, no OR.)
-                *
-                * If saop_control is SAOP_REQUIRE and istoplevel is false, the caller
-                * is only interested in indexquals involving ScalarArrayOps, so don't
-                * set useful_predicate to true.
                 */
                useful_predicate = false;
                if (index->indpred != NIL)
@@ -308,8 +317,7 @@ find_usable_indexes(PlannerInfo *root, RelOptInfo *rel,
                                if (!predicate_implied_by(index->indpred, all_clauses))
                                        continue;       /* can't use it at all */
 
-                               if (saop_control != SAOP_REQUIRE &&
-                                       !predicate_implied_by(index->indpred, outer_clauses))
+                               if (!predicate_implied_by(index->indpred, outer_clauses))
                                        useful_predicate = true;
                        }
                }
@@ -399,10 +407,53 @@ find_usable_indexes(PlannerInfo *root, RelOptInfo *rel,
 
 
 /*
+ * find_saop_paths
+ *             Find all the potential indexpaths that make use of ScalarArrayOpExpr
+ *             clauses.  The executor only supports these in bitmap scans, not
+ *             plain indexscans, so we need to segregate them from the normal case.
+ *             Otherwise, same API as find_usable_indexes().
+ *             Returns a list of IndexPaths.
+ */
+static List *
+find_saop_paths(PlannerInfo *root, RelOptInfo *rel,
+                               List *clauses, List *outer_clauses,
+                               bool istoplevel, bool isjoininner,
+                               Relids outer_relids)
+{
+       bool            have_saop = false;
+       ListCell   *l;
+
+       /*
+        * Since find_usable_indexes is relatively expensive, don't bother to
+        * run it unless there are some top-level ScalarArrayOpExpr clauses.
+        */
+       foreach(l, clauses)
+       {
+               RestrictInfo *rinfo = (RestrictInfo *) lfirst(l);
+
+               Assert(IsA(rinfo, RestrictInfo));
+               if (IsA(rinfo->clause, ScalarArrayOpExpr))
+               {
+                       have_saop = true;
+                       break;
+               }
+       }
+       if (!have_saop)
+               return NIL;
+
+       return find_usable_indexes(root, rel,
+                                                          clauses, outer_clauses,
+                                                          istoplevel, isjoininner,
+                                                          outer_relids,
+                                                          SAOP_REQUIRE);
+}
+
+
+/*
  * generate_bitmap_or_paths
- *             Look through the list of clauses to find OR clauses and
- *             ScalarArrayOpExpr clauses, and generate a BitmapOrPath for each one
- *             we can handle that way.  Return a list of the generated BitmapOrPaths.
+ *             Look through the list of clauses to find OR clauses, and generate
+ *             a BitmapOrPath for each one we can handle that way.  Return a list
+ *             of the generated BitmapOrPaths.
  *
  * outer_clauses is a list of additional clauses that can be assumed true
  * for the purpose of generating indexquals, but are not to be searched for
@@ -416,7 +467,6 @@ generate_bitmap_or_paths(PlannerInfo *root, RelOptInfo *rel,
 {
        List       *result = NIL;
        List       *all_clauses;
-       bool            have_saop = false;
        ListCell   *l;
 
        /*
@@ -433,16 +483,9 @@ generate_bitmap_or_paths(PlannerInfo *root, RelOptInfo *rel,
                ListCell   *j;
 
                Assert(IsA(rinfo, RestrictInfo));
-               /*
-                * In this loop we ignore RestrictInfos that aren't ORs; but take
-                * note of ScalarArrayOpExpr for later.
-                */
+               /* Ignore RestrictInfos that aren't ORs */
                if (!restriction_is_or_clause(rinfo))
-               {
-                       if (IsA(rinfo->clause, ScalarArrayOpExpr))
-                               have_saop = true;
                        continue;
-               }
 
                /*
                 * We must be able to match at least one index to each of the arms of
@@ -516,29 +559,6 @@ generate_bitmap_or_paths(PlannerInfo *root, RelOptInfo *rel,
                }
        }
 
-       /*
-        * If we saw any top-level ScalarArrayOpExpr clauses, see if we can
-        * generate a bitmap index path that uses those but not any OR clauses.
-        */
-       if (have_saop)
-       {
-               List       *pathlist;
-               Path       *bitmapqual;
-
-               pathlist = find_usable_indexes(root, rel,
-                                                                          clauses,
-                                                                          outer_clauses,
-                                                                          false,
-                                                                          isjoininner,
-                                                                          outer_relids,
-                                                                          SAOP_REQUIRE);
-               if (pathlist != NIL)
-               {
-                       bitmapqual = (Path *) create_bitmap_or_path(root, rel, pathlist);
-                       result = lappend(result, bitmapqual);
-               }
-       }
-
        return result;
 }
 
@@ -1429,8 +1449,8 @@ best_inner_indexscan(PlannerInfo *root, RelOptInfo *rel,
                                                                         SAOP_FORBID);
 
        /*
-        * Generate BitmapOrPaths for any suitable OR-clauses or ScalarArrayOpExpr
-        * clauses present in the clause list.
+        * Generate BitmapOrPaths for any suitable OR-clauses present in the
+        * clause list.
         */
        bitindexpaths = generate_bitmap_or_paths(root, rel,
                                                                                         clause_list, NIL,
@@ -1438,6 +1458,16 @@ best_inner_indexscan(PlannerInfo *root, RelOptInfo *rel,
                                                                                         outer_relids);
 
        /*
+        * Likewise, generate paths using ScalarArrayOpExpr clauses; these can't
+        * be simple indexscans but they can be used in bitmap scans.
+        */
+       bitindexpaths = list_concat(bitindexpaths,
+                                                               find_saop_paths(root, rel,
+                                                                                               clause_list, NIL,
+                                                                                               false, true,
+                                                                                               outer_relids));
+
+       /*
         * Include the regular index paths in bitindexpaths.
         */
        bitindexpaths = list_concat(bitindexpaths, list_copy(indexpaths));