From e938544befb4b7eaecf1287d46cb39f4cda05567 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Tue, 31 Jul 2007 19:54:11 +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 | 4 ++- src/backend/optimizer/util/relnode.c | 7 +++-- 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, 93 insertions(+), 8 deletions(-) diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c index dc26fc40cac..6e803ebdc19 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.167.4.3 2005/12/06 16:50:55 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/optimizer/path/indxpath.c,v 1.167.4.4 2007/07/31 19:54:10 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -386,6 +386,8 @@ group_clauses_by_indexkey_for_join(Query *root, { clausegroup = remove_redundant_join_clauses(root, clausegroup, + outer_relids, + rel->relids, jointype); } diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c index f1a16518395..8dc002c592b 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.175 2004/12/31 22:00:08 pgsql Exp $ + * $PostgreSQL: pgsql/src/backend/optimizer/plan/createplan.c,v 1.175.4.1 2007/07/31 19:54:10 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -955,6 +955,8 @@ create_nestloop_plan(Query *root, select_nonredundant_join_clauses(root, joinrestrictclauses, linitial(indexclauses), + 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 5a45b4a4444..9b47ff17a70 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.64 2004/12/31 22:00:23 pgsql Exp $ + * $PostgreSQL: pgsql/src/backend/optimizer/util/relnode.c,v 1.64.4.1 2007/07/31 19:54:10 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -461,7 +461,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); list_free(rlist); diff --git a/src/backend/optimizer/util/restrictinfo.c b/src/backend/optimizer/util/restrictinfo.c index 20590a98e0e..4349143e946 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.31 2004/12/31 22:00:23 pgsql Exp $ + * $PostgreSQL: pgsql/src/backend/optimizer/util/restrictinfo.c,v 1.31.4.1 2007/07/31 19:54:10 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -31,6 +31,8 @@ static Expr *make_sub_restrictinfos(Expr *clause, static RestrictInfo *join_clause_is_redundant(Query *root, RestrictInfo *rinfo, List *reference_list, + Relids outer_relids, + Relids inner_relids, JoinType jointype); @@ -314,6 +316,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; @@ -341,7 +345,9 @@ remove_redundant_join_clauses(Query *root, List *restrictinfo_list, RestrictInfo *prevrinfo; /* is it redundant with any prior clause? */ - prevrinfo = join_clause_is_redundant(root, rinfo, result, jointype); + prevrinfo = join_clause_is_redundant(root, rinfo, result, + outer_relids, inner_relids, + jointype); if (prevrinfo == NULL) { /* no, so add it to result list */ @@ -377,6 +383,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; @@ -387,7 +395,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) != NULL) + if (join_clause_is_redundant(root, rinfo, reference_list, + outer_relids, inner_relids, + jointype) != NULL) continue; /* otherwise, add it to result list */ @@ -420,6 +430,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 @@ -429,6 +445,8 @@ static RestrictInfo * join_clause_is_redundant(Query *root, RestrictInfo *rinfo, List *reference_list, + Relids outer_relids, + Relids inner_relids, JoinType jointype) { ListCell *refitem; @@ -450,6 +468,14 @@ join_clause_is_redundant(Query *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 a9e3bcac25e..f4a6514a3e2 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.26 2004/12/31 22:03:36 pgsql Exp $ + * $PostgreSQL: pgsql/src/include/optimizer/restrictinfo.h,v 1.26.4.1 2007/07/31 19:54:11 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -27,10 +27,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 4540b8fdbc4..8e34ca4da27 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/expected/join_1.out b/src/test/regress/expected/join_1.out index 7f8134bee5d..60d0c0832c7 100644 --- a/src/test/regress/expected/join_1.out +++ b/src/test/regress/expected/join_1.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 9bda6f1d002..70911408d41 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; -- 2.39.5