open-dsi
/
postgres
mirror of https://github.com/postgres/postgres
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix some problems with selectivity estimation for partial indexes.

Browse Source 
First, genericcostestimate() was being way too liberal about including
partial-index conditions in its selectivity estimate, resulting in
substantial underestimates for situations such as an indexqual "x = 42"
used with an index on x "WHERE x >= 40 AND x < 50".  While the code is
intentionally set up to favor selecting partial indexes when available,
this was too much...

Second, choose_bitmap_and() was likewise easily fooled by cases of this
type, since it would similarly think that the partial index had selectivity
independent of the indexqual.

Fixed by using predicate_implied_by() rather than simple equality checks
to determine redundancy.  This is a good deal more expensive but I don't
see much alternative.  At least the extra cost is only paid when there's
actually a partial index under consideration.

Per report from Jeff Davis.  I'm not going to risk back-patching this,
though.
REL8_3_STABLE
Tom Lane 19 years ago
parent 2b49e5d3cb
commit 54d20024c1
4 changed files with 120 additions and 60 deletions
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Show Stats Download Patch File Download Diff File
  1. 109
      src/backend/optimizer/path/indxpath.c
  2. 51
      src/backend/utils/adt/selfuncs.c
  3. 9
      src/test/regress/expected/select.out
  4. 11
      src/test/regress/sql/select.sql

109
src/backend/optimizer/path/indxpath.c
Unescape View File

@ -9,7 +9,7 @@
*
*
* IDENTIFICATION
* $PostgreSQL: pgsql/src/backend/optimizer/path/indxpath.c,v 1.217 2007/03/17 00:11:04 tgl Exp $
* $PostgreSQL: pgsql/src/backend/optimizer/path/indxpath.c,v 1.218 2007/03/21 22:18:12 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@ -57,8 +57,8 @@ static Path *choose_bitmap_and(PlannerInfo *root, RelOptInfo *rel,
static int bitmap_path_comparator(const void *a, const void *b);
static Cost bitmap_and_cost_est(PlannerInfo *root, RelOptInfo *rel,
List *paths, RelOptInfo *outer_rel);
static List *pull_indexpath_quals(Path *bitmapqual);
static bool lists_intersect_ptr(List *list1, List *list2);
static void find_indexpath_quals(Path *bitmapqual, List **quals, List **preds);
static bool lists_intersect(List *list1, List *list2);
static bool match_clause_to_indexcol(IndexOptInfo *index,
int indexcol, Oid opfamily,
RestrictInfo *rinfo,
@ -562,6 +562,7 @@ choose_bitmap_and(PlannerInfo *root, RelOptInfo *rel,
Path **patharray;
Cost costsofar;
List *qualsofar;
List *firstpred;
ListCell *lastcell;
int i;
ListCell *l;
@ -586,8 +587,7 @@ choose_bitmap_and(PlannerInfo *root, RelOptInfo *rel,
* 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.)
* simple equality should suffice.)
*
* You might think the condition for redundancy should be "all index
* conditions already used", not "any", but this turns out to be wrong.
@ -597,10 +597,27 @@ choose_bitmap_and(PlannerInfo *root, RelOptInfo *rel,
* non-selective. In any case, we'd surely be drastically misestimating
* the selectivity if we count the same condition twice.
*
* We include index predicate conditions in the redundancy test. Because
* the test is just for pointer equality and not equal(), the effect is
* that use of the same partial index in two different AND elements is
* considered redundant. (XXX is this too strong?)
* We must also consider index predicate conditions in checking for
* redundancy, because the estimated selectivity of a partial index
* includes its predicate even if the explicit index conditions don't.
* Here we have to work harder than just checking expression equality:
* we check to see if any of the predicate clauses are implied by
* index conditions or predicate clauses of previous paths. This covers
* cases such as a condition "x = 42" used with a plain index, followed
* by a clauseless scan of a partial index "WHERE x >= 40 AND x < 50".
* Also, we reject indexes that have a qual condition matching any
* previously-used index's predicate (by including predicate conditions
* into qualsofar). It should be sufficient to check equality in this
* case, not implication, since we've sorted the paths by selectivity
* and so tighter conditions are seen first --- only for exactly equal
* cases might the partial index come first.
*
* XXX the reason we need all these redundancy checks is that costsize.c
* and clausesel.c aren't very smart about redundant clauses: they will
* usually double-count the redundant clauses, producing a too-small
* selectivity that makes a redundant AND look like it reduces the total
* cost. Perhaps someday that code will be smarter and we can remove
* these heuristics.
*
* Note: outputting the selected sub-paths in selectivity order is a good
* thing even if we weren't using that as part of the selection method,
@ -619,18 +636,38 @@ choose_bitmap_and(PlannerInfo *root, RelOptInfo *rel,
paths = list_make1(patharray[0]);
costsofar = bitmap_and_cost_est(root, rel, paths, outer_rel);
qualsofar = pull_indexpath_quals(patharray[0]);
find_indexpath_quals(patharray[0], &qualsofar, &firstpred);
qualsofar = list_concat(qualsofar, firstpred);
lastcell = list_head(paths); /* for quick deletions */
for (i = 1; i < npaths; i++)
{
Path *newpath = patharray[i];
List *newqual;
List *newpred;
Cost newcost;
newqual = pull_indexpath_quals(newpath);
if (lists_intersect_ptr(newqual, qualsofar))
find_indexpath_quals(newpath, &newqual, &newpred);
if (lists_intersect(newqual, qualsofar))
continue; /* consider it redundant */
if (newpred)
{
bool redundant = false;
/* we check each predicate clause separately */
foreach(l, newpred)
{
Node *np = (Node *) lfirst(l);
if (predicate_implied_by(list_make1(np), qualsofar))
{
redundant = true;
break; /* out of inner loop */
}
}
if (redundant)
continue;
}
/* tentatively add newpath to paths, so we can estimate cost */
paths = lappend(paths, newpath);
newcost = bitmap_and_cost_est(root, rel, paths, outer_rel);
@ -638,7 +675,7 @@ choose_bitmap_and(PlannerInfo *root, RelOptInfo *rel,
{
/* keep newpath in paths, update subsidiary variables */
costsofar = newcost;
qualsofar = list_concat(qualsofar, newqual);
qualsofar = list_concat(list_concat(qualsofar, newqual), newpred);
lastcell = lnext(lastcell);
}
else
@ -710,35 +747,39 @@ bitmap_and_cost_est(PlannerInfo *root, RelOptInfo *rel,
}
/*
* pull_indexpath_quals
* find_indexpath_quals
*
* Given the Path structure for a plain or bitmap indexscan, extract a list
* Given the Path structure for a plain or bitmap indexscan, extract lists
* of all the indexquals and index predicate conditions 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 expressions used in the Path,
* The result lists contain pointers to the expressions 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).
* modify the lists (eg, by concat'ing with other lists).
*/
static List *
pull_indexpath_quals(Path *bitmapqual)
static void
find_indexpath_quals(Path *bitmapqual, List **quals, List **preds)
{
List *result = NIL;
ListCell *l;
*quals = NIL;
*preds = NIL;
if (IsA(bitmapqual, BitmapAndPath))
{
BitmapAndPath *apath = (BitmapAndPath *) bitmapqual;
foreach(l, apath->bitmapquals)
{
List *sublist;
List *subquals;
List *subpreds;
sublist = pull_indexpath_quals((Path *) lfirst(l));
result = list_concat(result, sublist);
find_indexpath_quals((Path *) lfirst(l), &subquals, &subpreds);
*quals = list_concat(*quals, subquals);
*preds = list_concat(*preds, subpreds);
}
}
else if (IsA(bitmapqual, BitmapOrPath))
@ -747,36 +788,36 @@ pull_indexpath_quals(Path *bitmapqual)
foreach(l, opath->bitmapquals)
{
List *sublist;
List *subquals;
List *subpreds;
sublist = pull_indexpath_quals((Path *) lfirst(l));
result = list_concat(result, sublist);
find_indexpath_quals((Path *) lfirst(l), &subquals, &subpreds);
*quals = list_concat(*quals, subquals);
*preds = list_concat(*preds, subpreds);
}
}
else if (IsA(bitmapqual, IndexPath))
{
IndexPath *ipath = (IndexPath *) bitmapqual;
result = get_actual_clauses(ipath->indexclauses);
result = list_concat(result, list_copy(ipath->indexinfo->indpred));
*quals = get_actual_clauses(ipath->indexclauses);
*preds = list_copy(ipath->indexinfo->indpred);
}
else
elog(ERROR, "unrecognized node type: %d", nodeTag(bitmapqual));
return result;
}
/*
* lists_intersect_ptr
* lists_intersect
* Detect whether two lists have a nonempty intersection,
* using pointer equality to compare members.
* using equal() 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)
lists_intersect(List *list1, List *list2)
{
ListCell *cell1;
@ -787,7 +828,7 @@ lists_intersect_ptr(List *list1, List *list2)
foreach(cell2, list2)
{
if (lfirst(cell2) == datum1)
if (equal(lfirst(cell2), datum1))
return true;
}
}

51
src/backend/utils/adt/selfuncs.c
Unescape View File

@ -15,7 +15,7 @@
*
*
* IDENTIFICATION
* $PostgreSQL: pgsql/src/backend/utils/adt/selfuncs.c,v 1.229 2007/03/17 00:11:05 tgl Exp $
* $PostgreSQL: pgsql/src/backend/utils/adt/selfuncs.c,v 1.230 2007/03/21 22:18:12 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@ -86,6 +86,7 @@
#include "optimizer/pathnode.h"
#include "optimizer/paths.h"
#include "optimizer/plancat.h"
#include "optimizer/predtest.h"
#include "optimizer/restrictinfo.h"
#include "optimizer/var.h"
#include "parser/parse_coerce.h"
@ -4775,36 +4776,38 @@ genericcostestimate(PlannerInfo *root,
List *selectivityQuals;
ListCell *l;
/*
/*----------
* If the index is partial, AND the index predicate with the explicitly
* given indexquals to produce a more accurate idea of the index
* selectivity. This may produce redundant clauses. We get rid of exact
* duplicates in the code below. We expect that most cases of partial
* redundancy (such as "x < 4" from the qual and "x < 5" from the
* predicate) will be recognized and handled correctly by
* clauselist_selectivity(). This assumption is somewhat fragile, since
* it depends on predicate_implied_by() and clauselist_selectivity()
* having similar capabilities, and there are certainly many cases where
* we will end up with a too-low selectivity estimate. This will bias the
* system in favor of using partial indexes where possible, which is not
* necessarily a bad thing. But it'd be nice to do better someday.
* selectivity. However, we need to be careful not to insert redundant
* clauses, because clauselist_selectivity() is easily fooled into
* computing a too-low selectivity estimate. Our approach is to add
* only the index predicate clause(s) that cannot be proven to be implied
* by the given indexquals. This successfully handles cases such as a
* qual "x = 42" used with a partial index "WHERE x >= 40 AND x < 50".
* There are many other cases where we won't detect redundancy, leading
* to a too-low selectivity estimate, which will bias the system in favor
* of using partial indexes where possible. That is not necessarily bad
* though.
*
* Note that index->indpred and indexQuals are both in implicit-AND form,
* so ANDing them together just takes merging the lists. However,
* eliminating duplicates is a bit trickier because indexQuals contains
* RestrictInfo nodes and the indpred does not. It is okay to pass a
* mixed list to clauselist_selectivity, but we have to work a bit to
* generate a list without logical duplicates. (We could just list_union
* indpred and strippedQuals, but then we'd not get caching of per-qual
* selectivity estimates.)
* Note that indexQuals contains RestrictInfo nodes while the indpred
* does not. This is OK for both predicate_implied_by() and
* clauselist_selectivity().
*----------
*/
if (index->indpred != NIL)
{
List *strippedQuals;
List *predExtraQuals;
List *predExtraQuals = NIL;
foreach(l, index->indpred)
{
Node *predQual = (Node *) lfirst(l);
List *oneQual = list_make1(predQual);
strippedQuals = get_actual_clauses(indexQuals);
predExtraQuals = list_difference(index->indpred, strippedQuals);
if (!predicate_implied_by(oneQual, indexQuals))
predExtraQuals = list_concat(predExtraQuals, oneQual);
}
/* list_concat avoids modifying the passed-in indexQuals list */
selectivityQuals = list_concat(predExtraQuals, indexQuals);
}
else

9
src/test/regress/expected/select.out
Unescape View File

@ -209,9 +209,13 @@ SELECT onek.unique1, onek.string4 FROM onek
-- test partial btree indexes
--
-- As of 7.2, planner probably won't pick an indexscan without stats,
-- so ANALYZE first.
-- so ANALYZE first. Also, we want to prevent it from picking a bitmapscan
-- followed by sort, because that could hide index ordering problems.
--
ANALYZE onek2;
SET enable_seqscan TO off;
SET enable_bitmapscan TO off;
SET enable_sort TO off;
--
-- awk '{if($1<10){print $0;}else{next;}}' onek.data | sort +0n -1
--
@ -288,6 +292,9 @@ SELECT onek2.unique1, onek2.stringu1 FROM onek2
999 | LMAAAA
(19 rows)
RESET enable_seqscan;
RESET enable_bitmapscan;
RESET enable_sort;
SELECT two, stringu1, ten, string4
INTO TABLE tmp
FROM onek;

11
src/test/regress/sql/select.sql
Unescape View File

@ -59,10 +59,15 @@ SELECT onek.unique1, onek.string4 FROM onek
-- test partial btree indexes
--
-- As of 7.2, planner probably won't pick an indexscan without stats,
-- so ANALYZE first.
-- so ANALYZE first. Also, we want to prevent it from picking a bitmapscan
-- followed by sort, because that could hide index ordering problems.
--
ANALYZE onek2;
SET enable_seqscan TO off;
SET enable_bitmapscan TO off;
SET enable_sort TO off;
--
-- awk '{if($1<10){print $0;}else{next;}}' onek.data | sort +0n -1
--
@ -81,6 +86,10 @@ SELECT onek2.unique1, onek2.stringu1 FROM onek2
SELECT onek2.unique1, onek2.stringu1 FROM onek2
WHERE onek2.unique1 > 980;
RESET enable_seqscan;
RESET enable_bitmapscan;
RESET enable_sort;
SELECT two, stringu1, ten, string4
INTO TABLE tmp

Write Preview
Loading…
Cancel
Save