Adjust the definition of is_pushed_down so that it's always true for INNER

tglsfdc · tglsfdc · commit 8249409bc19b · 2007-02-16T20:57:19.000Z
JOIN quals, just like WHERE quals, even if they reference every one of the
join's relations.  Now that we can reorder outer and inner joins, it's
possible for such a qual to end up being assigned to an outer join plan node,
and we mustn't have it treated as a join qual rather than a filter qual for
the node.  (If it were, the join could produce null-extended rows that it
shouldn't.)  Per bug report from Pelle Johansson.
diff --git a/src/backend/optimizer/plan/initsplan.c b/src/backend/optimizer/plan/initsplan.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/optimizer/plan/initsplan.c,v 1.130 2007/02/13 02:31:03 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/optimizer/plan/initsplan.c,v 1.131 2007/02/16 20:57:19 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -43,7 +43,6 @@ static OuterJoinInfo *make_outerjoininfo(PlannerInfo *root,
 				   Relids left_rels, Relids right_rels,
 				   bool is_full_join, Node *clause);
 static void distribute_qual_to_rels(PlannerInfo *root, Node *clause,
-						bool is_pushed_down,
 						bool is_deduced,
 						bool below_outer_join,
 						Relids qualscope,
@@ -283,12 +282,11 @@ deconstruct_recurse(PlannerInfo *root, Node *jtnode, bool below_outer_join,
 		}
 
 		/*
-		 * Now process the top-level quals.  These are always marked as
-		 * "pushed down", since they clearly didn't come from a JOIN expr.
+		 * Now process the top-level quals.
 		 */
 		foreach(l, (List *) f->quals)
 			distribute_qual_to_rels(root, (Node *) lfirst(l),
-									true, false, below_outer_join,
+									false, below_outer_join,
 									*qualscope, NULL, NULL);
 	}
 	else if (IsA(jtnode, JoinExpr))
@@ -389,7 +387,7 @@ deconstruct_recurse(PlannerInfo *root, Node *jtnode, bool below_outer_join,
 		/* Process the qual clauses */
 		foreach(qual, (List *) j->quals)
 			distribute_qual_to_rels(root, (Node *) lfirst(qual),
-									false, false, below_outer_join,
+									false, below_outer_join,
 									*qualscope, ojscope, nonnullable_rels);
 
 		/* Now we can add the OuterJoinInfo to oj_info_list */
@@ -600,8 +598,6 @@ make_outerjoininfo(PlannerInfo *root,
  *	  EquivalenceClasses.
  *
  * 'clause': the qual clause to be distributed
- * 'is_pushed_down': if TRUE, force the clause to be marked 'is_pushed_down'
- *		(this indicates the clause came from a FromExpr, not a JoinExpr)
  * 'is_deduced': TRUE if the qual came from implied-equality deduction
  * 'below_outer_join': TRUE if the qual is from a JOIN/ON that is below the
  *		nullable side of a higher-level outer join
@@ -619,14 +615,14 @@ make_outerjoininfo(PlannerInfo *root,
  */
 static void
 distribute_qual_to_rels(PlannerInfo *root, Node *clause,
-						bool is_pushed_down,
 						bool is_deduced,
 						bool below_outer_join,
 						Relids qualscope,
 						Relids ojscope,
 						Relids outerjoin_nonnullable)
 {
 	Relids		relids;
+	bool		is_pushed_down;
 	bool		outerjoin_delayed;
 	bool		pseudoconstant = false;
 	bool		maybe_equivalence;
@@ -692,17 +688,37 @@ distribute_qual_to_rels(PlannerInfo *root, Node *clause,
 				root->hasPseudoConstantQuals = true;
 				/* if not below outer join, push it to top of tree */
 				if (!below_outer_join)
-				{
 					relids = get_relids_in_jointree((Node *) root->parse->jointree);
-					is_pushed_down = true;
-				}
 			}
 		}
 	}
 
-	/*
+	/*----------
 	 * Check to see if clause application must be delayed by outer-join
 	 * considerations.
+	 *
+	 * A word about is_pushed_down: we mark the qual as "pushed down" if
+	 * it is (potentially) applicable at a level different from its original
+	 * syntactic level.  This flag is used to distinguish OUTER JOIN ON quals
+	 * from other quals pushed down to the same joinrel.  The rules are:
+	 *		WHERE quals and INNER JOIN quals: is_pushed_down = true.
+	 *		Non-degenerate OUTER JOIN quals: is_pushed_down = false.
+	 *		Degenerate OUTER JOIN quals: is_pushed_down = true.
+	 * A "degenerate" OUTER JOIN qual is one that doesn't mention the
+	 * non-nullable side, and hence can be pushed down into the nullable side
+	 * without changing the join result.  It is correct to treat it as a
+	 * regular filter condition at the level where it is evaluated.
+	 *
+	 * Note: it is not immediately obvious that a simple boolean is enough
+	 * for this: if for some reason we were to attach a degenerate qual to
+	 * its original join level, it would need to be treated as an outer join
+	 * qual there.  However, this cannot happen, because all the rels the
+	 * clause mentions must be in the outer join's min_righthand, therefore
+	 * the join it needs must be formed before the outer join; and we always
+	 * attach quals to the lowest level where they can be evaluated.  But
+	 * if we were ever to re-introduce a mechanism for delaying evaluation
+	 * of "expensive" quals, this area would need work.
+	 *----------
 	 */
 	if (is_deduced)
 	{
@@ -713,6 +729,7 @@ distribute_qual_to_rels(PlannerInfo *root, Node *clause,
 		 * where the qual belongs.
 		 */
 		Assert(!ojscope);
+		is_pushed_down = true;
 		outerjoin_delayed = false;
 		/* Don't feed it back for more deductions */
 		maybe_equivalence = false;
@@ -722,12 +739,7 @@ distribute_qual_to_rels(PlannerInfo *root, Node *clause,
 	{
 		/*
 		 * The qual is attached to an outer join and mentions (some of the)
-		 * rels on the nonnullable side.
-		 *
-		 * Note: an outer-join qual that mentions only nullable-side rels can
-		 * be pushed down into the nullable side without changing the join
-		 * result, so we treat it almost the same as an ordinary inner-join
-		 * qual (see below).
+		 * rels on the nonnullable side, so it's not degenerate.
 		 *
 		 * We can't use such a clause to deduce equivalence (the left and right
 		 * sides might be unequal above the join because one of them has gone
@@ -751,12 +763,19 @@ distribute_qual_to_rels(PlannerInfo *root, Node *clause,
 		 */
 		Assert(ojscope);
 		relids = ojscope;
+		is_pushed_down = false;
 		outerjoin_delayed = true;
 		Assert(!pseudoconstant);
 	}
 	else
 	{
-		/* Normal qual clause; check to see if must be delayed by outer join */
+		/*
+		 * Normal qual clause or degenerate outer-join clause.  Either way,
+		 * we can mark it as pushed-down.
+		 */
+		is_pushed_down = true;
+
+		/* Check to see if must be delayed by outer join */
 		outerjoin_delayed = check_outerjoin_delay(root, &relids);
 
 		if (outerjoin_delayed)
@@ -791,17 +810,6 @@ distribute_qual_to_rels(PlannerInfo *root, Node *clause,
 		maybe_outer_join = false;
 	}
 
-	/*
-	 * Mark the qual as "pushed down" if it can be applied at a level below
-	 * its original syntactic level.  This allows us to distinguish original
-	 * JOIN/ON quals from higher-level quals pushed down to the same joinrel.
-	 * A qual originating from WHERE is always considered "pushed down". Note
-	 * that for an outer-join qual, we have to compare to ojscope not
-	 * qualscope.
-	 */
-	if (!is_pushed_down)
-		is_pushed_down = !bms_equal(relids, ojscope ? ojscope : qualscope);
-
 	/*
 	 * Build the RestrictInfo node itself.
 	 */
@@ -915,7 +923,7 @@ distribute_qual_to_rels(PlannerInfo *root, Node *clause,
  * If so, add relids to *relids_p to reflect the lowest safe level for
  * evaluating the qual, and return TRUE.
  *
- * For a non-outer-join qual, we can evaluate the qual as soon as (1) we have
+ * For an is_pushed_down qual, we can evaluate the qual as soon as (1) we have
  * all the rels it mentions, and (2) we are at or above any outer joins that
  * can null any of these rels and are below the syntactic location of the
  * given qual.  We must enforce (2) because pushing down such a clause below
@@ -935,7 +943,7 @@ distribute_qual_to_rels(PlannerInfo *root, Node *clause,
  * B/C join is done first then the join to A can null C, so a qual actually
  * mentioning only C cannot be applied below the join to A.
  *
- * For an outer-join qual, this isn't going to determine where we place the
+ * For a non-pushed-down qual, this isn't going to determine where we place the
  * qual, but we need to determine outerjoin_delayed anyway so we can decide
  * whether the qual is potentially useful for equivalence deductions.
  */
@@ -1101,12 +1109,9 @@ process_implied_equality(PlannerInfo *root,
 
 	/*
 	 * Push the new clause into all the appropriate restrictinfo lists.
-	 *
-	 * Note: we mark the qual "pushed down" to ensure that it can never be
-	 * taken for an original JOIN/ON clause.
 	 */
 	distribute_qual_to_rels(root, (Node *) clause,
-							true, true, below_outer_join,
+							true, below_outer_join,
 							qualscope, NULL, NULL);
 }
 
diff --git a/src/include/nodes/relation.h b/src/include/nodes/relation.h
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2007, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/nodes/relation.h,v 1.134 2007/01/22 20:00:40 tgl Exp $
+ * $PostgreSQL: pgsql/src/include/nodes/relation.h,v 1.135 2007/02/16 20:57:19 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -797,14 +797,14 @@ typedef struct HashPath
  * join, we prevent it from being evaluated below the outer join's joinrel.
  * When we do form the outer join's joinrel, we still need to distinguish
  * those quals that are actually in that join's JOIN/ON condition from those
- * that appeared higher in the tree and were pushed down to the join rel
+ * that appeared elsewhere in the tree and were pushed down to the join rel
  * because they used no other rels.  That's what the is_pushed_down flag is
- * for; it tells us that a qual came from a point above the join of the
- * set of base rels listed in required_relids.	A clause that originally came
- * from WHERE will *always* have its is_pushed_down flag set; a clause that
- * came from an INNER JOIN condition, but doesn't use all the rels being
- * joined, will also have is_pushed_down set because it will get attached to
- * some lower joinrel.
+ * for; it tells us that a qual is not an OUTER JOIN qual for the set of base
+ * rels listed in required_relids.  A clause that originally came from WHERE
+ * or an INNER JOIN condition will *always* have its is_pushed_down flag set.
+ * It's possible for an OUTER JOIN clause to be marked is_pushed_down too,
+ * if we decide that it can be pushed down into the nullable side of the join.
+ * In that case it acts as a plain filter qual for wherever it gets evaluated.
  *
  * When application of a qual must be delayed by outer join, we also mark it
  * with outerjoin_delayed = true.  This isn't redundant with required_relids
