diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c index 28182c2372..0403fd4c92 100644 --- a/src/backend/optimizer/path/indxpath.c +++ b/src/backend/optimizer/path/indxpath.c @@ -9,7 +9,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/optimizer/path/indxpath.c,v 1.147.4.2 2005/12/06 16:59:22 tgl Exp $ + * $Header: /cvsroot/pgsql/src/backend/optimizer/path/indxpath.c,v 1.147.4.3 2007/07/31 19:54:26 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -656,7 +656,9 @@ group_clauses_by_indexkey_for_join(Query *root, List *nl; nl = remove_redundant_join_clauses(root, - FastListValue(&clausegroup), + FastListValue(&clausegroup), + outer_relids, + rel->relids, jointype); FastListFromList(&clausegroup, nl); } diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c index bbbc013fff..a28d195e41 100644 --- a/src/backend/optimizer/plan/createplan.c +++ b/src/backend/optimizer/plan/createplan.c @@ -10,7 +10,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/optimizer/plan/createplan.c,v 1.157.2.3 2004/02/29 17:36:48 tgl Exp $ + * $Header: /cvsroot/pgsql/src/backend/optimizer/plan/createplan.c,v 1.157.2.4 2007/07/31 19:54:26 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -927,6 +927,8 @@ create_nestloop_plan(Query *root, select_nonredundant_join_clauses(root, joinrestrictclauses, lfirst(indexjoinclauses), + best_path->outerjoinpath->parent->relids, + best_path->innerjoinpath->parent->relids, best_path->jointype); } } diff --git a/src/backend/optimizer/util/relnode.c b/src/backend/optimizer/util/relnode.c index ea15e20f2b..6ad2e82319 100644 --- a/src/backend/optimizer/util/relnode.c +++ b/src/backend/optimizer/util/relnode.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/optimizer/util/relnode.c,v 1.52.4.1 2003/12/08 18:20:10 tgl Exp $ + * $Header: /cvsroot/pgsql/src/backend/optimizer/util/relnode.c,v 1.52.4.2 2007/07/31 19:54:27 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -466,7 +466,10 @@ build_joinrel_restrictlist(Query *root, * previous clauses (see optimizer/README for discussion). We detect * that case and omit the redundant clause from the result list. */ - result = remove_redundant_join_clauses(root, rlist, jointype); + result = remove_redundant_join_clauses(root, rlist, + outer_rel->relids, + inner_rel->relids, + jointype); freeList(rlist); diff --git a/src/backend/optimizer/util/restrictinfo.c b/src/backend/optimizer/util/restrictinfo.c index 0c21e94e68..d3aa8098bd 100644 --- a/src/backend/optimizer/util/restrictinfo.c +++ b/src/backend/optimizer/util/restrictinfo.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/optimizer/util/restrictinfo.c,v 1.19 2003/08/04 02:40:01 momjian Exp $ + * $Header: /cvsroot/pgsql/src/backend/optimizer/util/restrictinfo.c,v 1.19.4.1 2007/07/31 19:54:27 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -23,6 +23,8 @@ static bool join_clause_is_redundant(Query *root, RestrictInfo *rinfo, List *reference_list, + Relids outer_relids, + Relids inner_relids, JoinType jointype); @@ -105,6 +107,8 @@ get_actual_join_clauses(List *restrictinfo_list, */ List * remove_redundant_join_clauses(Query *root, List *restrictinfo_list, + Relids outer_relids, + Relids inner_relids, JoinType jointype) { List *result = NIL; @@ -115,7 +119,9 @@ remove_redundant_join_clauses(Query *root, List *restrictinfo_list, RestrictInfo *rinfo = (RestrictInfo *) lfirst(item); /* drop it if redundant with any prior clause */ - if (join_clause_is_redundant(root, rinfo, result, jointype)) + if (join_clause_is_redundant(root, rinfo, result, + outer_relids, inner_relids, + jointype)) continue; /* otherwise, add it to result list */ @@ -143,6 +149,8 @@ List * select_nonredundant_join_clauses(Query *root, List *restrictinfo_list, List *reference_list, + Relids outer_relids, + Relids inner_relids, JoinType jointype) { List *result = NIL; @@ -153,7 +161,9 @@ select_nonredundant_join_clauses(Query *root, RestrictInfo *rinfo = (RestrictInfo *) lfirst(item); /* drop it if redundant with any reference clause */ - if (join_clause_is_redundant(root, rinfo, reference_list, jointype)) + if (join_clause_is_redundant(root, rinfo, reference_list, + outer_relids, inner_relids, + jointype)) continue; /* otherwise, add it to result list */ @@ -185,6 +195,12 @@ select_nonredundant_join_clauses(Query *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 @@ -194,6 +210,8 @@ static bool join_clause_is_redundant(Query *root, RestrictInfo *rinfo, List *reference_list, + Relids outer_relids, + Relids inner_relids, JoinType jointype) { /* always consider exact duplicates redundant */ @@ -228,16 +246,31 @@ join_clause_is_redundant(Query *root, if (redundant) { /* - * It looks redundant, now check for "var = const" case. If - * left_relids/right_relids are set, then there are definitely - * vars on both sides; else we must check the hard way. + * It looks redundant, now check for special cases. This is + * ugly and slow because of the mistaken decision to not set + * left_relids/right_relids all the time, as 8.0 and up do. + * Not going to change that in 7.x though. */ - if (rinfo->left_relids) - return true; /* var = var, so redundant */ - if (contain_var_clause(get_leftop(rinfo->clause)) && - contain_var_clause(get_rightop(rinfo->clause))) - return true; /* var = var, so redundant */ - /* else var = const, not redundant */ + Relids left_relids = rinfo->left_relids; + Relids right_relids = rinfo->right_relids; + + if (left_relids == NULL) + left_relids = pull_varnos(get_leftop(rinfo->clause)); + if (right_relids == NULL) + right_relids = pull_varnos(get_rightop(rinfo->clause)); + + if (bms_is_empty(left_relids) || bms_is_empty(right_relids)) + return false; /* var = const, so not redundant */ + + /* check for either side mentioning both rels */ + if (bms_overlap(left_relids, outer_relids) && + bms_overlap(left_relids, inner_relids)) + return false; /* clause LHS uses both, so not redundant */ + if (bms_overlap(right_relids, outer_relids) && + bms_overlap(right_relids, inner_relids)) + return false; /* clause RHS uses both, so not redundant */ + + return true; /* else it really is redundant */ } } diff --git a/src/include/optimizer/restrictinfo.h b/src/include/optimizer/restrictinfo.h index 3e7dc9edd9..6b4c8c6217 100644 --- a/src/include/optimizer/restrictinfo.h +++ b/src/include/optimizer/restrictinfo.h @@ -7,7 +7,7 @@ * Portions Copyright (c) 1996-2003, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $Id: restrictinfo.h,v 1.19 2003/08/04 02:40:14 momjian Exp $ + * $Id: restrictinfo.h,v 1.19.4.1 2007/07/31 19:54:27 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -22,10 +22,14 @@ extern void get_actual_join_clauses(List *restrictinfo_list, List **joinquals, List **otherquals); extern List *remove_redundant_join_clauses(Query *root, List *restrictinfo_list, + Relids outer_relids, + Relids inner_relids, JoinType jointype); extern List *select_nonredundant_join_clauses(Query *root, List *restrictinfo_list, List *reference_list, + Relids outer_relids, + Relids inner_relids, JoinType jointype); #endif /* RESTRICTINFO_H */ diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out index 7a1da6799e..ff48b4ed3f 100644 --- a/src/test/regress/expected/join.out +++ b/src/test/regress/expected/join.out @@ -2147,3 +2147,19 @@ DROP TABLE t2; DROP TABLE t3; DROP TABLE J1_TBL; DROP TABLE J2_TBL; +-- +-- 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 9bda6f1d00..70911408d4 100644 --- a/src/test/regress/sql/join.sql +++ b/src/test/regress/sql/join.sql @@ -349,3 +349,19 @@ DROP TABLE t3; DROP TABLE J1_TBL; DROP TABLE J2_TBL; + +-- +-- 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;