From 1fcd4b7a07a5cfb00e064c4bdd984e52cde71a5c Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 25 Apr 2005 03:58:30 +0000 Subject: [PATCH] While determining the filter clauses for an index scan (either plain or bitmap), use pred_test to be a little smarter about cases where a filter clause is logically unnecessary. This may be overkill for the plain indexscan case, but it's definitely useful for OR'd bitmap scans. --- src/backend/optimizer/path/indxpath.c | 14 +++++-- src/backend/optimizer/plan/createplan.c | 52 +++++++++++++++++++------ 2 files changed, 51 insertions(+), 15 deletions(-) diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c index f55f7fc918..0a398849f8 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.178 2005/04/25 01:30:13 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/optimizer/path/indxpath.c,v 1.179 2005/04/25 03:58:29 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -500,8 +500,14 @@ choose_bitmap_and(Query *root, RelOptInfo *rel, List *paths) * And we consider an index redundant if all its index conditions were * already used by earlier indexes. (We could use pred_test() to have * a more intelligent, but much more expensive, check --- but in most - * cases simple equality should suffice, since after all the index - * conditions are all coming from the same query clauses.) + * cases simple pointer equality should suffice, since after all the + * index conditions are all coming from the same RestrictInfo lists.) + * + * 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. */ /* Convert list to array so we can apply qsort */ @@ -530,7 +536,7 @@ choose_bitmap_and(Query *root, RelOptInfo *rel, List *paths) if (IsA(newpath, IndexPath)) { newqual = ((IndexPath *) newpath)->indexclauses; - if (list_difference(newqual, qualsofar) == NIL) + if (list_difference_ptr(newqual, qualsofar) == NIL) continue; /* redundant */ } diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c index 8c06278f8f..3feff40423 100644 --- a/src/backend/optimizer/plan/createplan.c +++ b/src/backend/optimizer/plan/createplan.c @@ -10,7 +10,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/optimizer/plan/createplan.c,v 1.186 2005/04/25 02:14:47 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/optimizer/plan/createplan.c,v 1.187 2005/04/25 03:58:30 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -720,6 +720,7 @@ create_indexscan_plan(Query *root, List *nonlossy_indexquals; List *indexstrategy; List *indexsubtype; + ListCell *l; IndexScan *scan_plan; /* it should be a base rel... */ @@ -768,13 +769,31 @@ create_indexscan_plan(Query *root, * checked (either by the index itself, or by nodeIndexscan.c), but if * there are any "special" operators involved then they must be included * in qpqual. Also, any lossy index operators must be rechecked in - * the qpqual. The upshot is that qpquals must contain scan_clauses + * the qpqual. The upshot is that qpqual must contain scan_clauses * minus whatever appears in nonlossy_indexquals. + * + * In normal cases simple pointer equality checks will be enough to + * spot duplicate RestrictInfos, so we try that first. In some situations + * (particularly with OR'd index conditions) we may have scan_clauses + * that are not equal to, but are logically implied by, the index quals; + * so we also try a pred_test() check to see if we can discard quals + * that way. + * + * While at it, we strip off the RestrictInfos to produce a list of + * plain expressions. */ - qpqual = list_difference_ptr(scan_clauses, nonlossy_indexquals); + qpqual = NIL; + foreach(l, scan_clauses) + { + RestrictInfo *rinfo = (RestrictInfo *) lfirst(l); - /* Reduce RestrictInfo list to bare expressions */ - qpqual = get_actual_clauses(qpqual); + Assert(IsA(rinfo, RestrictInfo)); + if (list_member_ptr(nonlossy_indexquals, rinfo)) + continue; + if (pred_test(list_make1(rinfo->clause), nonlossy_indexquals)) + continue; + qpqual = lappend(qpqual, rinfo->clause); + } /* Sort clauses into best execution order */ qpqual = order_qual_clauses(root, qpqual); @@ -813,6 +832,7 @@ create_bitmap_scan_plan(Query *root, List *bitmapqualorig; List *indexquals; List *qpqual; + ListCell *l; BitmapHeapScan *scan_plan; /* it should be a base rel... */ @@ -848,13 +868,23 @@ create_bitmap_scan_plan(Query *root, * must be added to qpqual. The upshot is that qpquals must contain * scan_clauses minus whatever appears in indexquals. * - * NOTE: when there are OR clauses in indexquals, the simple equality - * check used by list_difference will only detect matches in case of - * chance equality of the OR subclause ordering. This is probably all - * right for now because that order will match what's in scan_clauses - * ... but perhaps we need more smarts here. + * In normal cases simple equal() checks will be enough to spot duplicate + * clauses, so we try that first. In some situations (particularly with + * OR'd index conditions) we may have scan_clauses that are not equal to, + * but are logically implied by, the index quals; so we also try a + * pred_test() check to see if we can discard quals that way. */ - qpqual = list_difference(scan_clauses, indexquals); + qpqual = NIL; + foreach(l, scan_clauses) + { + Node *clause = (Node *) lfirst(l); + + if (list_member(indexquals, clause)) + continue; + if (pred_test(list_make1(clause), indexquals)) + continue; + qpqual = lappend(qpqual, clause); + } /* Sort clauses into best execution order */ qpqual = order_qual_clauses(root, qpqual); -- 2.40.0