diff options
| -rw-r--r-- | src/backend/nodes/list.c | 160 | ||||
| -rw-r--r-- | src/backend/optimizer/path/indxpath.c | 110 | ||||
| -rw-r--r-- | src/backend/optimizer/path/orindxpath.c | 20 | ||||
| -rw-r--r-- | src/backend/optimizer/plan/createplan.c | 70 | ||||
| -rw-r--r-- | src/backend/optimizer/plan/planagg.c | 6 | ||||
| -rw-r--r-- | src/backend/optimizer/util/restrictinfo.c | 94 | ||||
| -rw-r--r-- | src/include/nodes/pg_list.h | 12 | ||||
| -rw-r--r-- | src/include/optimizer/paths.h | 5 | ||||
| -rw-r--r-- | src/include/optimizer/restrictinfo.h | 5 |
9 files changed, 399 insertions, 83 deletions
diff --git a/src/backend/nodes/list.c b/src/backend/nodes/list.c index b56cfe3791c..80043834b63 100644 --- a/src/backend/nodes/list.c +++ b/src/backend/nodes/list.c @@ -9,7 +9,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/nodes/list.c,v 1.64 2005/03/17 05:47:01 neilc Exp $ + * $PostgreSQL: pgsql/src/backend/nodes/list.c,v 1.65 2005/07/28 20:26:19 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -686,9 +686,13 @@ list_delete_first(List *list) * The returned list is newly-allocated, although the content of the * cells is the same (i.e. any pointed-to objects are not copied). * - * NB: Bizarrely, this function will NOT remove any duplicates that - * are present in list1 (so it is not really a union at all!). Also, - * this function could probably be implemented a lot faster if it is a + * NB: this function will NOT remove any duplicates that are present + * in list1 (so it only performs a "union" if list1 is known unique to + * start with). Also, if you are about to write "x = list_union(x, y)" + * you probably want to use list_concat_unique() instead to avoid wasting + * the list cells of the old x list. + * + * This function could probably be implemented a lot faster if it is a * performance bottleneck. */ List * @@ -888,7 +892,153 @@ list_difference_oid(List *list1, List *list2) return result; } -/* Free all storage in a list, and optionally the pointed-to elements */ +/* + * Append datum to list, but only if it isn't already in the list. + * + * Whether an element is already a member of the list is determined + * via equal(). + */ +List * +list_append_unique(List *list, void *datum) +{ + if (list_member(list, datum)) + return list; + else + return lappend(list, datum); +} + +/* + * This variant of list_append_unique() determines list membership via + * simple pointer equality. + */ +List * +list_append_unique_ptr(List *list, void *datum) +{ + if (list_member_ptr(list, datum)) + return list; + else + return lappend(list, datum); +} + +/* + * This variant of list_append_unique() operates upon lists of integers. + */ +List * +list_append_unique_int(List *list, int datum) +{ + if (list_member_int(list, datum)) + return list; + else + return lappend_int(list, datum); +} + +/* + * This variant of list_append_unique() operates upon lists of OIDs. + */ +List * +list_append_unique_oid(List *list, Oid datum) +{ + if (list_member_oid(list, datum)) + return list; + else + return lappend_oid(list, datum); +} + +/* + * Append to list1 each member of list2 that isn't already in list1. + * + * Whether an element is already a member of the list is determined + * via equal(). + * + * This is almost the same functionality as list_union(), but list1 is + * modified in-place rather than being copied. Note also that list2's cells + * are not inserted in list1, so the analogy to list_concat() isn't perfect. + */ +List * +list_concat_unique(List *list1, List *list2) +{ + ListCell *cell; + + Assert(IsPointerList(list1)); + Assert(IsPointerList(list2)); + + foreach(cell, list2) + { + if (!list_member(list1, lfirst(cell))) + list1 = lappend(list1, lfirst(cell)); + } + + check_list_invariants(list1); + return list1; +} + +/* + * This variant of list_concat_unique() determines list membership via + * simple pointer equality. + */ +List * +list_concat_unique_ptr(List *list1, List *list2) +{ + ListCell *cell; + + Assert(IsPointerList(list1)); + Assert(IsPointerList(list2)); + + foreach(cell, list2) + { + if (!list_member_ptr(list1, lfirst(cell))) + list1 = lappend(list1, lfirst(cell)); + } + + check_list_invariants(list1); + return list1; +} + +/* + * This variant of list_concat_unique() operates upon lists of integers. + */ +List * +list_concat_unique_int(List *list1, List *list2) +{ + ListCell *cell; + + Assert(IsIntegerList(list1)); + Assert(IsIntegerList(list2)); + + foreach(cell, list2) + { + if (!list_member_int(list1, lfirst_int(cell))) + list1 = lappend_int(list1, lfirst_int(cell)); + } + + check_list_invariants(list1); + return list1; +} + +/* + * This variant of list_concat_unique() operates upon lists of OIDs. + */ +List * +list_concat_unique_oid(List *list1, List *list2) +{ + ListCell *cell; + + Assert(IsOidList(list1)); + Assert(IsOidList(list2)); + + foreach(cell, list2) + { + if (!list_member_oid(list1, lfirst_oid(cell))) + list1 = lappend_oid(list1, lfirst_oid(cell)); + } + + check_list_invariants(list1); + return list1; +} + +/* + * Free all storage in a list, and optionally the pointed-to elements + */ static void list_free_private(List *list, bool deep) { diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c index 07b67a88228..bd0b09fcfe9 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.186 2005/07/02 23:00:40 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/optimizer/path/indxpath.c,v 1.187 2005/07/28 20:26:20 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -212,6 +212,8 @@ create_index_paths(PlannerInfo *root, RelOptInfo *rel) * subclause to any index on x alone, because such a Path would already have * been generated at the upper level. So we could use an index on x,y,z * or an index on x,y for the OR subclause, but not an index on just x. + * When dealing with a partial index, a match of the index predicate to + * one of the "current" clauses also makes the index usable. * * If istoplevel is true (indicating we are considering the top level of a * rel's restriction clauses), we will include indexes in the result that @@ -248,6 +250,8 @@ find_usable_indexes(PlannerInfo *root, RelOptInfo *rel, List *restrictclauses; List *index_pathkeys; List *useful_pathkeys; + bool useful_predicate; + bool found_clause; bool index_is_ordered; /* @@ -258,29 +262,60 @@ find_usable_indexes(PlannerInfo *root, RelOptInfo *rel, * Otherwise, we have to test whether the added clauses are * sufficient to imply the predicate. If so, we could use * the index in the current context. + * + * We set useful_predicate to true iff the predicate was proven + * using the current set of clauses. This is needed to prevent + * matching a 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 (index->indpred != NIL && !index->predOK) + useful_predicate = false; + if (index->indpred != NIL) { - if (istoplevel) - continue; /* no point in trying to prove it */ + if (index->predOK) + { + if (istoplevel) + { + /* we know predicate was proven from these clauses */ + useful_predicate = true; + } + } + else + { + if (istoplevel) + continue; /* no point in trying to prove it */ - /* Form all_clauses if not done already */ - if (all_clauses == NIL) - all_clauses = list_concat(list_copy(clauses), - outer_clauses); + /* Form all_clauses if not done already */ + if (all_clauses == NIL) + all_clauses = list_concat(list_copy(clauses), + outer_clauses); - if (!predicate_implied_by(index->indpred, all_clauses) || - predicate_implied_by(index->indpred, outer_clauses)) - continue; + if (!predicate_implied_by(index->indpred, all_clauses)) + continue; /* can't use it at all */ + + if (!predicate_implied_by(index->indpred, outer_clauses)) + useful_predicate = true; + } } /* * 1. Match the index against the available restriction clauses. + * found_clause is set true only if at least one of the current + * clauses was used. */ restrictclauses = group_clauses_by_indexkey(index, clauses, outer_clauses, - outer_relids); + outer_relids, + &found_clause); + + /* + * Not all index AMs support scans with no restriction clauses. + * We can't generate a scan over an index with amoptionalkey = false + * unless there's at least one restriction clause. + */ + if (restrictclauses == NIL && !index->amoptionalkey) + continue; /* * 2. Compute pathkeys describing index's ordering, if any, then @@ -300,20 +335,12 @@ find_usable_indexes(PlannerInfo *root, RelOptInfo *rel, /* * 3. Generate an indexscan path if there are relevant restriction - * clauses OR the index ordering is potentially useful for later - * merging or final output ordering. - * - * If there is a predicate, consider it anyway since the index - * predicate has already been found to match the query. The - * selectivity of the predicate might alone make the index useful. - * - * Note: not all index AMs support scans with no restriction clauses. - * We can't generate a scan over an index with amoptionalkey = false - * unless there's at least one restriction clause. + * clauses in the current clauses, OR the index ordering is + * potentially useful for later merging or final output ordering, + * OR the index has a predicate that was proven by the current + * clauses. */ - if (restrictclauses != NIL || - (index->amoptionalkey && - (useful_pathkeys != NIL || index->indpred != NIL))) + if (found_clause || useful_pathkeys != NIL || useful_predicate) { ipath = create_index_path(root, index, restrictclauses, @@ -627,10 +654,9 @@ bitmap_and_cost_est(PlannerInfo *root, RelOptInfo *rel, List *paths) * with one index key (in no particular order); the top list is ordered by * index key. (This is depended on by expand_indexqual_conditions().) * - * As explained in the comments for find_usable_indexes(), we can use - * clauses from either of the given lists, but the result is required to - * use at least one clause from the "current clauses" list. We return - * NIL if we don't find any such clause. + * We can use clauses from either the current clauses or outer_clauses lists, + * but *found_clause is set TRUE only if we used at least one clause from + * the "current clauses" list. See find_usable_indexes() for motivation. * * outer_relids determines what Vars will be allowed on the other side * of a possible index qual; see match_clause_to_indexcol(). @@ -643,18 +669,25 @@ bitmap_and_cost_est(PlannerInfo *root, RelOptInfo *rel, List *paths) * Example: given an index on (A,B,C), we would return ((C1 C2) () (C3 C4)) * if we find that clauses C1 and C2 use column A, clauses C3 and C4 use * column C, and no clauses use column B. + * + * Note: in some circumstances we may find the same RestrictInfos coming + * from multiple places. Defend against redundant outputs by using + * list_append_unique_ptr (pointer equality should be good enough). */ List * group_clauses_by_indexkey(IndexOptInfo *index, List *clauses, List *outer_clauses, - Relids outer_relids) + Relids outer_relids, + bool *found_clause) { List *clausegroup_list = NIL; - bool found_clause = false; + bool found_outer_clause = false; int indexcol = 0; Oid *classes = index->classlist; - if (clauses == NIL) + *found_clause = false; /* default result */ + + if (clauses == NIL && outer_clauses == NIL) return NIL; /* cannot succeed */ do @@ -675,8 +708,8 @@ group_clauses_by_indexkey(IndexOptInfo *index, rinfo, outer_relids)) { - clausegroup = lappend(clausegroup, rinfo); - found_clause = true; + clausegroup = list_append_unique_ptr(clausegroup, rinfo); + *found_clause = true; } } @@ -691,7 +724,10 @@ group_clauses_by_indexkey(IndexOptInfo *index, curClass, rinfo, outer_relids)) - clausegroup = lappend(clausegroup, rinfo); + { + clausegroup = list_append_unique_ptr(clausegroup, rinfo); + found_outer_clause = true; + } } /* @@ -707,8 +743,8 @@ group_clauses_by_indexkey(IndexOptInfo *index, } while (!DoneMatchingIndexKeys(classes)); - if (!found_clause) - return NIL; + if (!*found_clause && !found_outer_clause) + return NIL; /* no indexable clauses anywhere */ return clausegroup_list; } diff --git a/src/backend/optimizer/path/orindxpath.c b/src/backend/optimizer/path/orindxpath.c index 884329edd97..eb1e1a6ffcd 100644 --- a/src/backend/optimizer/path/orindxpath.c +++ b/src/backend/optimizer/path/orindxpath.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/optimizer/path/orindxpath.c,v 1.73 2005/07/02 23:00:40 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/optimizer/path/orindxpath.c,v 1.74 2005/07/28 20:26:20 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -134,14 +134,24 @@ create_or_index_quals(PlannerInfo *root, RelOptInfo *rel) return false; /* - * Convert the path's indexclauses structure to a RestrictInfo tree, - * and add it to the rel's restriction list. + * Convert the path's indexclauses structure to a RestrictInfo tree. + * We include any partial-index predicates so as to get a reasonable + * representation of what the path is actually scanning. */ - newrinfos = make_restrictinfo_from_bitmapqual((Path *) bestpath, true); - Assert(list_length(newrinfos) == 1); + newrinfos = make_restrictinfo_from_bitmapqual((Path *) bestpath, + true, true); + + /* It's possible we get back something other than a single OR clause */ + if (list_length(newrinfos) != 1) + return false; or_rinfo = (RestrictInfo *) linitial(newrinfos); Assert(IsA(or_rinfo, RestrictInfo)); + if (!restriction_is_or_clause(or_rinfo)) + return false; + /* + * OK, add it to the rel's restriction list. + */ rel->baserestrictinfo = list_concat(rel->baserestrictinfo, newrinfos); /* diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c index 6c4c345ca8e..697355cfdf8 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.195 2005/07/23 21:05:46 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/optimizer/plan/createplan.c,v 1.196 2005/07/28 20:26:21 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -900,7 +900,7 @@ create_bitmap_scan_plan(PlannerInfo *root, */ if (best_path->isjoininner) { - scan_clauses = list_union(scan_clauses, bitmapqualorig); + scan_clauses = list_concat_unique(scan_clauses, bitmapqualorig); } /* @@ -962,6 +962,9 @@ create_bitmap_scan_plan(PlannerInfo *root, * (in implicit-AND form, without RestrictInfos) describing the original index * conditions and the generated indexqual conditions. The latter is made to * exclude lossy index operators. + * + * Note: if you find yourself changing this, you probably need to change + * make_restrictinfo_from_bitmapqual too. */ static Plan * create_bitmap_subplan(PlannerInfo *root, Path *bitmapqual, @@ -977,6 +980,13 @@ create_bitmap_subplan(PlannerInfo *root, Path *bitmapqual, List *subindexquals = NIL; ListCell *l; + /* + * There may well be redundant quals among the subplans, since a + * top-level WHERE qual might have gotten used to form several + * different index quals. We don't try exceedingly hard to + * eliminate redundancies, but we do eliminate obvious duplicates + * by using list_concat_unique. + */ foreach(l, apath->bitmapquals) { Plan *subplan; @@ -986,8 +996,8 @@ create_bitmap_subplan(PlannerInfo *root, Path *bitmapqual, subplan = create_bitmap_subplan(root, (Path *) lfirst(l), &subqual, &subindexqual); subplans = lappend(subplans, subplan); - subquals = list_concat(subquals, subqual); - subindexquals = list_concat(subindexquals, subindexqual); + subquals = list_concat_unique(subquals, subqual); + subindexquals = list_concat_unique(subindexquals, subindexqual); } plan = (Plan *) make_bitmap_and(subplans); plan->startup_cost = apath->path.startup_cost; @@ -1004,8 +1014,15 @@ create_bitmap_subplan(PlannerInfo *root, Path *bitmapqual, List *subplans = NIL; List *subquals = NIL; List *subindexquals = NIL; + bool const_true_subqual = false; + bool const_true_subindexqual = false; ListCell *l; + /* + * Here, we detect both obvious redundancies and qual-free subplans. + * A qual-free subplan would cause us to generate "... OR true ..." + * which we may as well reduce to just "true". + */ foreach(l, opath->bitmapquals) { Plan *subplan; @@ -1015,10 +1032,16 @@ create_bitmap_subplan(PlannerInfo *root, Path *bitmapqual, subplan = create_bitmap_subplan(root, (Path *) lfirst(l), &subqual, &subindexqual); subplans = lappend(subplans, subplan); - subquals = lappend(subquals, - make_ands_explicit(subqual)); - subindexquals = lappend(subindexquals, - make_ands_explicit(subindexqual)); + if (subqual == NIL) + const_true_subqual = true; + else if (!const_true_subqual) + subquals = list_append_unique(subquals, + make_ands_explicit(subqual)); + if (subindexqual == NIL) + const_true_subindexqual = true; + else if (!const_true_subindexqual) + subindexquals = list_append_unique(subindexquals, + make_ands_explicit(subindexqual)); } plan = (Plan *) make_bitmap_or(subplans); plan->startup_cost = opath->path.startup_cost; @@ -1026,8 +1049,23 @@ create_bitmap_subplan(PlannerInfo *root, Path *bitmapqual, plan->plan_rows = clamp_row_est(opath->bitmapselectivity * opath->path.parent->tuples); plan->plan_width = 0; /* meaningless */ - *qual = list_make1(make_orclause(subquals)); - *indexqual = list_make1(make_orclause(subindexquals)); + /* + * If there were constant-TRUE subquals, the OR reduces to constant + * TRUE. Also, avoid generating one-element ORs, which could happen + * due to redundancy elimination. + */ + if (const_true_subqual) + *qual = NIL; + else if (list_length(subquals) <= 1) + *qual = subquals; + else + *qual = list_make1(make_orclause(subquals)); + if (const_true_subindexqual) + *indexqual = NIL; + else if (list_length(subindexquals) <= 1) + *indexqual = subindexquals; + else + *indexqual = list_make1(make_orclause(subindexquals)); } else if (IsA(bitmapqual, IndexPath)) { @@ -1204,6 +1242,14 @@ create_nestloop_plan(PlannerInfo *root, { /* * Same deal for bitmapped index scans. + * + * Note: both here and above, we ignore any implicit index restrictions + * associated with the use of partial indexes. This is OK because + * we're only trying to prove we can dispense with some join quals; + * failing to prove that doesn't result in an incorrect plan. It is + * the right way to proceed because adding more quals to the stuff + * we got from the original query would just make it harder to detect + * duplication. */ BitmapHeapPath *innerpath = (BitmapHeapPath *) best_path->innerjoinpath; @@ -1212,7 +1258,9 @@ create_nestloop_plan(PlannerInfo *root, List *bitmapclauses; bitmapclauses = - make_restrictinfo_from_bitmapqual(innerpath->bitmapqual, true); + make_restrictinfo_from_bitmapqual(innerpath->bitmapqual, + true, + false); joinrestrictclauses = select_nonredundant_join_clauses(root, joinrestrictclauses, diff --git a/src/backend/optimizer/plan/planagg.c b/src/backend/optimizer/plan/planagg.c index 0208e910536..81f526ac113 100644 --- a/src/backend/optimizer/plan/planagg.c +++ b/src/backend/optimizer/plan/planagg.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/optimizer/plan/planagg.c,v 1.6 2005/07/23 21:05:46 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/optimizer/plan/planagg.c,v 1.7 2005/07/28 20:26:21 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -313,6 +313,7 @@ build_minmax_path(PlannerInfo *root, RelOptInfo *rel, MinMaxAggInfo *info) List *restrictclauses; IndexPath *new_path; Cost new_cost; + bool found_clause; /* Ignore non-btree indexes */ if (index->relam != BTREE_AM_OID) @@ -346,7 +347,8 @@ build_minmax_path(PlannerInfo *root, RelOptInfo *rel, MinMaxAggInfo *info) restrictclauses = group_clauses_by_indexkey(index, index->rel->baserestrictinfo, NIL, - NULL); + NULL, + &found_clause); if (list_length(restrictclauses) < indexcol) continue; /* definitely haven't got enough */ diff --git a/src/backend/optimizer/util/restrictinfo.c b/src/backend/optimizer/util/restrictinfo.c index ba3d82dd307..deb16152fb8 100644 --- a/src/backend/optimizer/util/restrictinfo.c +++ b/src/backend/optimizer/util/restrictinfo.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/optimizer/util/restrictinfo.c,v 1.38 2005/07/02 23:00:41 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/optimizer/util/restrictinfo.c,v 1.39 2005/07/28 20:26:21 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -17,6 +17,7 @@ #include "optimizer/clauses.h" #include "optimizer/cost.h" #include "optimizer/paths.h" +#include "optimizer/predtest.h" #include "optimizer/restrictinfo.h" #include "optimizer/var.h" @@ -70,31 +71,49 @@ make_restrictinfo(Expr *clause, bool is_pushed_down, Relids required_relids) * RestrictInfo node(s) equivalent to the condition represented by the * indexclauses of the Path structure. * - * The result is a List since we might need to return multiple RestrictInfos. + * The result is a List (effectively, implicit-AND representation) of + * RestrictInfos. + * + * If include_predicates is true, we add any partial index predicates to + * the explicit index quals. When this is not true, we return a condition + * that might be weaker than the actual scan represents. * * To do this through the normal make_restrictinfo() API, callers would have * to strip off the RestrictInfo nodes present in the indexclauses lists, and * then make_restrictinfo() would have to build new ones. It's better to have * a specialized routine to allow sharing of RestrictInfos. + * + * The qual manipulations here are much the same as in create_bitmap_subplan; + * keep the two routines in sync! */ List * -make_restrictinfo_from_bitmapqual(Path *bitmapqual, bool is_pushed_down) +make_restrictinfo_from_bitmapqual(Path *bitmapqual, + bool is_pushed_down, + bool include_predicates) { List *result; + ListCell *l; if (IsA(bitmapqual, BitmapAndPath)) { BitmapAndPath *apath = (BitmapAndPath *) bitmapqual; - ListCell *l; + /* + * There may well be redundant quals among the subplans, since a + * top-level WHERE qual might have gotten used to form several + * different index quals. We don't try exceedingly hard to + * eliminate redundancies, but we do eliminate obvious duplicates + * by using list_concat_unique. + */ result = NIL; foreach(l, apath->bitmapquals) { List *sublist; sublist = make_restrictinfo_from_bitmapqual((Path *) lfirst(l), - is_pushed_down); - result = list_concat(result, sublist); + is_pushed_down, + include_predicates); + result = list_concat_unique(result, sublist); } } else if (IsA(bitmapqual, BitmapOrPath)) @@ -102,38 +121,77 @@ make_restrictinfo_from_bitmapqual(Path *bitmapqual, bool is_pushed_down) BitmapOrPath *opath = (BitmapOrPath *) bitmapqual; List *withris = NIL; List *withoutris = NIL; - ListCell *l; + /* + * Here, we detect both obvious redundancies and qual-free subplans. + * A qual-free subplan would cause us to generate "... OR true ..." + * which we may as well reduce to just "true". + */ foreach(l, opath->bitmapquals) { List *sublist; sublist = make_restrictinfo_from_bitmapqual((Path *) lfirst(l), - is_pushed_down); + is_pushed_down, + include_predicates); if (sublist == NIL) { - /* constant TRUE input yields constant TRUE OR result */ - /* (though this probably cannot happen) */ + /* + * If we find a qual-less subscan, it represents a constant + * TRUE, and hence the OR result is also constant TRUE, so + * we can stop here. + */ return NIL; } /* Create AND subclause with RestrictInfos */ - withris = lappend(withris, make_ands_explicit(sublist)); + withris = list_append_unique(withris, + make_ands_explicit(sublist)); /* And one without */ sublist = get_actual_clauses(sublist); - withoutris = lappend(withoutris, make_ands_explicit(sublist)); + withoutris = list_append_unique(withoutris, + make_ands_explicit(sublist)); + } + + /* + * Avoid generating one-element ORs, which could happen + * due to redundancy elimination. + */ + if (list_length(withris) <= 1) + result = withris; + else + { + /* Here's the magic part not available to outside callers */ + result = + list_make1(make_restrictinfo_internal(make_orclause(withoutris), + make_orclause(withris), + is_pushed_down, + NULL)); } - /* Here's the magic part not available to outside callers */ - result = - list_make1(make_restrictinfo_internal(make_orclause(withoutris), - make_orclause(withris), - is_pushed_down, - NULL)); } else if (IsA(bitmapqual, IndexPath)) { IndexPath *ipath = (IndexPath *) bitmapqual; result = list_copy(ipath->indexclauses); + if (include_predicates && ipath->indexinfo->indpred != NIL) + { + foreach(l, ipath->indexinfo->indpred) + { + Expr *pred = (Expr *) lfirst(l); + + /* + * We know that the index predicate must have been implied + * by the query condition as a whole, but it may or may not + * be implied by the conditions that got pushed into the + * bitmapqual. Avoid generating redundant conditions. + */ + if (!predicate_implied_by(list_make1(pred), result)) + result = lappend(result, + make_restrictinfo(pred, + is_pushed_down, + NULL)); + } + } } else { diff --git a/src/include/nodes/pg_list.h b/src/include/nodes/pg_list.h index d54337d8148..564745e9a59 100644 --- a/src/include/nodes/pg_list.h +++ b/src/include/nodes/pg_list.h @@ -30,7 +30,7 @@ * Portions Copyright (c) 1996-2005, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/include/nodes/pg_list.h,v 1.51 2004/12/31 22:03:34 pgsql Exp $ + * $PostgreSQL: pgsql/src/include/nodes/pg_list.h,v 1.52 2005/07/28 20:26:22 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -224,6 +224,16 @@ extern List *list_difference_ptr(List *list1, List *list2); extern List *list_difference_int(List *list1, List *list2); extern List *list_difference_oid(List *list1, List *list2); +extern List *list_append_unique(List *list, void *datum); +extern List *list_append_unique_ptr(List *list, void *datum); +extern List *list_append_unique_int(List *list, int datum); +extern List *list_append_unique_oid(List *list, Oid datum); + +extern List *list_concat_unique(List *list1, List *list2); +extern List *list_concat_unique_ptr(List *list1, List *list2); +extern List *list_concat_unique_int(List *list1, List *list2); +extern List *list_concat_unique_oid(List *list1, List *list2); + extern void list_free(List *list); extern void list_free_deep(List *list); diff --git a/src/include/optimizer/paths.h b/src/include/optimizer/paths.h index be46bf53dd4..8f26c8e8f0d 100644 --- a/src/include/optimizer/paths.h +++ b/src/include/optimizer/paths.h @@ -7,7 +7,7 @@ * Portions Copyright (c) 1996-2005, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/include/optimizer/paths.h,v 1.85 2005/06/10 22:25:37 tgl Exp $ + * $PostgreSQL: pgsql/src/include/optimizer/paths.h,v 1.86 2005/07/28 20:26:22 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -43,7 +43,8 @@ extern Path *best_inner_indexscan(PlannerInfo *root, RelOptInfo *rel, Relids outer_relids, JoinType jointype); extern List *group_clauses_by_indexkey(IndexOptInfo *index, List *clauses, List *outer_clauses, - Relids outer_relids); + Relids outer_relids, + bool *found_clause); extern bool match_index_to_operand(Node *operand, int indexcol, IndexOptInfo *index); extern List *expand_indexqual_conditions(IndexOptInfo *index, diff --git a/src/include/optimizer/restrictinfo.h b/src/include/optimizer/restrictinfo.h index 5a9c2f27224..527c5f500a5 100644 --- a/src/include/optimizer/restrictinfo.h +++ b/src/include/optimizer/restrictinfo.h @@ -7,7 +7,7 @@ * Portions Copyright (c) 1996-2005, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/include/optimizer/restrictinfo.h,v 1.32 2005/07/02 23:00:42 tgl Exp $ + * $PostgreSQL: pgsql/src/include/optimizer/restrictinfo.h,v 1.33 2005/07/28 20:26:22 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -21,7 +21,8 @@ extern RestrictInfo *make_restrictinfo(Expr *clause, bool is_pushed_down, Relids required_relids); extern List *make_restrictinfo_from_bitmapqual(Path *bitmapqual, - bool is_pushed_down); + bool is_pushed_down, + bool include_predicates); extern bool restriction_is_or_clause(RestrictInfo *restrictinfo); extern List *get_actual_clauses(List *restrictinfo_list); extern void get_actual_join_clauses(List *restrictinfo_list, |