Fix join-removal logic for pseudoconstant and outerjoin-delayed quals.

In these cases a qual can get marked with the removable rel in its
required_relids, but this is just to schedule its evaluation correctly, not
because it really depends on the rel.  We were assuming that, in effect,
we could throw away *all* quals so marked, which is nonsense.  Tighten up
the logic to be a little more paranoid about which quals belong to the
outer join being considered for removal, and arrange for all quals that
don't belong to be updated so they will still get evaluated correctly.

Also fix another problem that happened to be exposed by this test case,
which was that make_join_rel() was failing to notice some cases where
a constant-false qual could be used to prove a join relation empty.  If it's
a pushed-down constant false, then the relation is empty even if it's an
outer join, because the qual applies after the outer join expansion.

Per report from Nathan Grange.  Back-patch into 9.0.

1 files changed, 65 insertions, 18 deletions

diff --git a/src/backend/optimizer/plan/analyzejoins.c b/src/backend/optimizer/plan/analyzejoins.c
index 0d90d072eaa..c0860beb3cb 100644
--- a/src/backend/optimizer/plan/analyzejoins.c
+++ b/src/backend/optimizer/plan/analyzejoins.c
@@ -16,19 +16,21 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/optimizer/plan/analyzejoins.c,v 1.3 2010/07/06 19:18:56 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/backend/optimizer/plan/analyzejoins.c,v 1.4 2010/09/14 23:15:29 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
 #include "postgres.h"
 
+#include "optimizer/joininfo.h"
 #include "optimizer/pathnode.h"
 #include "optimizer/paths.h"
 #include "optimizer/planmain.h"
 
 /* local functions */
 static bool join_is_removable(PlannerInfo *root, SpecialJoinInfo *sjinfo);
-static void remove_rel_from_query(PlannerInfo *root, int relid);
+static void remove_rel_from_query(PlannerInfo *root, int relid,
+								  Relids joinrelids);
 static List *remove_rel_from_joinlist(List *joinlist, int relid, int *nremoved);
 
 
@@ -67,7 +69,9 @@ restart:
 		 */
 		innerrelid = bms_singleton_member(sjinfo->min_righthand);
 
-		remove_rel_from_query(root, innerrelid);
+		remove_rel_from_query(root, innerrelid,
+							  bms_union(sjinfo->min_lefthand,
+										sjinfo->min_righthand));
 
 		/* We verify that exactly one reference gets removed from joinlist */
 		nremoved = 0;
@@ -216,19 +220,25 @@ join_is_removable(PlannerInfo *root, SpecialJoinInfo *sjinfo)
 	{
 		RestrictInfo *restrictinfo = (RestrictInfo *) lfirst(l);
 
-		/* Ignore clauses not pertinent to this join */
-		if (!bms_is_subset(restrictinfo->required_relids, joinrelids))
-			continue;
-
 		/*
-		 * If we find a pushed-down clause, it must have come from above the
-		 * outer join and it must contain references to the inner rel.	(If it
-		 * had only outer-rel variables, it'd have been pushed down into the
-		 * outer rel.)	Therefore, we can conclude that join removal is unsafe
-		 * without any examination of the clause contents.
+		 * If it's not a join clause for this outer join, we can't use it.
+		 * Note that if the clause is pushed-down, then it is logically from
+		 * above the outer join, even if it references no other rels (it might
+		 * be from WHERE, for example).
 		 */
-		if (restrictinfo->is_pushed_down)
-			return false;
+		if (restrictinfo->is_pushed_down ||
+			!bms_equal(restrictinfo->required_relids, joinrelids))
+		{
+			/*
+			 * If such a clause actually references the inner rel then
+			 * join removal has to be disallowed.  We have to check this
+			 * despite the previous attr_needed checks because of the
+			 * possibility of pushed-down clauses referencing the rel.
+			 */
+			if (bms_is_member(innerrelid, restrictinfo->clause_relids))
+				return false;
+			continue;			/* else, ignore; not useful here */
+		}
 
 		/* Ignore if it's not a mergejoinable clause */
 		if (!restrictinfo->can_join ||
@@ -299,14 +309,14 @@ join_is_removable(PlannerInfo *root, SpecialJoinInfo *sjinfo)
  * We are not terribly thorough here.  We must make sure that the rel is
  * no longer treated as a baserel, and that attributes of other baserels
  * are no longer marked as being needed at joins involving this rel.
- * In particular, we don't bother removing join quals involving the rel from
- * the joininfo lists; they'll just get ignored, since we will never form a
- * join relation at which they could be evaluated.
+ * Also, join quals involving the rel have to be removed from the joininfo
+ * lists, but only if they belong to the outer join identified by joinrelids.
  */
 static void
-remove_rel_from_query(PlannerInfo *root, int relid)
+remove_rel_from_query(PlannerInfo *root, int relid, Relids joinrelids)
 {
 	RelOptInfo *rel = find_base_rel(root, relid);
+	List	   *joininfos;
 	Index		rti;
 	ListCell   *l;
 
@@ -379,6 +389,43 @@ remove_rel_from_query(PlannerInfo *root, int relid)
 
 		phinfo->ph_needed = bms_del_member(phinfo->ph_needed, relid);
 	}
+
+	/*
+	 * Remove any joinquals referencing the rel from the joininfo lists.
+	 *
+	 * In some cases, a joinqual has to be put back after deleting its
+	 * reference to the target rel.  This can occur for pseudoconstant and
+	 * outerjoin-delayed quals, which can get marked as requiring the rel in
+	 * order to force them to be evaluated at or above the join.  We can't
+	 * just discard them, though.  Only quals that logically belonged to the
+	 * outer join being discarded should be removed from the query.
+	 *
+	 * We must make a copy of the rel's old joininfo list before starting the
+	 * loop, because otherwise remove_join_clause_from_rels would destroy the
+	 * list while we're scanning it.
+	 */
+	joininfos = list_copy(rel->joininfo);
+	foreach(l, joininfos)
+	{
+		RestrictInfo *rinfo = (RestrictInfo *) lfirst(l);
+
+		remove_join_clause_from_rels(root, rinfo, rinfo->required_relids);
+
+		if (rinfo->is_pushed_down ||
+			!bms_equal(rinfo->required_relids, joinrelids))
+		{
+			/* Recheck that qual doesn't actually reference the target rel */
+			Assert(!bms_is_member(relid, rinfo->clause_relids));
+			/*
+			 * The required_relids probably aren't shared with anything else,
+			 * but let's copy them just to be sure.
+			 */
+			rinfo->required_relids = bms_copy(rinfo->required_relids);
+			rinfo->required_relids = bms_del_member(rinfo->required_relids,
+													relid);
+			distribute_restrictinfo_to_rels(root, rinfo);
+		}
+	}
 }
 
 /*