Fix thinko in recent changes to handle ScalarArrayOpExpr as an indexable

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.
This commit is contained in:
Tom Lane 2006-05-18 17:12:10 +00:00
parent d0f9ca34bd
commit d18e334c65
1 changed files with 74 additions and 44 deletions

View 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);
@ -189,6 +193,15 @@ create_index_paths(PlannerInfo *root, RelOptInfo *rel)
false, NULL);
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;
}
}
@ -398,11 +406,54 @@ 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,14 +1449,24 @@ 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,
true,
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.
*/