Fix planner problems with LATERAL references in PlaceHolderVars.

The planner largely failed to consider the possibility that a
PlaceHolderVar's expression might contain a lateral reference to a Var
coming from somewhere outside the PHV's syntactic scope.  We had a previous
report of a problem in this area, which I tried to fix in a quick-hack way
in commit 4da6439bd8553059766011e2a42c6e39df08717f, but Antonin Houska
pointed out that there were still some problems, and investigation turned
up other issues.  This patch largely reverts that commit in favor of a more
thoroughly thought-through solution.  The new theory is that a PHV's
ph_eval_at level cannot be higher than its original syntactic level.  If it
contains lateral references, those don't change the ph_eval_at level, but
rather they create a lateral-reference requirement for the ph_eval_at join
relation.  The code in joinpath.c needs to handle that.

Another issue is that createplan.c wasn't handling nested PlaceHolderVars
properly.

In passing, push knowledge of lateral-reference checks for join clauses
into join_clause_is_movable_to.  This is mainly so that FDWs don't need
to deal with it.

This patch doesn't fix the original join-qual-placement problem reported by
Jeremy Evans (and indeed, one of the new regression test cases shows the
wrong answer because of that).  But the PlaceHolderVar problems need to be
fixed before that issue can be addressed, so committing this separately
seems reasonable.

1 files changed, 16 insertions, 12 deletions

diff --git a/src/backend/optimizer/plan/analyzejoins.c b/src/backend/optimizer/plan/analyzejoins.c
index daab355b1d3..2271a7c35e0 100644
--- a/src/backend/optimizer/plan/analyzejoins.c
+++ b/src/backend/optimizer/plan/analyzejoins.c
@@ -202,7 +202,9 @@ join_is_removable(PlannerInfo *root, SpecialJoinInfo *sjinfo)
 	 * that will be used above the join.  We only need to fail if such a PHV
 	 * actually references some inner-rel attributes; but the correct check
 	 * for that is relatively expensive, so we first check against ph_eval_at,
-	 * which must mention the inner rel if the PHV uses any inner-rel attrs.
+	 * which must mention the inner rel if the PHV uses any inner-rel attrs as
+	 * non-lateral references.	Note that if the PHV's syntactic scope is just
+	 * the inner rel, we can't drop the rel even if the PHV is variable-free.
 	 */
 	foreach(l, root->placeholder_list)
 	{
@@ -210,9 +212,13 @@ join_is_removable(PlannerInfo *root, SpecialJoinInfo *sjinfo)
 
 		if (bms_is_subset(phinfo->ph_needed, joinrelids))
 			continue;			/* PHV is not used above the join */
+		if (bms_overlap(phinfo->ph_lateral, innerrel->relids))
+			return false;		/* it references innerrel laterally */
 		if (!bms_overlap(phinfo->ph_eval_at, innerrel->relids))
 			continue;			/* it definitely doesn't reference innerrel */
-		if (bms_overlap(pull_varnos((Node *) phinfo->ph_var),
+		if (bms_is_subset(phinfo->ph_eval_at, innerrel->relids))
+			return false;		/* there isn't any other place to eval PHV */
+		if (bms_overlap(pull_varnos((Node *) phinfo->ph_var->phexpr),
 						innerrel->relids))
 			return false;		/* it does reference innerrel */
 	}
@@ -355,7 +361,7 @@ remove_rel_from_query(PlannerInfo *root, int relid, Relids joinrelids)
 	 * Likewise remove references from LateralJoinInfo data structures.
 	 *
 	 * If we are deleting a LATERAL subquery, we can forget its
-	 * LateralJoinInfo altogether.	Otherwise, make sure the target is not
+	 * LateralJoinInfos altogether.  Otherwise, make sure the target is not
 	 * included in any lateral_lhs set.  (It probably can't be, since that
 	 * should have precluded deciding to remove it; but let's cope anyway.)
 	 */
@@ -364,29 +370,27 @@ remove_rel_from_query(PlannerInfo *root, int relid, Relids joinrelids)
 		LateralJoinInfo *ljinfo = (LateralJoinInfo *) lfirst(l);
 
 		nextl = lnext(l);
-		if (ljinfo->lateral_rhs == relid)
+		ljinfo->lateral_rhs = bms_del_member(ljinfo->lateral_rhs, relid);
+		if (bms_is_empty(ljinfo->lateral_rhs))
 			root->lateral_info_list = list_delete_ptr(root->lateral_info_list,
 													  ljinfo);
 		else
+		{
 			ljinfo->lateral_lhs = bms_del_member(ljinfo->lateral_lhs, relid);
+			Assert(!bms_is_empty(ljinfo->lateral_lhs));
+		}
 	}
 
 	/*
 	 * Likewise remove references from PlaceHolderVar data structures.
-	 *
-	 * Here we have a special case: if a PHV's eval_at set is just the target
-	 * relid, we want to leave it that way instead of reducing it to the empty
-	 * set.  An empty eval_at set would confuse later processing since it
-	 * would match every possible eval placement.
 	 */
 	foreach(l, root->placeholder_list)
 	{
 		PlaceHolderInfo *phinfo = (PlaceHolderInfo *) lfirst(l);
 
 		phinfo->ph_eval_at = bms_del_member(phinfo->ph_eval_at, relid);
-		if (bms_is_empty(phinfo->ph_eval_at))	/* oops, belay that */
-			phinfo->ph_eval_at = bms_add_member(phinfo->ph_eval_at, relid);
-
+		Assert(!bms_is_empty(phinfo->ph_eval_at));
+		Assert(!bms_is_member(relid, phinfo->ph_lateral));
 		phinfo->ph_needed = bms_del_member(phinfo->ph_needed, relid);
 	}