Improve pull_up_subqueries logic so that it doesn't insert unnecessary

PlaceHolderVar nodes in join quals appearing in or below the lowest
outer join that could null the subquery being pulled up.  This improves
the planner's ability to recognize constant join quals, and probably
helps with detection of common sort keys (equivalence classes) as well.

src/backend/optimizer/plan/planner.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/optimizer/plan/planner.c,v 1.254 2009/04/19 19:46:33 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/optimizer/plan/planner.c,v 1.255 2009/04/28 21:31:16 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -324,7 +324,7 @@ subquery_planner(PlannerGlobal *glob, Query *parse,
 	 * this query.
 	 */
 	parse->jointree = (FromExpr *)
-		pull_up_subqueries(root, (Node *) parse->jointree, false, false);
+		pull_up_subqueries(root, (Node *) parse->jointree, NULL, NULL);
 
 	/*
 	 * Detect whether any rangetable entries are RTE_JOIN kind; if not, we can

src/backend/optimizer/prep/prepjointree.c

@@ -16,7 +16,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/optimizer/prep/prepjointree.c,v 1.64 2009/02/27 23:30:29 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/optimizer/prep/prepjointree.c,v 1.65 2009/04/28 21:31:16 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -47,8 +47,8 @@ static Node *pull_up_sublinks_qual_recurse(PlannerInfo *root, Node *node,
 							  Relids available_rels, Node **jtlink);
 static Node *pull_up_simple_subquery(PlannerInfo *root, Node *jtnode,
 						RangeTblEntry *rte,
-						bool below_outer_join,
-						bool append_rel_member);
+						JoinExpr *lowest_outer_join,
+						AppendRelInfo *containing_appendrel);
 static Node *pull_up_simple_union_all(PlannerInfo *root, Node *jtnode,
 						 RangeTblEntry *rte);
 static void pull_up_union_leaf_queries(Node *setOp, PlannerInfo *root,
@@ -63,8 +63,9 @@ static bool is_simple_union_all_recurse(Node *setOp, Query *setOpQuery,
 static List *insert_targetlist_placeholders(PlannerInfo *root, List *tlist,
 											int varno, bool wrap_non_vars);
 static bool is_safe_append_member(Query *subquery);
-static void resolvenew_in_jointree(Node *jtnode, int varno,
-					   RangeTblEntry *rte, List *subtlist);
+static void resolvenew_in_jointree(Node *jtnode, int varno, RangeTblEntry *rte,
+					   List *subtlist, List *subtlist_with_phvs,
+					   JoinExpr *lowest_outer_join);
 static reduce_outer_joins_state *reduce_outer_joins_pass1(Node *jtnode);
 static void reduce_outer_joins_pass2(Node *jtnode,
 						 reduce_outer_joins_state *state,
@@ -440,14 +441,15 @@ inline_set_returning_functions(PlannerInfo *root)
  *		Also, subqueries that are simple UNION ALL structures can be
  *		converted into "append relations".
  *
- * below_outer_join is true if this jointree node is within the nullable
- * side of an outer join.  This forces use of the PlaceHolderVar mechanism
- * for non-nullable targetlist items.
+ * If this jointree node is within the nullable side of an outer join, then
+ * lowest_outer_join references the lowest such JoinExpr node; otherwise it
+ * is NULL.  This forces use of the PlaceHolderVar mechanism for references
+ * to non-nullable targetlist items, but only for references above that join.
  *
- * append_rel_member is true if we are looking at a member subquery of
- * an append relation.	This forces use of the PlaceHolderVar mechanism
- * for all non-Var targetlist items, and puts some additional restrictions
- * on what can be pulled up.
+ * If we are looking at a member subquery of an append relation,
+ * containing_appendrel describes that relation; else it is NULL.
+ * This forces use of the PlaceHolderVar mechanism for all non-Var targetlist
+ * items, and puts some additional restrictions on what can be pulled up.
  *
  * A tricky aspect of this code is that if we pull up a subquery we have
  * to replace Vars that reference the subquery's outputs throughout the
@@ -457,10 +459,15 @@ inline_set_returning_functions(PlannerInfo *root)
  * subquery RangeTblRef entries will be replaced.  Also, we can't turn
  * ResolveNew loose on the whole jointree, because it'll return a mutated
  * copy of the tree; we have to invoke it just on the quals, instead.
+ * This behavior is what makes it reasonable to pass lowest_outer_join as a
+ * pointer rather than some more-indirect way of identifying the lowest OJ.
+ * Likewise, we don't replace append_rel_list members but only their
+ * substructure, so the containing_appendrel reference is safe to use.
  */
 Node *
 pull_up_subqueries(PlannerInfo *root, Node *jtnode,
-				   bool below_outer_join, bool append_rel_member)
+				   JoinExpr *lowest_outer_join,
+				   AppendRelInfo *containing_appendrel)
 {
 	if (jtnode == NULL)
 		return NULL;
@@ -478,10 +485,11 @@ pull_up_subqueries(PlannerInfo *root, Node *jtnode,
 		 */
 		if (rte->rtekind == RTE_SUBQUERY &&
 			is_simple_subquery(rte->subquery) &&
-			(!append_rel_member || is_safe_append_member(rte->subquery)))
+			(containing_appendrel == NULL ||
+			 is_safe_append_member(rte->subquery)))
 			return pull_up_simple_subquery(root, jtnode, rte,
-										   below_outer_join,
-										   append_rel_member);
+										   lowest_outer_join,
+										   containing_appendrel);
 
 		/*
 		 * Alternatively, is it a simple UNION ALL subquery?  If so, flatten
@@ -503,44 +511,44 @@ pull_up_subqueries(PlannerInfo *root, Node *jtnode,
 		FromExpr   *f = (FromExpr *) jtnode;
 		ListCell   *l;
 
-		Assert(!append_rel_member);
+		Assert(containing_appendrel == NULL);
 		foreach(l, f->fromlist)
 			lfirst(l) = pull_up_subqueries(root, lfirst(l),
-										   below_outer_join, false);
+										   lowest_outer_join, NULL);
 	}
 	else if (IsA(jtnode, JoinExpr))
 	{
 		JoinExpr   *j = (JoinExpr *) jtnode;
 
-		Assert(!append_rel_member);
+		Assert(containing_appendrel == NULL);
 		/* Recurse, being careful to tell myself when inside outer join */
 		switch (j->jointype)
 		{
 			case JOIN_INNER:
 				j->larg = pull_up_subqueries(root, j->larg,
-											 below_outer_join, false);
+											 lowest_outer_join, NULL);
 				j->rarg = pull_up_subqueries(root, j->rarg,
-											 below_outer_join, false);
+											 lowest_outer_join, NULL);
 				break;
 			case JOIN_LEFT:
 			case JOIN_SEMI:
 			case JOIN_ANTI:
 				j->larg = pull_up_subqueries(root, j->larg,
-											 below_outer_join, false);
+											 lowest_outer_join, NULL);
 				j->rarg = pull_up_subqueries(root, j->rarg,
-											 true, false);
+											 j, NULL);
 				break;
 			case JOIN_FULL:
 				j->larg = pull_up_subqueries(root, j->larg,
-											 true, false);
+											 j, NULL);
 				j->rarg = pull_up_subqueries(root, j->rarg,
-											 true, false);
+											 j, NULL);
 				break;
 			case JOIN_RIGHT:
 				j->larg = pull_up_subqueries(root, j->larg,
-											 true, false);
+											 j, NULL);
 				j->rarg = pull_up_subqueries(root, j->rarg,
-											 below_outer_join, false);
+											 lowest_outer_join, NULL);
 				break;
 			default:
 				elog(ERROR, "unrecognized join type: %d",
@@ -562,10 +570,14 @@ pull_up_subqueries(PlannerInfo *root, Node *jtnode,
  * subquery by pull_up_subqueries.	We return the replacement jointree node,
  * or jtnode itself if we determine that the subquery can't be pulled up after
  * all.
+ *
+ * rte is the RangeTblEntry referenced by jtnode.  Remaining parameters are
+ * as for pull_up_subqueries.
  */
 static Node *
 pull_up_simple_subquery(PlannerInfo *root, Node *jtnode, RangeTblEntry *rte,
-						bool below_outer_join, bool append_rel_member)
+						JoinExpr *lowest_outer_join,
+						AppendRelInfo *containing_appendrel)
 {
 	Query	   *parse = root->parse;
 	int			varno = ((RangeTblRef *) jtnode)->rtindex;
@@ -573,7 +585,8 @@ pull_up_simple_subquery(PlannerInfo *root, Node *jtnode, RangeTblEntry *rte,
 	PlannerInfo *subroot;
 	int			rtoffset;
 	List	   *subtlist;
-	ListCell   *rt;
+	List	   *subtlist_with_phvs;
+	ListCell   *lc;
 
 	/*
 	 * Need a modifiable copy of the subquery to hack on.  Even if we didn't
@@ -624,13 +637,13 @@ pull_up_simple_subquery(PlannerInfo *root, Node *jtnode, RangeTblEntry *rte,
 	 * pull_up_subqueries' processing is complete for its jointree and
 	 * rangetable.
 	 *
-	 * Note: below_outer_join = false is correct here even if we are within an
-	 * outer join in the upper query; the lower query starts with a clean
+	 * Note: we should pass NULL for containing-join info even if we are within
+	 * an outer join in the upper query; the lower query starts with a clean
 	 * slate for outer-join semantics.  Likewise, we say we aren't handling an
 	 * appendrel member.
 	 */
 	subquery->jointree = (FromExpr *)
-		pull_up_subqueries(subroot, (Node *) subquery->jointree, false, false);
+		pull_up_subqueries(subroot, (Node *) subquery->jointree, NULL, NULL);
 
 	/*
 	 * Now we must recheck whether the subquery is still simple enough to pull
@@ -641,7 +654,7 @@ pull_up_simple_subquery(PlannerInfo *root, Node *jtnode, RangeTblEntry *rte,
 	 * pull_up_subqueries.
 	 */
 	if (is_simple_subquery(subquery) &&
-		(!append_rel_member || is_safe_append_member(subquery)))
+		(containing_appendrel == NULL || is_safe_append_member(subquery)))
 	{
 		/* good to go */
 	}
@@ -677,51 +690,82 @@ pull_up_simple_subquery(PlannerInfo *root, Node *jtnode, RangeTblEntry *rte,
 	/*
 	 * The subquery's targetlist items are now in the appropriate form to
 	 * insert into the top query, but if we are under an outer join then
-	 * non-nullable items have to be turned into PlaceHolderVars.  If we
+	 * non-nullable items may have to be turned into PlaceHolderVars.  If we
 	 * are dealing with an appendrel member then anything that's not a
 	 * simple Var has to be turned into a PlaceHolderVar.
 	 */
-	if (below_outer_join || append_rel_member)
-		subtlist = insert_targetlist_placeholders(root, subquery->targetList,
-												  varno, append_rel_member);
-	else
 	subtlist = subquery->targetList;
+	if (lowest_outer_join != NULL || containing_appendrel != NULL)
+		subtlist_with_phvs = insert_targetlist_placeholders(root,
+															subtlist,
+															varno,
+															containing_appendrel != NULL);
+	else
+		subtlist_with_phvs = subtlist;
 
 	/*
 	 * Replace all of the top query's references to the subquery's outputs
 	 * with copies of the adjusted subtlist items, being careful not to
 	 * replace any of the jointree structure. (This'd be a lot cleaner if we
-	 * could use query_tree_mutator.)
+	 * could use query_tree_mutator.)  We have to use PHVs in the targetList,
+	 * returningList, and havingQual, since those are certainly above any
+	 * outer join.  resolvenew_in_jointree tracks its location in the jointree
+	 * and uses PHVs or not appropriately.
 	 */
 	parse->targetList = (List *)
 		ResolveNew((Node *) parse->targetList,
 				   varno, 0, rte,
-				   subtlist, CMD_SELECT, 0);
+				   subtlist_with_phvs, CMD_SELECT, 0);
 	parse->returningList = (List *)
 		ResolveNew((Node *) parse->returningList,
 				   varno, 0, rte,
-				   subtlist, CMD_SELECT, 0);
-	resolvenew_in_jointree((Node *) parse->jointree, varno,
-						   rte, subtlist);
+				   subtlist_with_phvs, CMD_SELECT, 0);
+	resolvenew_in_jointree((Node *) parse->jointree, varno, rte,
+						   subtlist, subtlist_with_phvs,
+						   lowest_outer_join);
 	Assert(parse->setOperations == NULL);
 	parse->havingQual =
 		ResolveNew(parse->havingQual,
 				   varno, 0, rte,
-				   subtlist, CMD_SELECT, 0);
-	root->append_rel_list = (List *)
-		ResolveNew((Node *) root->append_rel_list,
+				   subtlist_with_phvs, CMD_SELECT, 0);
+
+	/*
+	 * Replace references in the translated_vars lists of appendrels.
+	 * When pulling up an appendrel member, we do not need PHVs in the list
+	 * of the parent appendrel --- there isn't any outer join between.
+	 * Elsewhere, use PHVs for safety.  (This analysis could be made tighter
+	 * but it seems unlikely to be worth much trouble.)
+	 */
+	foreach(lc, root->append_rel_list)
+	{
+		AppendRelInfo *appinfo = (AppendRelInfo *) lfirst(lc);
+
+		appinfo->translated_vars = (List *)
+			ResolveNew((Node *) appinfo->translated_vars,
 					   varno, 0, rte,
-				   subtlist, CMD_SELECT, 0);
+					   (appinfo == containing_appendrel) ?
+					   subtlist : subtlist_with_phvs,
+					   CMD_SELECT, 0);
+	}
 
-	foreach(rt, parse->rtable)
+	/*
+	 * Replace references in the joinaliasvars lists of join RTEs.
+	 *
+	 * You might think that we could avoid using PHVs for alias vars of joins
+	 * below lowest_outer_join, but that doesn't work because the alias vars
+	 * could be referenced above that join; we need the PHVs to be present
+	 * in such references after the alias vars get flattened.  (It might be
+	 * worth trying to be smarter here, someday.)
+	 */
+	foreach(lc, parse->rtable)
 	{
-		RangeTblEntry *otherrte = (RangeTblEntry *) lfirst(rt);
+		RangeTblEntry *otherrte = (RangeTblEntry *) lfirst(lc);
 
 		if (otherrte->rtekind == RTE_JOIN)
 			otherrte->joinaliasvars = (List *)
 				ResolveNew((Node *) otherrte->joinaliasvars,
 						   varno, 0, rte,
-						   subtlist, CMD_SELECT, 0);
+						   subtlist_with_phvs, CMD_SELECT, 0);
 	}
 
 	/*
@@ -884,13 +928,13 @@ pull_up_union_leaf_queries(Node *setOp, PlannerInfo *root, int parentRTindex,
 		/*
 		 * Recursively apply pull_up_subqueries to the new child RTE.  (We
 		 * must build the AppendRelInfo first, because this will modify it.)
-		 * Note that we can pass below_outer_join = false even if we're
+		 * Note that we can pass NULL for containing-join info even if we're
 		 * actually under an outer join, because the child's expressions
 		 * aren't going to propagate up above the join.
 		 */
 		rtr = makeNode(RangeTblRef);
 		rtr->rtindex = childRTindex;
-		(void) pull_up_subqueries(root, (Node *) rtr, false, true);
+		(void) pull_up_subqueries(root, (Node *) rtr, NULL, appinfo);
 	}
 	else if (IsA(setOp, SetOperationStmt))
 	{
@@ -1096,7 +1140,7 @@ is_simple_union_all_recurse(Node *setOp, Query *setOpQuery, List *colTypes)
  *	  Insert PlaceHolderVar nodes into any non-junk targetlist items that are
  *	  not simple variables or strict functions of simple variables (and hence
  *	  might not correctly go to NULL when examined above the point of an outer
- *	  join).  We assume we can modify the tlist items in-place.
+ *	  join).
  *
  * varno is the upper-query relid of the subquery; this is used as the
  * syntactic location of the PlaceHolderVars.
@@ -1107,15 +1151,20 @@ static List *
 insert_targetlist_placeholders(PlannerInfo *root, List *tlist,
 							   int varno, bool wrap_non_vars)
 {
+	List	   *result = NIL;
 	ListCell   *lc;
 
 	foreach(lc, tlist)
 	{
 		TargetEntry *tle = (TargetEntry *) lfirst(lc);
+		TargetEntry *newtle;
 
-		/* ignore resjunk columns */
+		/* resjunk columns need not be changed */
 		if (tle->resjunk)
+		{
+			result = lappend(result, tle);
 			continue;
+		}
 
 		/*
 		 * Simple Vars always escape being wrapped.  This is common enough
@@ -1123,7 +1172,10 @@ insert_targetlist_placeholders(PlannerInfo *root, List *tlist,
 		 */
 		if (tle->expr && IsA(tle->expr, Var) &&
 			((Var *) tle->expr)->varlevelsup == 0)
+		{
+			result = lappend(result, tle);
 			continue;
+		}
 
 		if (!wrap_non_vars)
 		{
@@ -1136,15 +1188,22 @@ insert_targetlist_placeholders(PlannerInfo *root, List *tlist,
 			 */
 			if (contain_vars_of_level((Node *) tle->expr, 0) &&
 				!contain_nonstrict_functions((Node *) tle->expr))
+			{
+				result = lappend(result, tle);
 				continue;
 			}
+		}
 
 		/* Else wrap it in a PlaceHolderVar */
-		tle->expr = (Expr *) make_placeholder_expr(root,
+		newtle = makeNode(TargetEntry);
+		memcpy(newtle, tle, sizeof(TargetEntry));
+		newtle->expr = (Expr *)
+			make_placeholder_expr(root,
 								  tle->expr,
 								  bms_make_singleton(varno));
+		result = lappend(result, newtle);
 	}
-	return tlist;
+	return result;
 }
 
 /*
@@ -1187,10 +1246,15 @@ is_safe_append_member(Query *subquery)
  * Helper routine for pull_up_subqueries: do ResolveNew on every expression
  * in the jointree, without changing the jointree structure itself.  Ugly,
  * but there's no other way...
+ *
+ * If we are above lowest_outer_join then use subtlist_with_phvs; at or
+ * below it, use subtlist.  (When no outer joins are in the picture,
+ * these will be the same list.)
  */
 static void
-resolvenew_in_jointree(Node *jtnode, int varno,
-					   RangeTblEntry *rte, List *subtlist)
+resolvenew_in_jointree(Node *jtnode, int varno, RangeTblEntry *rte,
+					   List *subtlist, List *subtlist_with_phvs,
+					   JoinExpr *lowest_outer_join)
 {
 	if (jtnode == NULL)
 		return;
@@ -1204,20 +1268,32 @@ resolvenew_in_jointree(Node *jtnode, int varno,
 		ListCell   *l;
 
 		foreach(l, f->fromlist)
-			resolvenew_in_jointree(lfirst(l), varno, rte, subtlist);
+			resolvenew_in_jointree(lfirst(l), varno, rte,
+								   subtlist, subtlist_with_phvs,
+								   lowest_outer_join);
 		f->quals = ResolveNew(f->quals,
 							  varno, 0, rte,
-							  subtlist, CMD_SELECT, 0);
+							  subtlist_with_phvs, CMD_SELECT, 0);
 	}
 	else if (IsA(jtnode, JoinExpr))
 	{
 		JoinExpr   *j = (JoinExpr *) jtnode;
 
-		resolvenew_in_jointree(j->larg, varno, rte, subtlist);
-		resolvenew_in_jointree(j->rarg, varno, rte, subtlist);
+		if (j == lowest_outer_join)
+		{
+			/* no more PHVs in or below this join */
+			subtlist_with_phvs = subtlist;
+			lowest_outer_join = NULL;
+		}
+		resolvenew_in_jointree(j->larg, varno, rte,
+							   subtlist, subtlist_with_phvs,
+							   lowest_outer_join);
+		resolvenew_in_jointree(j->rarg, varno, rte,
+							   subtlist, subtlist_with_phvs,
+							   lowest_outer_join);
 		j->quals = ResolveNew(j->quals,
 							  varno, 0, rte,
-							  subtlist, CMD_SELECT, 0);
+							  subtlist_with_phvs, CMD_SELECT, 0);
 
 		/*
 		 * We don't bother to update the colvars list, since it won't be used

src/include/optimizer/prep.h

@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2009, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/optimizer/prep.h,v 1.64 2009/01/01 17:24:00 momjian Exp $
+ * $PostgreSQL: pgsql/src/include/optimizer/prep.h,v 1.65 2009/04/28 21:31:16 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -24,7 +24,8 @@
 extern void pull_up_sublinks(PlannerInfo *root);
 extern void inline_set_returning_functions(PlannerInfo *root);
 extern Node *pull_up_subqueries(PlannerInfo *root, Node *jtnode,
-				   bool below_outer_join, bool append_rel_member);
+				   JoinExpr *lowest_outer_join,
+				   AppendRelInfo *containing_appendrel);
 extern void reduce_outer_joins(PlannerInfo *root);
 extern Relids get_relids_in_jointree(Node *jtnode, bool include_joins);
 extern Relids get_relids_for_join(PlannerInfo *root, int joinrelid);