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

Since commit b448f1c, 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

Author: tglsfdc

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.
  *

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;

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;