When removing a left join, clean out references in EquivalenceClasses.

Since commit b448f1c8d, we've been able to remove left joins
(that are otherwise removable) even when they are underneath
other left joins, a case that was previously prevented by a
delay_upper_joins check.  This is a clear improvement, but
it has a surprising side-effect: it's now possible that there
are EquivalenceClasses whose relid sets mention the removed
baserel and/or outer join.  If we fail to clean those up,
we may drop essential join quals due to not having any join
level that appears to satisfy their relid sets.

(It's not quite 100% clear that this was impossible before.
But the lack of complaints since we added join removal a dozen
years ago strongly suggests that it was impossible.)

Richard Guo and Tom Lane, per bug #17976 from Zuming Jiang

Discussion: https://postgr.es/m/17976-4b638b525e9a983b@postgresql.org

diff --git a/src/backend/optimizer/plan/analyzejoins.c b/src/backend/optimizer/plan/analyzejoins.c
index 6476e55e56886d564a075477b88e5ad3539bf854..9161c8a2964768cde2d80a2a61cc0e5b3e6d9a29 100644 (file)
--- a/src/backend/optimizer/plan/analyzejoins.c
+++ b/src/backend/optimizer/plan/analyzejoins.c
@@ -39,6 +39,8 @@ static void remove_rel_from_query(PlannerInfo *root, int relid,
                                  SpecialJoinInfo *sjinfo);
 static void remove_rel_from_restrictinfo(RestrictInfo *rinfo,
                                         int relid, int ojrelid);
+static void remove_rel_from_eclass(EquivalenceClass *ec,
+                                  int relid, int ojrelid);
 static List *remove_rel_from_joinlist(List *joinlist, int relid, int *nremoved);
 static bool rel_supports_distinctness(PlannerInfo *root, RelOptInfo *rel);
 static bool rel_is_distinct_for(PlannerInfo *root, RelOptInfo *rel,
@@ -511,6 +513,18 @@ remove_rel_from_query(PlannerInfo *root, int relid, SpecialJoinInfo *sjinfo)
        }
    }
 
+   /*
+    * Likewise remove references from EquivalenceClasses.
+    */
+   foreach(l, root->eq_classes)
+   {
+       EquivalenceClass *ec = (EquivalenceClass *) lfirst(l);
+
+       if (bms_is_member(relid, ec->ec_relids) ||
+           bms_is_member(ojrelid, ec->ec_relids))
+           remove_rel_from_eclass(ec, relid, ojrelid);
+   }
+
    /*
     * There may be references to the rel in root->fkey_list, but if so,
     * match_foreign_keys_to_quals() will get rid of them.
@@ -583,6 +597,60 @@ remove_rel_from_restrictinfo(RestrictInfo *rinfo, int relid, int ojrelid)
    }
 }
 
+/*
+ * Remove any references to relid or ojrelid from the EquivalenceClass.
+ *
+ * Like remove_rel_from_restrictinfo, we don't worry about cleaning out
+ * any nullingrel bits in contained Vars and PHVs.  (This might have to be
+ * improved sometime.)  We do need to fix the EC and EM relid sets to ensure
+ * that implied join equalities will be generated at the appropriate join
+ * level(s).
+ */
+static void
+remove_rel_from_eclass(EquivalenceClass *ec, int relid, int ojrelid)
+{
+   ListCell   *lc;
+
+   /* Fix up the EC's overall relids */
+   ec->ec_relids = bms_del_member(ec->ec_relids, relid);
+   ec->ec_relids = bms_del_member(ec->ec_relids, ojrelid);
+
+   /*
+    * Fix up the member expressions.  Any non-const member that ends with
+    * empty em_relids must be a Var or PHV of the removed relation.  We don't
+    * need it anymore, so we can drop it.
+    */
+   foreach(lc, ec->ec_members)
+   {
+       EquivalenceMember *cur_em = (EquivalenceMember *) lfirst(lc);
+
+       if (bms_is_member(relid, cur_em->em_relids) ||
+           bms_is_member(ojrelid, cur_em->em_relids))
+       {
+           Assert(!cur_em->em_is_const);
+           cur_em->em_relids = bms_del_member(cur_em->em_relids, relid);
+           cur_em->em_relids = bms_del_member(cur_em->em_relids, ojrelid);
+           if (bms_is_empty(cur_em->em_relids))
+               ec->ec_members = foreach_delete_current(ec->ec_members, lc);
+       }
+   }
+
+   /* Fix up the source clauses, in case we can re-use them later */
+   foreach(lc, ec->ec_sources)
+   {
+       RestrictInfo *rinfo = (RestrictInfo *) lfirst(lc);
+
+       remove_rel_from_restrictinfo(rinfo, relid, ojrelid);
+   }
+
+   /*
+    * Rather than expend code on fixing up any already-derived clauses, just
+    * drop them.  (At this point, any such clauses would be base restriction
+    * clauses, which we'd not need anymore anyway.)
+    */
+   ec->ec_derives = NIL;
+}
+
 /*
  * Remove any occurrences of the target relid from a joinlist structure.
  *

diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out
index 98b2667821e78e9f9da8c42aaa1813926b8e64b3..cc4c122fdd42b6a04c43fc0ad527f87ff4733228 100644 (file)
--- a/src/test/regress/expected/join.out
+++ b/src/test/regress/expected/join.out
@@ -5881,6 +5881,37 @@ where ss.stringu2 !~* ss.case1;
  doh!
 (1 row)
 
+rollback;
+-- another join removal bug: we must clean up EquivalenceClasses too
+begin;
+create temp table t (a int unique);
+insert into t values (1);
+explain (costs off)
+select 1
+from t t1
+  left join (select 2 as c
+             from t t2 left join t t3 on t2.a = t3.a) s
+    on true
+where t1.a = s.c;
+          QUERY PLAN          
+------------------------------
+ Nested Loop Left Join
+   Filter: (t1.a = (2))
+   ->  Seq Scan on t t1
+   ->  Materialize
+         ->  Seq Scan on t t2
+(5 rows)
+
+select 1
+from t t1
+  left join (select 2 as c
+             from t t2 left join t t3 on t2.a = t3.a) s
+    on true
+where t1.a = s.c;
+ ?column? 
+----------
+(0 rows)
+
 rollback;
 -- test cases where we can remove a join, but not a PHV computed at it
 begin;

diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql
index 7daa390b1d46f5632b0175e108de0d188979cd8e..e77e4695709debbc80057aac533e51d70e30ec9a 100644 (file)
--- a/src/test/regress/sql/join.sql
+++ b/src/test/regress/sql/join.sql
@@ -2167,6 +2167,29 @@ where ss.stringu2 !~* ss.case1;
 
 rollback;
 
+-- another join removal bug: we must clean up EquivalenceClasses too
+begin;
+
+create temp table t (a int unique);
+insert into t values (1);
+
+explain (costs off)
+select 1
+from t t1
+  left join (select 2 as c
+             from t t2 left join t t3 on t2.a = t3.a) s
+    on true
+where t1.a = s.c;
+
+select 1
+from t t1
+  left join (select 2 as c
+             from t t2 left join t t3 on t2.a = t3.a) s
+    on true
+where t1.a = s.c;
+
+rollback;
+
 -- test cases where we can remove a join, but not a PHV computed at it
 begin;