From 1c01a5108a4f37bfa34e42629a8bf8b7a7191192 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Thu, 18 May 2006 19:56:56 +0000 Subject: [PATCH] Fix choose_bitmap_and() so that partial index predicates are considered when deciding whether a potential additional indexscan is redundant or not. As now coded, any use of a partial index that was already used in a previous AND arm will be rejected as redundant. This might be overly restrictive, but not considering the point at all is definitely bad, as per example in bug #2441 from Arjen van der Meijden. In particular, a clauseless scan of a partial index was *never* considered redundant by the previous coding, and that's surely wrong. Being more flexible would also require some consideration of how not to double-count the index predicate's selectivity. --- src/backend/optimizer/path/indxpath.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c index 3b77ceb3fc0..4e105f879e1 100644 --- 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.191.2.7 2006/04/09 18:18:59 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/optimizer/path/indxpath.c,v 1.191.2.8 2006/05/18 19:56:56 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -543,10 +543,10 @@ choose_bitmap_and(PlannerInfo *root, RelOptInfo *rel, List *paths) * 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 - * it makes the right choice, but perhaps we need to work harder. + * 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?) * * 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, @@ -659,14 +659,14 @@ bitmap_and_cost_est(PlannerInfo *root, RelOptInfo *rel, List *paths) /* * 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. + * Given the Path structure for a plain or bitmap indexscan, extract a list + * 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 RestrictInfos used in the Path, + * The result list contains 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). */ @@ -704,7 +704,8 @@ pull_indexpath_quals(Path *bitmapqual) { IndexPath *ipath = (IndexPath *) bitmapqual; - result = list_copy(ipath->indexclauses); + result = get_actual_clauses(ipath->indexclauses); + result = list_concat(result, list_copy(ipath->indexinfo->indpred)); } else elog(ERROR, "unrecognized node type: %d", nodeTag(bitmapqual));