Restore the former RestrictInfo field valid_everywhere (but invert the flag

sense and rename to "outerjoin_delayed" to more clearly reflect what it
means).  I had decided that it was redundant in 8.1, but the folly of this
is exposed by a bug report from Sebastian Böck.  The place where it's
needed is to prevent orindxpath.c from cherry-picking arms of an outer-join
OR clause to form a relation restriction that isn't actually legal to push
down to the relation scan level.  There may be some legal cases that this
forbids optimizing, but we'd need much closer analysis to determine it.

9 files changed, 72 insertions, 26 deletions

diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 4a90b10b277..244f80751dd 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -15,7 +15,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/nodes/copyfuncs.c,v 1.316 2005/10/15 02:49:18 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/backend/nodes/copyfuncs.c,v 1.316.2.1 2005/11/14 23:54:32 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1249,6 +1249,7 @@ _copyRestrictInfo(RestrictInfo *from)
 
 	COPY_NODE_FIELD(clause);
 	COPY_SCALAR_FIELD(is_pushed_down);
+	COPY_SCALAR_FIELD(outerjoin_delayed);
 	COPY_SCALAR_FIELD(can_join);
 	COPY_BITMAPSET_FIELD(clause_relids);
 	COPY_BITMAPSET_FIELD(required_relids);
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index 9baa79dd935..1940d780ed9 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -18,7 +18,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/nodes/equalfuncs.c,v 1.253 2005/10/15 02:49:18 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/backend/nodes/equalfuncs.c,v 1.253.2.1 2005/11/14 23:54:34 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -602,6 +602,7 @@ _equalRestrictInfo(RestrictInfo *a, RestrictInfo *b)
 {
 	COMPARE_NODE_FIELD(clause);
 	COMPARE_SCALAR_FIELD(is_pushed_down);
+	COMPARE_SCALAR_FIELD(outerjoin_delayed);
 	COMPARE_BITMAPSET_FIELD(required_relids);
 
 	/*
diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index 19306b3e53d..841db47aebc 100644
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/nodes/outfuncs.c,v 1.261 2005/10/15 02:49:18 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/backend/nodes/outfuncs.c,v 1.261.2.1 2005/11/14 23:54:34 tgl Exp $
  *
  * NOTES
  *	  Every node type that can appear in stored rules' parsetrees *must*
@@ -1241,6 +1241,7 @@ _outRestrictInfo(StringInfo str, RestrictInfo *node)
 	/* NB: this isn't a complete set of fields */
 	WRITE_NODE_FIELD(clause);
 	WRITE_BOOL_FIELD(is_pushed_down);
+	WRITE_BOOL_FIELD(outerjoin_delayed);
 	WRITE_BOOL_FIELD(can_join);
 	WRITE_BITMAPSET_FIELD(clause_relids);
 	WRITE_BITMAPSET_FIELD(required_relids);
diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c
index 1790cc5266b..466c369cde5 100644
--- a/src/backend/optimizer/path/indxpath.c
+++ b/src/backend/optimizer/path/indxpath.c
@@ -9,7 +9,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/optimizer/path/indxpath.c,v 1.191 2005/10/15 02:49:19 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/backend/optimizer/path/indxpath.c,v 1.191.2.1 2005/11/14 23:54:35 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1917,6 +1917,7 @@ expand_indexqual_conditions(IndexOptInfo *index, List *clausegroups)
 					resultquals = lappend(resultquals,
 										  make_restrictinfo(boolqual,
 															true,
+															false,
 															NULL));
 					continue;
 				}
@@ -2166,7 +2167,7 @@ prefix_quals(Node *leftop, Oid opclass,
 			elog(ERROR, "no = operator for opclass %u", opclass);
 		expr = make_opclause(oproid, BOOLOID, false,
 							 (Expr *) leftop, (Expr *) prefix_const);
-		result = list_make1(make_restrictinfo(expr, true, NULL));
+		result = list_make1(make_restrictinfo(expr, true, false, NULL));
 		return result;
 	}
 
@@ -2181,7 +2182,7 @@ prefix_quals(Node *leftop, Oid opclass,
 		elog(ERROR, "no >= operator for opclass %u", opclass);
 	expr = make_opclause(oproid, BOOLOID, false,
 						 (Expr *) leftop, (Expr *) prefix_const);
-	result = list_make1(make_restrictinfo(expr, true, NULL));
+	result = list_make1(make_restrictinfo(expr, true, false, NULL));
 
 	/*-------
 	 * If we can create a string larger than the prefix, we can say
@@ -2197,7 +2198,7 @@ prefix_quals(Node *leftop, Oid opclass,
 			elog(ERROR, "no < operator for opclass %u", opclass);
 		expr = make_opclause(oproid, BOOLOID, false,
 							 (Expr *) leftop, (Expr *) greaterstr);
-		result = lappend(result, make_restrictinfo(expr, true, NULL));
+		result = lappend(result, make_restrictinfo(expr, true, false, NULL));
 	}
 
 	return result;
@@ -2268,7 +2269,7 @@ network_prefix_quals(Node *leftop, Oid expr_op, Oid opclass, Datum rightop)
 						 (Expr *) leftop,
 						 (Expr *) makeConst(datatype, -1, opr1right,
 											false, false));
-	result = list_make1(make_restrictinfo(expr, true, NULL));
+	result = list_make1(make_restrictinfo(expr, true, false, NULL));
 
 	/* create clause "key <= network_scan_last( rightop )" */
 
@@ -2283,7 +2284,7 @@ network_prefix_quals(Node *leftop, Oid expr_op, Oid opclass, Datum rightop)
 						 (Expr *) leftop,
 						 (Expr *) makeConst(datatype, -1, opr2right,
 											false, false));
-	result = lappend(result, make_restrictinfo(expr, true, NULL));
+	result = lappend(result, make_restrictinfo(expr, true, false, NULL));
 
 	return result;
 }
diff --git a/src/backend/optimizer/path/orindxpath.c b/src/backend/optimizer/path/orindxpath.c
index be5a0c3434f..5580b9a2772 100644
--- a/src/backend/optimizer/path/orindxpath.c
+++ b/src/backend/optimizer/path/orindxpath.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/optimizer/path/orindxpath.c,v 1.75 2005/10/15 02:49:20 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/backend/optimizer/path/orindxpath.c,v 1.75.2.1 2005/11/14 23:54:35 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -90,13 +90,19 @@ create_or_index_quals(PlannerInfo *root, RelOptInfo *rel)
 	ListCell   *i;
 
 	/*
-	 * Find potentially interesting OR joinclauses.
+	 * Find potentially interesting OR joinclauses.  Note we must ignore any
+	 * joinclauses that are marked outerjoin_delayed, because they cannot
+	 * be pushed down to the per-relation level due to outer-join rules.
+	 * (XXX in some cases it might be possible to allow this, but it would
+	 * require substantially more bookkeeping about where the clause came
+	 * from.)
 	 */
 	foreach(i, rel->joininfo)
 	{
 		RestrictInfo *rinfo = (RestrictInfo *) lfirst(i);
 
-		if (restriction_is_or_clause(rinfo))
+		if (restriction_is_or_clause(rinfo) &&
+			!rinfo->outerjoin_delayed)
 		{
 			/*
 			 * Use the generate_bitmap_or_paths() machinery to estimate the
diff --git a/src/backend/optimizer/plan/initsplan.c b/src/backend/optimizer/plan/initsplan.c
index dd8fc4fa2d7..0901c66518c 100644
--- 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.110 2005/10/15 02:49:20 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/backend/optimizer/plan/initsplan.c,v 1.110.2.1 2005/11/14 23:54:35 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -409,6 +409,7 @@ distribute_qual_to_rels(PlannerInfo *root, Node *clause,
 						Relids qualscope)
 {
 	Relids		relids;
+	bool		outerjoin_delayed;
 	bool		maybe_equijoin;
 	bool		maybe_outer_join;
 	RestrictInfo *restrictinfo;
@@ -451,6 +452,7 @@ distribute_qual_to_rels(PlannerInfo *root, Node *clause,
 		 */
 		Assert(bms_equal(relids, qualscope));
 		/* Needn't feed it back for more deductions */
+		outerjoin_delayed = false;
 		maybe_equijoin = false;
 		maybe_outer_join = false;
 	}
@@ -470,6 +472,7 @@ distribute_qual_to_rels(PlannerInfo *root, Node *clause,
 		 * except for not setting maybe_equijoin (see below).
 		 */
 		relids = qualscope;
+		outerjoin_delayed = true;
 
 		/*
 		 * We can't use such a clause to deduce equijoin (the left and right
@@ -499,13 +502,17 @@ distribute_qual_to_rels(PlannerInfo *root, Node *clause,
 		Relids		tmprelids;
 		int			relno;
 
+		outerjoin_delayed = false;
 		tmprelids = bms_copy(relids);
 		while ((relno = bms_first_member(tmprelids)) >= 0)
 		{
 			RelOptInfo *rel = find_base_rel(root, relno);
 
 			if (rel->outerjoinset != NULL)
+			{
 				addrelids = bms_add_members(addrelids, rel->outerjoinset);
+				outerjoin_delayed = true;
+			}
 		}
 		bms_free(tmprelids);
 
@@ -555,6 +562,7 @@ distribute_qual_to_rels(PlannerInfo *root, Node *clause,
 	 */
 	restrictinfo = make_restrictinfo((Expr *) clause,
 									 is_pushed_down,
+									 outerjoin_delayed,
 									 relids);
 
 	/*
diff --git a/src/backend/optimizer/util/restrictinfo.c b/src/backend/optimizer/util/restrictinfo.c
index d277cac7351..f79b7c188d2 100644
--- a/src/backend/optimizer/util/restrictinfo.c
+++ b/src/backend/optimizer/util/restrictinfo.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/optimizer/util/restrictinfo.c,v 1.41 2005/10/15 02:49:21 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/backend/optimizer/util/restrictinfo.c,v 1.41.2.1 2005/11/14 23:54:35 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -25,9 +25,11 @@
 static RestrictInfo *make_restrictinfo_internal(Expr *clause,
 						   Expr *orclause,
 						   bool is_pushed_down,
+						   bool outerjoin_delayed,
 						   Relids required_relids);
 static Expr *make_sub_restrictinfos(Expr *clause,
-					   bool is_pushed_down);
+					   bool is_pushed_down,
+					   bool outerjoin_delayed);
 static RestrictInfo *join_clause_is_redundant(PlannerInfo *root,
 						 RestrictInfo *rinfo,
 						 List *reference_list,
@@ -39,8 +41,8 @@ static RestrictInfo *join_clause_is_redundant(PlannerInfo *root,
  *
  * Build a RestrictInfo node containing the given subexpression.
  *
- * The is_pushed_down flag must be supplied by the caller.
- * required_relids can be NULL, in which case it defaults to the
+ * The is_pushed_down and outerjoin_delayed flags must be supplied by the
+ * caller.  required_relids can be NULL, in which case it defaults to the
  * actual clause contents (i.e., clause_relids).
  *
  * We initialize fields that depend only on the given subexpression, leaving
@@ -48,19 +50,25 @@ static RestrictInfo *join_clause_is_redundant(PlannerInfo *root,
  * later.
  */
 RestrictInfo *
-make_restrictinfo(Expr *clause, bool is_pushed_down, Relids required_relids)
+make_restrictinfo(Expr *clause,
+				  bool is_pushed_down,
+				  bool outerjoin_delayed,
+				  Relids required_relids)
 {
 	/*
 	 * If it's an OR clause, build a modified copy with RestrictInfos inserted
 	 * above each subclause of the top-level AND/OR structure.
 	 */
 	if (or_clause((Node *) clause))
-		return (RestrictInfo *) make_sub_restrictinfos(clause, is_pushed_down);
+		return (RestrictInfo *) make_sub_restrictinfos(clause,
+													   is_pushed_down,
+													   outerjoin_delayed);
 
 	/* Shouldn't be an AND clause, else AND/OR flattening messed up */
 	Assert(!and_clause((Node *) clause));
 
-	return make_restrictinfo_internal(clause, NULL, is_pushed_down,
+	return make_restrictinfo_internal(clause, NULL,
+									  is_pushed_down, outerjoin_delayed,
 									  required_relids);
 }
 
@@ -74,6 +82,9 @@ make_restrictinfo(Expr *clause, bool is_pushed_down, Relids required_relids)
  * The result is a List (effectively, implicit-AND representation) of
  * RestrictInfos.
  *
+ * The caller must pass is_pushed_down, but we assume outerjoin_delayed
+ * is false (no such qual should ever get into a bitmapqual).
+ *
  * If include_predicates is true, we add any partial index predicates to
  * the explicit index quals.  When this is not true, we return a condition
  * that might be weaker than the actual scan represents.
@@ -169,6 +180,7 @@ make_restrictinfo_from_bitmapqual(Path *bitmapqual,
 				list_make1(make_restrictinfo_internal(make_orclause(withoutris),
 													  make_orclause(withris),
 													  is_pushed_down,
+													  false,
 													  NULL));
 		}
 	}
@@ -193,6 +205,7 @@ make_restrictinfo_from_bitmapqual(Path *bitmapqual,
 					result = lappend(result,
 									 make_restrictinfo(pred,
 													   is_pushed_down,
+													   false,
 													   NULL));
 			}
 		}
@@ -213,13 +226,15 @@ make_restrictinfo_from_bitmapqual(Path *bitmapqual,
  */
 static RestrictInfo *
 make_restrictinfo_internal(Expr *clause, Expr *orclause,
-						   bool is_pushed_down, Relids required_relids)
+						   bool is_pushed_down, bool outerjoin_delayed,
+						   Relids required_relids)
 {
 	RestrictInfo *restrictinfo = makeNode(RestrictInfo);
 
 	restrictinfo->clause = clause;
 	restrictinfo->orclause = orclause;
 	restrictinfo->is_pushed_down = is_pushed_down;
+	restrictinfo->outerjoin_delayed = outerjoin_delayed;
 	restrictinfo->can_join = false;		/* may get set below */
 
 	/*
@@ -299,7 +314,8 @@ make_restrictinfo_internal(Expr *clause, Expr *orclause,
  * simple clauses are valid RestrictInfos.
  */
 static Expr *
-make_sub_restrictinfos(Expr *clause, bool is_pushed_down)
+make_sub_restrictinfos(Expr *clause,
+					   bool is_pushed_down, bool outerjoin_delayed)
 {
 	if (or_clause((Node *) clause))
 	{
@@ -309,10 +325,12 @@ make_sub_restrictinfos(Expr *clause, bool is_pushed_down)
 		foreach(temp, ((BoolExpr *) clause)->args)
 			orlist = lappend(orlist,
 							 make_sub_restrictinfos(lfirst(temp),
-													is_pushed_down));
+													is_pushed_down,
+													outerjoin_delayed));
 		return (Expr *) make_restrictinfo_internal(clause,
 												   make_orclause(orlist),
 												   is_pushed_down,
+												   outerjoin_delayed,
 												   NULL);
 	}
 	else if (and_clause((Node *) clause))
@@ -323,13 +341,15 @@ make_sub_restrictinfos(Expr *clause, bool is_pushed_down)
 		foreach(temp, ((BoolExpr *) clause)->args)
 			andlist = lappend(andlist,
 							  make_sub_restrictinfos(lfirst(temp),
-													 is_pushed_down));
+													 is_pushed_down,
+													 outerjoin_delayed));
 		return make_andclause(andlist);
 	}
 	else
 		return (Expr *) make_restrictinfo_internal(clause,
 												   NULL,
 												   is_pushed_down,
+												   outerjoin_delayed,
 												   NULL);
 }
 
diff --git a/src/include/nodes/relation.h b/src/include/nodes/relation.h
index 01aa96d7171..c40c2b6c62c 100644
--- a/src/include/nodes/relation.h
+++ b/src/include/nodes/relation.h
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2005, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/nodes/relation.h,v 1.119 2005/10/15 02:49:45 momjian Exp $
+ * $PostgreSQL: pgsql/src/include/nodes/relation.h,v 1.119.2.1 2005/11/14 23:54:36 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -714,6 +714,11 @@ typedef struct HashPath
  * joined, will also have is_pushed_down set because it will get attached to
  * some lower joinrel.
  *
+ * 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
+ * because that might equal clause_relids whether or not it's an outer-join
+ * clause.
+ *
  * In general, the referenced clause might be arbitrarily complex.	The
  * kinds of clauses we can handle as indexscan quals, mergejoin clauses,
  * or hashjoin clauses are fairly limited --- the code for each kind of
@@ -740,6 +745,8 @@ typedef struct RestrictInfo
 
 	bool		is_pushed_down; /* TRUE if clause was pushed down in level */
 
+	bool		outerjoin_delayed;		/* TRUE if delayed by outer join */
+
 	/*
 	 * This flag is set true if the clause looks potentially useful as a merge
 	 * or hash join clause, that is if it is a binary opclause with
diff --git a/src/include/optimizer/restrictinfo.h b/src/include/optimizer/restrictinfo.h
index 0715df59d68..e19b5a67627 100644
--- a/src/include/optimizer/restrictinfo.h
+++ b/src/include/optimizer/restrictinfo.h
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2005, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/optimizer/restrictinfo.h,v 1.34 2005/10/15 02:49:45 momjian Exp $
+ * $PostgreSQL: pgsql/src/include/optimizer/restrictinfo.h,v 1.34.2.1 2005/11/14 23:54:36 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -19,6 +19,7 @@
 
 extern RestrictInfo *make_restrictinfo(Expr *clause,
 				  bool is_pushed_down,
+				  bool outerjoin_delayed,
 				  Relids required_relids);
 extern List *make_restrictinfo_from_bitmapqual(Path *bitmapqual,
 								  bool is_pushed_down,