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.

diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c
index 77518f6c480589f979450f24af3f98a415e81b40..ab032f877fa7fe3c85faabf865f9066fc823b568 100644 (file)
--- a/src/backend/optimizer/path/indxpath.c
+++ b/src/backend/optimizer/path/indxpath.c
@@ -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 9b7493b93e740a2d77a0cf03ebd998047d634dd4..d5988a5f5f0cc2b0bfcb191982124fbc47f3e57e 100644 (file)
--- a/src/backend/optimizer/plan/createplan.c
+++ b/src/backend/optimizer/plan/createplan.c
@@ -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 b671ce2c2c003900205a88adc7f3b889e58b8d8a..245fd76aa03c82fb0a0f5f9b1029c4ce12c35200 100644 (file)
--- a/src/backend/optimizer/util/relnode.c
+++ b/src/backend/optimizer/util/relnode.c
@@ -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 997e0e1e48498ee7110dce4239dac1a292e22e7e..b57d09802d8a4800a1c8d7440fb043d5c4de7c0c 100644 (file)
--- a/src/backend/optimizer/util/restrictinfo.c
+++ b/src/backend/optimizer/util/restrictinfo.c
@@ -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 93424bd713e0518da32a47f7d82a8da82835d7c6..a7df2eb9e08d42ec6323a301d8b2f783c85d2919 100644 (file)
--- a/src/include/optimizer/restrictinfo.h
+++ b/src/include/optimizer/restrictinfo.h
@@ -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 7a1da6799e53d7bf502b17cd4a25b14e78f90fa7..ff48b4ed3f5bf981db14eacb1159e8c8c2c01194 100644 (file)
--- 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 9bda6f1d00249377bb888ab993d79041f7b7926a..70911408d417423818e51ab04ea003a86984bb49 100644 (file)
--- 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;