From 4e957954d6d14967db69d4e08f5fcc6d82d1f9fe Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Tue, 31 Jul 2007 19:54:01 +0000 Subject: [PATCH] Fix a bug in the original implementation of redundant-join-clause removal: clauses in which one side or the other references both sides of the join cannot be removed as redundant, because that expression won't have been constrained below the join. Per report from Sergey Burladyan. --- src/backend/optimizer/path/indxpath.c | 4 ++- src/backend/optimizer/plan/createplan.c | 10 ++++--- src/backend/optimizer/util/relnode.c | 4 ++- src/backend/optimizer/util/restrictinfo.c | 32 ++++++++++++++++++++--- src/include/optimizer/restrictinfo.h | 6 ++++- src/test/regress/expected/join.out | 16 ++++++++++++ src/test/regress/expected/join_1.out | 16 ++++++++++++ src/test/regress/sql/join.sql | 16 ++++++++++++ 8 files changed, 95 insertions(+), 9 deletions(-) diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c index 12ec16e2aec..2cd8ca0c138 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.11 2007/05/22 01:40:52 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/optimizer/path/indxpath.c,v 1.191.2.12 2007/07/31 19:54:00 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -1542,6 +1542,8 @@ find_clauses_for_join(PlannerInfo *root, RelOptInfo *rel, { clause_list = remove_redundant_join_clauses(root, clause_list, + outer_relids, + rel->relids, isouterjoin); } diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c index eba6d9393b2..730f9cc3d38 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.202.2.4 2006/05/18 18:57:37 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/optimizer/plan/createplan.c,v 1.202.2.5 2007/07/31 19:54:00 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -1290,7 +1290,9 @@ create_nestloop_plan(PlannerInfo *root, select_nonredundant_join_clauses(root, joinrestrictclauses, innerpath->indexclauses, - IS_OUTER_JOIN(best_path->jointype)); + best_path->outerjoinpath->parent->relids, + best_path->innerjoinpath->parent->relids, + IS_OUTER_JOIN(best_path->jointype)); } } else if (IsA(best_path->innerjoinpath, BitmapHeapPath)) @@ -1322,7 +1324,9 @@ create_nestloop_plan(PlannerInfo *root, select_nonredundant_join_clauses(root, joinrestrictclauses, bitmapclauses, - IS_OUTER_JOIN(best_path->jointype)); + best_path->outerjoinpath->parent->relids, + best_path->innerjoinpath->parent->relids, + IS_OUTER_JOIN(best_path->jointype)); } } diff --git a/src/backend/optimizer/util/relnode.c b/src/backend/optimizer/util/relnode.c index 20b656e0b30..28a7b6c6caa 100644 --- a/src/backend/optimizer/util/relnode.c +++ b/src/backend/optimizer/util/relnode.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/optimizer/util/relnode.c,v 1.72.2.1 2005/11/22 18:23:12 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/optimizer/util/relnode.c,v 1.72.2.2 2007/07/31 19:54:00 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -540,6 +540,8 @@ build_joinrel_restrictlist(PlannerInfo *root, * omit the redundant clause from the result list. */ result = remove_redundant_join_clauses(root, rlist, + outer_rel->relids, + inner_rel->relids, IS_OUTER_JOIN(jointype)); list_free(rlist); diff --git a/src/backend/optimizer/util/restrictinfo.c b/src/backend/optimizer/util/restrictinfo.c index 8257c69c32e..baeb2eb0a05 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.41.2.4 2006/04/07 17:05:47 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/optimizer/util/restrictinfo.c,v 1.41.2.5 2007/07/31 19:54:00 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -34,6 +34,8 @@ static Expr *make_sub_restrictinfos(Expr *clause, static RestrictInfo *join_clause_is_redundant(PlannerInfo *root, RestrictInfo *rinfo, List *reference_list, + Relids outer_relids, + Relids inner_relids, bool isouterjoin); @@ -473,6 +475,8 @@ get_actual_join_clauses(List *restrictinfo_list, */ List * remove_redundant_join_clauses(PlannerInfo *root, List *restrictinfo_list, + Relids outer_relids, + Relids inner_relids, bool isouterjoin) { List *result = NIL; @@ -499,7 +503,9 @@ remove_redundant_join_clauses(PlannerInfo *root, List *restrictinfo_list, RestrictInfo *prevrinfo; /* is it redundant with any prior clause? */ - prevrinfo = join_clause_is_redundant(root, rinfo, result, isouterjoin); + prevrinfo = join_clause_is_redundant(root, rinfo, result, + outer_relids, inner_relids, + isouterjoin); if (prevrinfo == NULL) { /* no, so add it to result list */ @@ -535,6 +541,8 @@ List * select_nonredundant_join_clauses(PlannerInfo *root, List *restrictinfo_list, List *reference_list, + Relids outer_relids, + Relids inner_relids, bool isouterjoin) { List *result = NIL; @@ -545,7 +553,9 @@ select_nonredundant_join_clauses(PlannerInfo *root, RestrictInfo *rinfo = (RestrictInfo *) lfirst(item); /* drop it if redundant with any reference clause */ - if (join_clause_is_redundant(root, rinfo, reference_list, isouterjoin) != NULL) + if (join_clause_is_redundant(root, rinfo, reference_list, + outer_relids, inner_relids, + isouterjoin) != NULL) continue; /* otherwise, add it to result list */ @@ -578,6 +588,12 @@ select_nonredundant_join_clauses(PlannerInfo *root, * of the latter, even though they might seem redundant by the pathkey * membership test. * + * Also, we cannot eliminate clauses wherein one side mentions vars from + * both relations, as in "WHERE t1.f1 = t2.f1 AND t1.f1 = t1.f2 - t2.f2". + * In this example, "t1.f2 - t2.f2" could not have been computed at all + * before forming the join of t1 and t2, so it certainly wasn't constrained + * earlier. + * * Weird special case: if we have two clauses that seem redundant * except one is pushed down into an outer join and the other isn't, * then they're not really redundant, because one constrains the @@ -587,6 +603,8 @@ static RestrictInfo * join_clause_is_redundant(PlannerInfo *root, RestrictInfo *rinfo, List *reference_list, + Relids outer_relids, + Relids inner_relids, bool isouterjoin) { ListCell *refitem; @@ -608,6 +626,14 @@ join_clause_is_redundant(PlannerInfo *root, bms_is_empty(rinfo->right_relids)) return NULL; /* var = const, so not redundant */ + /* check for either side mentioning both rels */ + if (bms_overlap(rinfo->left_relids, outer_relids) && + bms_overlap(rinfo->left_relids, inner_relids)) + return NULL; /* clause LHS uses both, so not redundant */ + if (bms_overlap(rinfo->right_relids, outer_relids) && + bms_overlap(rinfo->right_relids, inner_relids)) + return NULL; /* clause RHS uses both, so not redundant */ + cache_mergeclause_pathkeys(root, rinfo); foreach(refitem, reference_list) diff --git a/src/include/optimizer/restrictinfo.h b/src/include/optimizer/restrictinfo.h index e19b5a67627..4f298b15e92 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.34.2.1 2005/11/14 23:54:36 tgl Exp $ + * $PostgreSQL: pgsql/src/include/optimizer/restrictinfo.h,v 1.34.2.2 2007/07/31 19:54:00 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -30,10 +30,14 @@ extern void get_actual_join_clauses(List *restrictinfo_list, List **joinquals, List **otherquals); extern List *remove_redundant_join_clauses(PlannerInfo *root, List *restrictinfo_list, + Relids outer_relids, + Relids inner_relids, bool isouterjoin); extern List *select_nonredundant_join_clauses(PlannerInfo *root, List *restrictinfo_list, List *reference_list, + Relids outer_relids, + Relids inner_relids, bool isouterjoin); #endif /* RESTRICTINFO_H */ diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out index a6ccd7502eb..5d228af265e 100644 --- a/src/test/regress/expected/join.out +++ b/src/test/regress/expected/join.out @@ -2225,3 +2225,19 @@ select tt1.*, tt2.* from tt2 right join tt1 on tt1.joincol = tt2.joincol; 2 | | | (3 rows) +-- +-- regression test for problems of the sort depicted in bug #3494 +-- +create temp table tt5(f1 int, f2 int); +create temp table tt6(f1 int, f2 int); +insert into tt5 values(1, 10); +insert into tt5 values(1, 11); +insert into tt6 values(1, 9); +insert into tt6 values(1, 2); +insert into tt6 values(2, 9); +select * from tt5,tt6 where tt5.f1 = tt6.f1 and tt5.f1 = tt5.f2 - tt6.f2; + f1 | f2 | f1 | f2 +----+----+----+---- + 1 | 10 | 1 | 9 +(1 row) + diff --git a/src/test/regress/expected/join_1.out b/src/test/regress/expected/join_1.out index 97f2e31b561..ec20a1b1fcb 100644 --- a/src/test/regress/expected/join_1.out +++ b/src/test/regress/expected/join_1.out @@ -2225,3 +2225,19 @@ select tt1.*, tt2.* from tt2 right join tt1 on tt1.joincol = tt2.joincol; 2 | | | (3 rows) +-- +-- regression test for problems of the sort depicted in bug #3494 +-- +create temp table tt5(f1 int, f2 int); +create temp table tt6(f1 int, f2 int); +insert into tt5 values(1, 10); +insert into tt5 values(1, 11); +insert into tt6 values(1, 9); +insert into tt6 values(1, 2); +insert into tt6 values(2, 9); +select * from tt5,tt6 where tt5.f1 = tt6.f1 and tt5.f1 = tt5.f2 - tt6.f2; + f1 | f2 | f1 | f2 +----+----+----+---- + 1 | 10 | 1 | 9 +(1 row) + diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql index 737f71853bc..6a763620a0b 100644 --- a/src/test/regress/sql/join.sql +++ b/src/test/regress/sql/join.sql @@ -403,3 +403,19 @@ set enable_nestloop to off; select tt1.*, tt2.* from tt1 left join tt2 on tt1.joincol = tt2.joincol; select tt1.*, tt2.* from tt2 right join tt1 on tt1.joincol = tt2.joincol; + +-- +-- regression test for problems of the sort depicted in bug #3494 +-- + +create temp table tt5(f1 int, f2 int); +create temp table tt6(f1 int, f2 int); + +insert into tt5 values(1, 10); +insert into tt5 values(1, 11); + +insert into tt6 values(1, 9); +insert into tt6 values(1, 2); +insert into tt6 values(2, 9); + +select * from tt5,tt6 where tt5.f1 = tt6.f1 and tt5.f1 = tt5.f2 - tt6.f2;