Don't remove an RTE_RESULT needed by a nested PlaceHolderVar

When remove_useless_result_rtes() removes an RTE_RESULT, any
PlaceHolderVars that were to be evaluated there must be evaluated
somewhere else.  If the RTE_RESULT is the nullable side of an outer
join, there is no other place that would preserve the PHVs' nulling
behavior, so find_dependent_phvs() refuses the removal if any such PHV
exists anywhere in the query.  If the RTE_RESULT is a child of an
inner join, its PHVs can instead be evaluated at the join of all its
sibling rels, unless a sibling needs them before that join is formed.
find_dependent_phvs_in_jointree() checks for that by looking for such
PHVs in the join quals and LATERAL RTEs of the siblings.

That misses a PHV that is evaluated at some but not all of the sibling
rels and contains one of the RTE_RESULT's PHVs.  Such a PHV can appear
anywhere in the query, such as in the targetlist.  This can happen
when pulled-up expressions are wrapped in PHVs because of outer joins
or grouping sets.  Once the RTE_RESULT is removed, the outer PHV needs
a value that is only available at a join above its own evaluation
level.  The outer PHV's rel thus becomes laterally dependent on the
other sibling rels.  If one of those rels laterally references it in
turn, we end up with a rel that is laterally dependent on itself,
causing an assertion failure in create_lateral_join_info().

To fix, also search the whole query for PHVs that are evaluated at
some but not all of the sibling rels, and refuse to remove the
RTE_RESULT if any of them contains one of its PHVs.  An outer PHV that
is evaluated at all of the sibling rels is not a problem, since the
moved PHV is available there, so we don't block removal in that common
case.

Back-patch to all supported branches.

Author: Richard Guo <guofenglinux@gmail.com>
Reviewed-by: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Discussion: https://postgr.es/m/CAMbWs4_hLOsW8_SePp_3EmYTLX8Oc4zEZEvypyPRszpoMhDdGQ@mail.gmail.com
Backpatch-through: 14

src/backend/optimizer/prep/prepjointree.c

@@ -3459,14 +3459,17 @@ remove_result_refs(PlannerInfo *root, int varno, Node *newjtloc)
  *
  * find_dependent_phvs should be used when we want to see if there are
  * any such PHVs anywhere in the Query.  Another use-case is to see if
- * a subtree of the join tree contains such PHVs; but for that, we have
+ * a subtree of the join tree needs such PHVs; but for that, we have
  * to look not only at the join tree nodes themselves but at the
- * referenced RTEs.  For that, use find_dependent_phvs_in_jointree.
+ * referenced RTEs, and also inside any PHVs elsewhere in the Query that
+ * are evaluated within the subtree.  For that, use
+ * find_dependent_phvs_in_jointree.
  */
 
 typedef struct
 {
 	Relids		relids;
+	Relids		siblingrels;	/* base RT indexes of target's sibling rels */
 	int			sublevels_up;
 } find_dependent_phvs_context;
 
@@ -3507,6 +3510,50 @@ find_dependent_phvs_walker(Node *node,
 								  (void *) context);
 }
 
+/*
+ * Look for PlaceHolderVars that are to be evaluated at some but not all of
+ * context->siblingrels and contain a dependent PHV in their expressions.
+ */
+static bool
+find_nested_dependent_phvs_walker(Node *node,
+								  find_dependent_phvs_context *context)
+{
+	if (node == NULL)
+		return false;
+	if (IsA(node, PlaceHolderVar))
+	{
+		PlaceHolderVar *phv = (PlaceHolderVar *) node;
+
+		if (phv->phlevelsup == context->sublevels_up)
+		{
+			bool		match;
+
+			match = (!bms_is_subset(context->relids, phv->phrels) &&
+					 bms_overlap(phv->phrels, context->siblingrels) &&
+					 !bms_is_subset(context->siblingrels, phv->phrels));
+			if (match &&
+				find_dependent_phvs_walker((Node *) phv->phexpr, context))
+				return true;
+		}
+		/* fall through to examine children */
+	}
+	if (IsA(node, Query))
+	{
+		/* Recurse into subselects */
+		bool		result;
+
+		context->sublevels_up++;
+		result = query_tree_walker((Query *) node,
+								   find_nested_dependent_phvs_walker,
+								   context, 0);
+		context->sublevels_up--;
+		return result;
+	}
+
+	return expression_tree_walker(node, find_nested_dependent_phvs_walker,
+								  context);
+}
+
 static bool
 find_dependent_phvs(PlannerInfo *root, int varno)
 {
@@ -3517,6 +3564,7 @@ find_dependent_phvs(PlannerInfo *root, int varno)
 		return false;
 
 	context.relids = bms_make_singleton(varno);
+	context.siblingrels = NULL;
 	context.sublevels_up = 0;
 
 	return query_tree_walker(root->parse,
@@ -3537,6 +3585,7 @@ find_dependent_phvs_in_jointree(PlannerInfo *root, Node *node, int varno)
 		return false;
 
 	context.relids = bms_make_singleton(varno);
+	context.siblingrels = NULL;
 	context.sublevels_up = 0;
 
 	/*
@@ -3566,6 +3615,19 @@ find_dependent_phvs_in_jointree(PlannerInfo *root, Node *node, int varno)
 			return true;
 	}
 
+	/*
+	 * Lastly, check for PHVs that are evaluated at some but not all of the
+	 * sibling rels and contain a dependent PHV.  Removing the RTE_RESULT
+	 * would move the dependent PHV up to the join of all the sibling rels, so
+	 * it would not yet be available where the outer PHV is evaluated.  The
+	 * outer PHV can appear anywhere in the query (e.g., in the targetlist),
+	 * so search the whole query.
+	 */
+	context.siblingrels = bms_del_member(subrelids, varno);
+	if (query_tree_walker(root->parse, find_nested_dependent_phvs_walker,
+						  &context, 0))
+		return true;
+
 	return false;
 }
 

src/test/regress/expected/join.out

@@ -3420,6 +3420,89 @@ select * from
  1 | 2 | 2
 (1 row)
 
+-- Nor an RTE_RESULT whose PHV is nested in a PHV evaluated at some but not
+-- all of the sibling rels
+explain (verbose, costs off)
+select ss.z
+from (select 1) s1 left join
+  ((select 1) s2 left join (select 1 as x) v on true
+   cross join (lateral (select coalesce(v.x, 0) as z, f1 from int4_tbl) ss
+               cross join lateral (select ss.f1 offset 0) g))
+  on true;
+                        QUERY PLAN                         
+-----------------------------------------------------------
+ Nested Loop Left Join
+   Output: (COALESCE((1), 0))
+   ->  Result
+   ->  Nested Loop
+         Output: (COALESCE((1), 0))
+         ->  Nested Loop
+               Output: int4_tbl.f1, (COALESCE((1), 0))
+               ->  Result
+                     Output: 1
+               ->  Seq Scan on public.int4_tbl
+                     Output: int4_tbl.f1, COALESCE((1), 0)
+         ->  Result
+               Output: NULL::integer
+(13 rows)
+
+select ss.z
+from (select 1) s1 left join
+  ((select 1) s2 left join (select 1 as x) v on true
+   cross join (lateral (select coalesce(v.x, 0) as z, f1 from int4_tbl) ss
+               cross join lateral (select ss.f1 offset 0) g))
+  on true;
+ z 
+---
+ 1
+ 1
+ 1
+ 1
+ 1
+(5 rows)
+
+-- Same, with the PHVs coming from grouping sets
+explain (costs off)
+select s1.x
+from (values (1)) as v(x),
+     lateral (select v.x + 1, 2) as s1(x, y)
+     cross join generate_series(s1.y, s1.y) as g
+group by grouping sets (1, 1);
+                   QUERY PLAN                   
+------------------------------------------------
+ HashAggregate
+   Hash Key: (((1) + 1))
+   Hash Key: (((1) + 1))
+   ->  Nested Loop
+         ->  Nested Loop
+               ->  Result
+               ->  Result
+         ->  Function Scan on generate_series g
+(8 rows)
+
+select s1.x
+from (values (1)) as v(x),
+     lateral (select v.x + 1, 2) as s1(x, y)
+     cross join generate_series(s1.y, s1.y) as g
+group by grouping sets (1, 1);
+ x 
+---
+ 2
+ 2
+(2 rows)
+
+-- But we can remove it if the outer PHV covers all of the sibling rels
+explain (costs off)
+select s.y from (values (1)) v(x), lateral (select v.x + f1 as y from int4_tbl) s
+group by grouping sets (1, ());
+          QUERY PLAN           
+-------------------------------
+ MixedAggregate
+   Hash Key: (1 + int4_tbl.f1)
+   Group Key: ()
+   ->  Seq Scan on int4_tbl
+(4 rows)
+
 -- Test proper handling of appendrel PHVs during useless-RTE removal
 explain (costs off)
 select * from

src/test/regress/sql/join.sql

@@ -1091,6 +1091,40 @@ select * from
   (select 1 as x) ss1 left join (select 2 as y) ss2 on (true),
   lateral (select ss2.y as z limit 1) ss3;
 
+-- Nor an RTE_RESULT whose PHV is nested in a PHV evaluated at some but not
+-- all of the sibling rels
+explain (verbose, costs off)
+select ss.z
+from (select 1) s1 left join
+  ((select 1) s2 left join (select 1 as x) v on true
+   cross join (lateral (select coalesce(v.x, 0) as z, f1 from int4_tbl) ss
+               cross join lateral (select ss.f1 offset 0) g))
+  on true;
+select ss.z
+from (select 1) s1 left join
+  ((select 1) s2 left join (select 1 as x) v on true
+   cross join (lateral (select coalesce(v.x, 0) as z, f1 from int4_tbl) ss
+               cross join lateral (select ss.f1 offset 0) g))
+  on true;
+
+-- Same, with the PHVs coming from grouping sets
+explain (costs off)
+select s1.x
+from (values (1)) as v(x),
+     lateral (select v.x + 1, 2) as s1(x, y)
+     cross join generate_series(s1.y, s1.y) as g
+group by grouping sets (1, 1);
+select s1.x
+from (values (1)) as v(x),
+     lateral (select v.x + 1, 2) as s1(x, y)
+     cross join generate_series(s1.y, s1.y) as g
+group by grouping sets (1, 1);
+
+-- But we can remove it if the outer PHV covers all of the sibling rels
+explain (costs off)
+select s.y from (values (1)) v(x), lateral (select v.x + f1 as y from int4_tbl) s
+group by grouping sets (1, ());
+
 -- Test proper handling of appendrel PHVs during useless-RTE removal
 explain (costs off)
 select * from