Fix nested PlaceHolderVar expressions that appear only in targetlists.

A PlaceHolderVar's expression might contain another, lower-level
PlaceHolderVar.  If the outer PlaceHolderVar is used, the inner one
certainly will be also, and so we have to make sure that both of them get
into the placeholder_list with correct ph_may_need values during the
initial pre-scan of the query (before deconstruct_jointree starts).
We did this correctly for PlaceHolderVars appearing in the query quals,
but overlooked the issue for those appearing in the top-level targetlist;
with the result that nested placeholders referenced only in the targetlist
did not work correctly, as illustrated in bug #6154.

While at it, add some error checking to find_placeholder_info to ensure
that we don't try to create new placeholders after it's too late to do so;
they have to all be created before deconstruct_jointree starts.

Back-patch to 8.4 where the PlaceHolderVar mechanism was introduced.

1 files changed, 17 insertions, 10 deletions

diff --git a/src/backend/optimizer/plan/initsplan.c b/src/backend/optimizer/plan/initsplan.c
index 394cda32e57..9cfc56ea541 100644
--- a/src/backend/optimizer/plan/initsplan.c
+++ b/src/backend/optimizer/plan/initsplan.c
@@ -137,7 +137,7 @@ build_base_rel_tlists(PlannerInfo *root, List *final_tlist)
 
 	if (tlist_vars != NIL)
 	{
-		add_vars_to_targetlist(root, tlist_vars, bms_make_singleton(0));
+		add_vars_to_targetlist(root, tlist_vars, bms_make_singleton(0), true);
 		list_free(tlist_vars);
 	}
 }
@@ -151,10 +151,15 @@ build_base_rel_tlists(PlannerInfo *root, List *final_tlist)
  *
  *	  The list may also contain PlaceHolderVars.  These don't necessarily
  *	  have a single owning relation; we keep their attr_needed info in
- *	  root->placeholder_list instead.
+ *	  root->placeholder_list instead.  If create_new_ph is true, it's OK
+ *	  to create new PlaceHolderInfos, and we also have to update ph_may_need;
+ *	  otherwise, the PlaceHolderInfos must already exist, and we should only
+ *	  update their ph_needed.  (It should be true before deconstruct_jointree
+ *	  begins, and false after that.)
  */
 void
-add_vars_to_targetlist(PlannerInfo *root, List *vars, Relids where_needed)
+add_vars_to_targetlist(PlannerInfo *root, List *vars,
+					   Relids where_needed, bool create_new_ph)
 {
 	ListCell   *temp;
 
@@ -185,18 +190,20 @@ add_vars_to_targetlist(PlannerInfo *root, List *vars, Relids where_needed)
 		else if (IsA(node, PlaceHolderVar))
 		{
 			PlaceHolderVar *phv = (PlaceHolderVar *) node;
-			PlaceHolderInfo *phinfo = find_placeholder_info(root, phv);
+			PlaceHolderInfo *phinfo = find_placeholder_info(root, phv,
+															create_new_ph);
 
+			/* Always adjust ph_needed */
 			phinfo->ph_needed = bms_add_members(phinfo->ph_needed,
 												where_needed);
 
 			/*
-			 * Update ph_may_need too.	This is currently only necessary when
-			 * being called from build_base_rel_tlists, but we may as well do
-			 * it always.
+			 * If we are creating PlaceHolderInfos, mark them with the
+			 * correct maybe-needed locations.  Otherwise, it's too late to
+			 * change that.
 			 */
-			phinfo->ph_may_need = bms_add_members(phinfo->ph_may_need,
-												  where_needed);
+			if (create_new_ph)
+				mark_placeholder_maybe_needed(root, phinfo, where_needed);
 		}
 		else
 			elog(ERROR, "unrecognized node type: %d", (int) nodeTag(node));
@@ -1035,7 +1042,7 @@ distribute_qual_to_rels(PlannerInfo *root, Node *clause,
 										   PVC_RECURSE_AGGREGATES,
 										   PVC_INCLUDE_PLACEHOLDERS);
 
-		add_vars_to_targetlist(root, vars, relids);
+		add_vars_to_targetlist(root, vars, relids, false);
 		list_free(vars);
 	}