Remove ph_may_need from PlaceHolderInfo, with attendant simplifications.

The planner logic that attempted to make a preliminary estimate of the
ph_needed levels for PlaceHolderVars seems to be completely broken by
lateral references.  Fortunately, the potential join order optimization
that this code supported seems to be of relatively little value in
practice; so let's just get rid of it rather than trying to fix it.

Getting rid of this allows fairly substantial simplifications in
placeholder.c, too, so planning in such cases should be a bit faster.

Issue noted while pursuing bugs reported by Jeremy Evans and Antonin
Houska, though this doesn't in itself fix either of their reported cases.
What this does do is prevent an Assert crash in the kind of query
illustrated by the added regression test.  (I'm not sure that the plan for
that query is stable enough across platforms to be usable as a regression
test output ... but we'll soon find out from the buildfarm.)

Back-patch to 9.3.  The problem case can't arise without LATERAL, so
no need to touch older branches.

2 files changed, 9 insertions, 29 deletions

diff --git a/src/backend/optimizer/plan/analyzejoins.c b/src/backend/optimizer/plan/analyzejoins.c
index a7db69c85bf..daab355b1d3 100644
--- a/src/backend/optimizer/plan/analyzejoins.c
+++ b/src/backend/optimizer/plan/analyzejoins.c
@@ -388,8 +388,6 @@ remove_rel_from_query(PlannerInfo *root, int relid, Relids joinrelids)
 			phinfo->ph_eval_at = bms_add_member(phinfo->ph_eval_at, relid);
 
 		phinfo->ph_needed = bms_del_member(phinfo->ph_needed, relid);
-		/* ph_may_need probably isn't used after this, but fix it anyway */
-		phinfo->ph_may_need = bms_del_member(phinfo->ph_may_need, relid);
 	}
 
 	/*
diff --git a/src/backend/optimizer/plan/initsplan.c b/src/backend/optimizer/plan/initsplan.c
index 8efb94b44d4..07c4dddd24e 100644
--- a/src/backend/optimizer/plan/initsplan.c
+++ b/src/backend/optimizer/plan/initsplan.c
@@ -155,10 +155,9 @@ 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.  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.)
+ *	  to create new PlaceHolderInfos; otherwise, the PlaceHolderInfos must
+ *	  already exist, and we should only update their ph_needed.  (This should
+ *	  be true before deconstruct_jointree begins, and false after that.)
  */
 void
 add_vars_to_targetlist(PlannerInfo *root, List *vars,
@@ -196,20 +195,8 @@ add_vars_to_targetlist(PlannerInfo *root, List *vars,
 			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);
-
-			/*
-			 * If we are creating PlaceHolderInfos, mark them with the correct
-			 * maybe-needed locations.	Otherwise, it's too late to change
-			 * that, so we'd better not have set ph_needed to more than
-			 * ph_may_need.
-			 */
-			if (create_new_ph)
-				mark_placeholder_maybe_needed(root, phinfo, where_needed);
-			else
-				Assert(bms_is_subset(phinfo->ph_needed, phinfo->ph_may_need));
 		}
 		else
 			elog(ERROR, "unrecognized node type: %d", (int) nodeTag(node));
@@ -235,7 +222,7 @@ add_vars_to_targetlist(PlannerInfo *root, List *vars,
  * means setting suitable where_needed values for them.
  *
  * This has to run before deconstruct_jointree, since it might result in
- * creation of PlaceHolderInfos or extension of their ph_may_need sets.
+ * creation of PlaceHolderInfos.
  */
 void
 find_lateral_references(PlannerInfo *root)
@@ -1005,11 +992,11 @@ make_outerjoininfo(PlannerInfo *root,
 
 	/*
 	 * Examine PlaceHolderVars.  If a PHV is supposed to be evaluated within
-	 * this join's nullable side, and it may get used above this join, then
-	 * ensure that min_righthand contains the full eval_at set of the PHV.
-	 * This ensures that the PHV actually can be evaluated within the RHS.
-	 * Note that this works only because we should already have determined the
-	 * final eval_at level for any PHV syntactically within this join.
+	 * this join's nullable side, then ensure that min_righthand contains the
+	 * full eval_at set of the PHV.  This ensures that the PHV actually can be
+	 * evaluated within the RHS.  Note that this works only because we should
+	 * already have determined the final eval_at level for any PHV
+	 * syntactically within this join.
 	 */
 	foreach(l, root->placeholder_list)
 	{
@@ -1020,11 +1007,6 @@ make_outerjoininfo(PlannerInfo *root,
 		if (!bms_is_subset(ph_syn_level, right_rels))
 			continue;
 
-		/* We can also ignore it if it's certainly not used above this join */
-		/* XXX this test is probably overly conservative */
-		if (bms_is_subset(phinfo->ph_may_need, min_righthand))
-			continue;
-
 		/* Else, prevent join from being formed before we eval the PHV */
 		min_righthand = bms_add_members(min_righthand, phinfo->ph_eval_at);
 	}