Fix bogus outer-join commutation with lateral refs in PlaceHolderVars

make_outerjoininfo() decides whether outer join identity 3 applies
between an outer join and a lower one in its LHS by checking that the
join clause is strict for the lower join's RHS.  It relied on the
clause then necessarily referencing that RHS.  That is not so when the
strictness comes through a PlaceHolderVar: find_nonnullable_rels()
looks inside the PHV's expression and reports rels it references
laterally, while pull_varnos() reports only where the PHV is
evaluated.

In that case we marked the two joins as commutable although the clause
does not use the lower join's RHS at all.  Joinrels for the same set
of base rels could then be built both with and without the upper
join's relid, leading to an assertion failure in
standard_join_search().  The bogus commutation also caused an extra
clone of the upper join's clause to be generated, which could be
applied at a higher join in addition to the clone already applied
below it.  In non-assert builds, this could lead to planner errors
such as "variable not found in subplan target lists".

To fix, ignore rels that the clause is strict for but does not
reference, consistent with clause_relids treating a PHV as computed at
its ph_eval_at.

Back-patch to v16, where this commutation logic was introduced.

Author: Richard Guo <guofenglinux@gmail.com>
Reviewed-by: Tender Wang <tndrwang@gmail.com>
Discussion: https://postgr.es/m/CAMbWs49xvh4-uPv3rxWkk_rtMT1ic+psHvEAxaFEXu2Rf-32Tw@mail.gmail.com
Backpatch-through: 16

src/backend/optimizer/plan/initsplan.c

@@ -1432,6 +1432,13 @@ make_outerjoininfo(PlannerInfo *root,
 	 */
 	strict_relids = find_nonnullable_rels((Node *) clause);
 
+	/*
+	 * find_nonnullable_rels looks inside PlaceHolderVars, so it can report
+	 * rels that the clause references only laterally.  Ignore those, since
+	 * like clause_relids we treat a PHV as computed at its ph_eval_at.
+	 */
+	strict_relids = bms_int_members(strict_relids, clause_relids);
+
 	/* Remember whether the clause is strict for any LHS relations */
 	sjinfo->lhs_strict = bms_overlap(strict_relids, left_rels);
 

src/test/regress/expected/join.out

@@ -5797,6 +5797,36 @@ where s2.q2 > t1.f1;
 (1 row)
 
 reset from_collapse_limit;
+--
+-- check that an outer join is not taken to commute with a lower outer join
+-- when its clause references the lower join's RHS only laterally within a
+-- PlaceHolderVar
+--
+explain (costs off)
+select 1 from int4_tbl t1
+  left join int4_tbl t2 on true
+  left join lateral (select t2.f1, 1 from int4_tbl t3) s(x, y) on true
+  left join int4_tbl t4 on t4.f1 = s.x
+  join int4_tbl t5 on true
+where s.y is not null;
+                      QUERY PLAN                       
+-------------------------------------------------------
+ Nested Loop
+   ->  Hash Left Join
+         Hash Cond: ((t2.f1) = t4.f1)
+         ->  Nested Loop
+               ->  Nested Loop Left Join
+                     ->  Seq Scan on int4_tbl t1
+                     ->  Materialize
+                           ->  Seq Scan on int4_tbl t2
+               ->  Seq Scan on int4_tbl t3
+                     Filter: (1 IS NOT NULL)
+         ->  Hash
+               ->  Seq Scan on int4_tbl t4
+   ->  Materialize
+         ->  Seq Scan on int4_tbl t5
+(14 rows)
+
 --
 -- test successful handling of full join underneath left join (bug #14105)
 --

src/test/regress/sql/join.sql

@@ -2044,6 +2044,20 @@ where s2.q2 > t1.f1;
 
 reset from_collapse_limit;
 
+--
+-- check that an outer join is not taken to commute with a lower outer join
+-- when its clause references the lower join's RHS only laterally within a
+-- PlaceHolderVar
+--
+
+explain (costs off)
+select 1 from int4_tbl t1
+  left join int4_tbl t2 on true
+  left join lateral (select t2.f1, 1 from int4_tbl t3) s(x, y) on true
+  left join int4_tbl t4 on t4.f1 = s.x
+  join int4_tbl t5 on true
+where s.y is not null;
+
 --
 -- test successful handling of full join underneath left join (bug #14105)
 --