postgres
diff --git a/‎src/backend/nodes/outfuncs.c
Copy file name to clipboardExpand all lines: src/backend/nodes/outfuncs.c
+1Lines changed: 1 addition & 0 deletions b/‎src/backend/nodes/outfuncs.c
Copy file name to clipboardExpand all lines: src/backend/nodes/outfuncs.c
+1Lines changed: 1 addition & 0 deletions
diff --git a/‎src/backend/optimizer/path/equivclass.c
Copy file name to clipboardExpand all lines: src/backend/optimizer/path/equivclass.c
+17-2Lines changed: 17 additions & 2 deletions b/‎src/backend/optimizer/path/equivclass.c
Copy file name to clipboardExpand all lines: src/backend/optimizer/path/equivclass.c
+17-2Lines changed: 17 additions & 2 deletions
diff --git a/‎src/backend/optimizer/path/pathkeys.c
Copy file name to clipboardExpand all lines: src/backend/optimizer/path/pathkeys.c
+28-4Lines changed: 28 additions & 4 deletions b/‎src/backend/optimizer/path/pathkeys.c
Copy file name to clipboardExpand all lines: src/backend/optimizer/path/pathkeys.c
+28-4Lines changed: 28 additions & 4 deletions
diff --git a/‎src/backend/optimizer/plan/initsplan.c
Copy file name to clipboardExpand all lines: src/backend/optimizer/plan/initsplan.c
+20Lines changed: 20 additions & 0 deletions b/‎src/backend/optimizer/plan/initsplan.c
Copy file name to clipboardExpand all lines: src/backend/optimizer/plan/initsplan.c
+20Lines changed: 20 additions & 0 deletions
diff --git a/‎src/include/nodes/relation.h
Copy file name to clipboardExpand all lines: src/include/nodes/relation.h
+10-1Lines changed: 10 additions & 1 deletion b/‎src/include/nodes/relation.h
Copy file name to clipboardExpand all lines: src/include/nodes/relation.h
+10-1Lines changed: 10 additions & 1 deletion
diff --git a/‎src/include/optimizer/paths.h
Copy file name to clipboardExpand all lines: src/include/optimizer/paths.h
+1Lines changed: 1 addition & 0 deletions b/‎src/include/optimizer/paths.h
Copy file name to clipboardExpand all lines: src/include/optimizer/paths.h
+1Lines changed: 1 addition & 0 deletions
diff --git a/‎src/test/regress/expected/join.out
Copy file name to clipboardExpand all lines: src/test/regress/expected/join.out
+29Lines changed: 29 additions & 0 deletions b/‎src/test/regress/expected/join.out
Copy file name to clipboardExpand all lines: src/test/regress/expected/join.out
+29Lines changed: 29 additions & 0 deletions
diff --git a/‎src/test/regress/sql/join.sql
Copy file name to clipboardExpand all lines: src/test/regress/sql/join.sql
+13Lines changed: 13 additions & 0 deletions b/‎src/test/regress/sql/join.sql
Copy file name to clipboardExpand all lines: src/test/regress/sql/join.sql
+13Lines changed: 13 additions & 0 deletions
@@ -1698,6 +1698,7 @@ _outPlannerInfo(StringInfo str, const PlannerInfo *node)
 	WRITE_UINT_FIELD(query_level);
 	WRITE_NODE_FIELD(plan_params);
 	WRITE_BITMAPSET_FIELD(all_baserels);
+	WRITE_BITMAPSET_FIELD(nullable_baserels);
 	WRITE_NODE_FIELD(join_rel_list);
 	WRITE_INT_FIELD(join_cur_level);
 	WRITE_NODE_FIELD(init_plans);
 
@@ -510,6 +510,13 @@ add_eq_member(EquivalenceClass *ec, Expr *expr, Relids relids,
  *	  equivalence class it is a member of; if none, optionally build a new
  *	  single-member EquivalenceClass for it.
  *
+ * expr is the expression, and nullable_relids is the set of base relids
+ * that are potentially nullable below it.	We actually only care about
+ * the set of such relids that are used in the expression; but for caller
+ * convenience, we perform that intersection step here.  The caller need
+ * only be sure that nullable_relids doesn't omit any nullable rels that
+ * might appear in the expr.
+ *
  * sortref is the SortGroupRef of the originating SortGroupClause, if any,
  * or zero if not.	(It should never be zero if the expression is volatile!)
  *
@@ -538,13 +545,15 @@ add_eq_member(EquivalenceClass *ec, Expr *expr, Relids relids,
 EquivalenceClass *
 get_eclass_for_sort_expr(PlannerInfo *root,
 						 Expr *expr,
+						 Relids nullable_relids,
 						 List *opfamilies,
 						 Oid opcintype,
 						 Oid collation,
 						 Index sortref,
 						 Relids rel,
 						 bool create_it)
 {
+	Relids		expr_relids;
 	EquivalenceClass *newec;
 	EquivalenceMember *newem;
 	ListCell   *lc1;
@@ -555,6 +564,12 @@ get_eclass_for_sort_expr(PlannerInfo *root,
 	 */
 	expr = canonicalize_ec_expression(expr, opcintype, collation);
 
+	/*
+	 * Get the precise set of nullable relids appearing in the expression.
+	 */
+	expr_relids = pull_varnos((Node *) expr);
+	nullable_relids = bms_intersect(nullable_relids, expr_relids);
+
 	/*
 	 * Scan through the existing EquivalenceClasses for a match
 	 */
@@ -629,8 +644,8 @@ get_eclass_for_sort_expr(PlannerInfo *root,
 	if (newec->ec_has_volatile && sortref == 0) /* should not happen */
 		elog(ERROR, "volatile EquivalenceClass has no sortref");
 
-	newem = add_eq_member(newec, copyObject(expr), pull_varnos((Node *) expr),
-						  NULL, false, opcintype);
+	newem = add_eq_member(newec, copyObject(expr), expr_relids,
+						  nullable_relids, false, opcintype);
 
 	/*
 	 * add_eq_member doesn't check for volatile functions, set-returning
 
@@ -154,6 +154,9 @@ pathkey_is_redundant(PathKey *new_pathkey, List *pathkeys)
  *	  Given an expression and sort-order information, create a PathKey.
  *	  The result is always a "canonical" PathKey, but it might be redundant.
  *
+ * expr is the expression, and nullable_relids is the set of base relids
+ * that are potentially nullable below it.
+ *
  * If the PathKey is being generated from a SortGroupClause, sortref should be
  * the SortGroupClause's SortGroupRef; otherwise zero.
  *
@@ -169,6 +172,7 @@ pathkey_is_redundant(PathKey *new_pathkey, List *pathkeys)
 static PathKey *
 make_pathkey_from_sortinfo(PlannerInfo *root,
 						   Expr *expr,
+						   Relids nullable_relids,
 						   Oid opfamily,
 						   Oid opcintype,
 						   Oid collation,
@@ -204,8 +208,8 @@ make_pathkey_from_sortinfo(PlannerInfo *root,
 			 equality_op);
 
 	/* Now find or (optionally) create a matching EquivalenceClass */
-	eclass = get_eclass_for_sort_expr(root, expr, opfamilies,
-									  opcintype, collation,
+	eclass = get_eclass_for_sort_expr(root, expr, nullable_relids,
+									  opfamilies, opcintype, collation,
 									  sortref, rel, create_it);
 
 	/* Fail if no EC and !create_it */
@@ -227,6 +231,7 @@ make_pathkey_from_sortinfo(PlannerInfo *root,
 static PathKey *
 make_pathkey_from_sortop(PlannerInfo *root,
 						 Expr *expr,
+						 Relids nullable_relids,
 						 Oid ordering_op,
 						 bool nulls_first,
 						 Index sortref,
@@ -248,6 +253,7 @@ make_pathkey_from_sortop(PlannerInfo *root,
 
 	return make_pathkey_from_sortinfo(root,
 									  expr,
+									  nullable_relids,
 									  opfamily,
 									  opcintype,
 									  collation,
@@ -461,9 +467,13 @@ build_index_pathkeys(PlannerInfo *root,
 			nulls_first = index->nulls_first[i];
 		}
 
-		/* OK, try to make a canonical pathkey for this sort key */
+		/*
+		 * OK, try to make a canonical pathkey for this sort key.  Note we're
+		 * underneath any outer joins, so nullable_relids should be NULL.
+		 */
 		cpathkey = make_pathkey_from_sortinfo(root,
 											  indexkey,
+											  NULL,
 											  index->sortopfamily[i],
 											  index->opcintype[i],
 											  index->indexcollations[i],
@@ -551,11 +561,14 @@ convert_subquery_pathkeys(PlannerInfo *root, RelOptInfo *rel,
 				 * expression is *not* volatile in the outer query: it's just
 				 * a Var referencing whatever the subquery emitted. (IOW, the
 				 * outer query isn't going to re-execute the volatile
-				 * expression itself.)	So this is okay.
+				 * expression itself.)	So this is okay.  Likewise, it's
+				 * correct to pass nullable_relids = NULL, because we're
+				 * underneath any outer joins appearing in the outer query.
 				 */
 				outer_ec =
 					get_eclass_for_sort_expr(root,
 											 outer_expr,
+											 NULL,
 											 sub_eclass->ec_opfamilies,
 											 sub_member->em_datatype,
 											 sub_eclass->ec_collation,
@@ -643,6 +656,7 @@ convert_subquery_pathkeys(PlannerInfo *root, RelOptInfo *rel,
 					/* See if we have a matching EC for that */
 					outer_ec = get_eclass_for_sort_expr(root,
 														outer_expr,
+														NULL,
 												   sub_eclass->ec_opfamilies,
 														sub_expr_type,
 														sub_expr_coll,
@@ -748,6 +762,13 @@ build_join_pathkeys(PlannerInfo *root,
  * The resulting PathKeys are always in canonical form.  (Actually, there
  * is no longer any code anywhere that creates non-canonical PathKeys.)
  *
+ * We assume that root->nullable_baserels is the set of base relids that could
+ * have gone to NULL below the SortGroupClause expressions.  This is okay if
+ * the expressions came from the query's top level (ORDER BY, DISTINCT, etc)
+ * and if this function is only invoked after deconstruct_jointree.  In the
+ * future we might have to make callers pass in the appropriate
+ * nullable-relids set, but for now it seems unnecessary.
+ *
  * 'sortclauses' is a list of SortGroupClause nodes
  * 'tlist' is the targetlist to find the referenced tlist entries in
  */
@@ -769,6 +790,7 @@ make_pathkeys_for_sortclauses(PlannerInfo *root,
 		Assert(OidIsValid(sortcl->sortop));
 		pathkey = make_pathkey_from_sortop(root,
 										   sortkey,
+										   root->nullable_baserels,
 										   sortcl->sortop,
 										   sortcl->nulls_first,
 										   sortcl->tleSortGroupRef,
@@ -824,6 +846,7 @@ initialize_mergeclause_eclasses(PlannerInfo *root, RestrictInfo *restrictinfo)
 	restrictinfo->left_ec =
 		get_eclass_for_sort_expr(root,
 								 (Expr *) get_leftop(clause),
+								 restrictinfo->nullable_relids,
 								 restrictinfo->mergeopfamilies,
 								 lefttype,
 								 ((OpExpr *) clause)->inputcollid,
@@ -833,6 +856,7 @@ initialize_mergeclause_eclasses(PlannerInfo *root, RestrictInfo *restrictinfo)
 	restrictinfo->right_ec =
 		get_eclass_for_sort_expr(root,
 								 (Expr *) get_rightop(clause),
+								 restrictinfo->nullable_relids,
 								 restrictinfo->mergeopfamilies,
 								 righttype,
 								 ((OpExpr *) clause)->inputcollid,
 
@@ -649,6 +649,9 @@ deconstruct_jointree(PlannerInfo *root)
 	Assert(root->parse->jointree != NULL &&
 		   IsA(root->parse->jointree, FromExpr));
 
+	/* this is filled as we scan the jointree */
+	root->nullable_baserels = NULL;
+
 	result = deconstruct_recurse(root, (Node *) root->parse->jointree, false,
 								 &qualscope, &inner_join_rels,
 								 &postponed_qual_list);
@@ -790,6 +793,7 @@ deconstruct_recurse(PlannerInfo *root, Node *jtnode, bool below_outer_join,
 					left_inners,
 					right_inners,
 					nonnullable_rels,
+					nullable_rels,
 					ojscope;
 		List	   *leftjoinlist,
 				   *rightjoinlist;
@@ -823,6 +827,8 @@ deconstruct_recurse(PlannerInfo *root, Node *jtnode, bool below_outer_join,
 				*inner_join_rels = *qualscope;
 				/* Inner join adds no restrictions for quals */
 				nonnullable_rels = NULL;
+				/* and it doesn't force anything to null, either */
+				nullable_rels = NULL;
 				break;
 			case JOIN_LEFT:
 			case JOIN_ANTI:
@@ -837,6 +843,7 @@ deconstruct_recurse(PlannerInfo *root, Node *jtnode, bool below_outer_join,
 				*qualscope = bms_union(leftids, rightids);
 				*inner_join_rels = bms_union(left_inners, right_inners);
 				nonnullable_rels = leftids;
+				nullable_rels = rightids;
 				break;
 			case JOIN_SEMI:
 				leftjoinlist = deconstruct_recurse(root, j->larg,
@@ -851,6 +858,13 @@ deconstruct_recurse(PlannerInfo *root, Node *jtnode, bool below_outer_join,
 				*inner_join_rels = bms_union(left_inners, right_inners);
 				/* Semi join adds no restrictions for quals */
 				nonnullable_rels = NULL;
+
+				/*
+				 * Theoretically, a semijoin would null the RHS; but since the
+				 * RHS can't be accessed above the join, this is immaterial
+				 * and we needn't account for it.
+				 */
+				nullable_rels = NULL;
 				break;
 			case JOIN_FULL:
 				leftjoinlist = deconstruct_recurse(root, j->larg,
@@ -865,16 +879,22 @@ deconstruct_recurse(PlannerInfo *root, Node *jtnode, bool below_outer_join,
 				*inner_join_rels = bms_union(left_inners, right_inners);
 				/* each side is both outer and inner */
 				nonnullable_rels = *qualscope;
+				nullable_rels = *qualscope;
 				break;
 			default:
 				/* JOIN_RIGHT was eliminated during reduce_outer_joins() */
 				elog(ERROR, "unrecognized join type: %d",
 					 (int) j->jointype);
 				nonnullable_rels = NULL;		/* keep compiler quiet */
+				nullable_rels = NULL;
 				leftjoinlist = rightjoinlist = NIL;
 				break;
 		}
 
+		/* Report all rels that will be nulled anywhere in the jointree */
+		root->nullable_baserels = bms_add_members(root->nullable_baserels,
+												  nullable_rels);
+
 		/*
 		 * For an OJ, form the SpecialJoinInfo now, because we need the OJ's
 		 * semantic scope (ojscope) to pass to distribute_qual_to_rels.  But
 
@@ -150,10 +150,19 @@ typedef struct PlannerInfo
 	/*
 	 * all_baserels is a Relids set of all base relids (but not "other"
 	 * relids) in the query; that is, the Relids identifier of the final join
-	 * we need to form.
+	 * we need to form.  This is computed in make_one_rel, just before we
+	 * start making Paths.
 	 */
 	Relids		all_baserels;
 
+	/*
+	 * nullable_baserels is a Relids set of base relids that are nullable by
+	 * some outer join in the jointree; these are rels that are potentially
+	 * nullable below the WHERE clause, SELECT targetlist, etc.  This is
+	 * computed in deconstruct_jointree.
+	 */
+	Relids		nullable_baserels;
+
 	/*
 	 * join_rel_list is a list of all join-relation RelOptInfos we have
 	 * considered in this planning run.  For small problems we just scan the
 
@@ -109,6 +109,7 @@ extern Expr *canonicalize_ec_expression(Expr *expr,
 extern void reconsider_outer_join_clauses(PlannerInfo *root);
 extern EquivalenceClass *get_eclass_for_sort_expr(PlannerInfo *root,
 						 Expr *expr,
+						 Relids nullable_relids,
 						 List *opfamilies,
 						 Oid opcintype,
 						 Oid collation,
 
@@ -2900,6 +2900,35 @@ select f1, unique2, case when unique2 is null then f1 else 0 end
   0 |       0 |    0
 (1 row)
 
+--
+-- another case with equivalence clauses above outer joins (bug #8591)
+--
+explain (costs off)
+select a.unique1, b.unique1, c.unique1, coalesce(b.twothousand, a.twothousand)
+  from tenk1 a left join tenk1 b on b.thousand = a.unique1                        left join tenk1 c on c.unique2 = coalesce(b.twothousand, a.twothousand)
+  where a.unique2 = 5530 and coalesce(b.twothousand, a.twothousand) = 44;
+                                         QUERY PLAN                                          
+---------------------------------------------------------------------------------------------
+ Nested Loop Left Join
+   ->  Nested Loop Left Join
+         Filter: (COALESCE(b.twothousand, a.twothousand) = 44)
+         ->  Index Scan using tenk1_unique2 on tenk1 a
+               Index Cond: (unique2 = 5530)
+         ->  Bitmap Heap Scan on tenk1 b
+               Recheck Cond: (thousand = a.unique1)
+               ->  Bitmap Index Scan on tenk1_thous_tenthous
+                     Index Cond: (thousand = a.unique1)
+   ->  Index Scan using tenk1_unique2 on tenk1 c
+         Index Cond: ((unique2 = COALESCE(b.twothousand, a.twothousand)) AND (unique2 = 44))
+(11 rows)
+
+select a.unique1, b.unique1, c.unique1, coalesce(b.twothousand, a.twothousand)
+  from tenk1 a left join tenk1 b on b.thousand = a.unique1                        left join tenk1 c on c.unique2 = coalesce(b.twothousand, a.twothousand)
+  where a.unique2 = 5530 and coalesce(b.twothousand, a.twothousand) = 44;
+ unique1 | unique1 | unique1 | coalesce 
+---------+---------+---------+----------
+(0 rows)
+
 --
 -- test ability to push constants through outer join clauses
 --
 
@@ -790,6 +790,19 @@ select f1, unique2, case when unique2 is null then f1 else 0 end
   from int4_tbl a left join tenk1 b on f1 = unique2
   where (case when unique2 is null then f1 else 0 end) = 0;
 
+--
+-- another case with equivalence clauses above outer joins (bug #8591)
+--
+
+explain (costs off)
+select a.unique1, b.unique1, c.unique1, coalesce(b.twothousand, a.twothousand)
+  from tenk1 a left join tenk1 b on b.thousand = a.unique1                        left join tenk1 c on c.unique2 = coalesce(b.twothousand, a.twothousand)
+  where a.unique2 = 5530 and coalesce(b.twothousand, a.twothousand) = 44;
+
+select a.unique1, b.unique1, c.unique1, coalesce(b.twothousand, a.twothousand)
+  from tenk1 a left join tenk1 b on b.thousand = a.unique1                        left join tenk1 c on c.unique2 = coalesce(b.twothousand, a.twothousand)
+  where a.unique2 = 5530 and coalesce(b.twothousand, a.twothousand) = 44;
+
 --
 -- test ability to push constants through outer join clauses
 --