Disallow removing placeholders during Self-Join Elimination.

fc069a3a6319 implements Self-Join Elimination (SJE), which can remove base
relations when appropriate.  However, regressions tests for SJE only cover
the case when placeholder variables (PHVs) are evaluated and needed only
in a single base rel.  If this baserel is removed due to SJE, its clauses,
including PHVs, will be transferred to the keeping relation.  Removing these
PHVs may trigger an error on plan creation -- thanks to the b3ff6c742f6c for
detecting that.

This commit skips removal of PHVs during SJE.  This might also happen that
we skip the removal of some PHVs that could be removed.  However, the overhead
of extra PHVs is small compared to the complexity of analysis needed to remove
them.

Reported-by: Alexander Lakhin <exclusion@gmail.com>
Author: Alena Rybakina <a.rybakina@postgrespro.ru>
Author: Andrei Lepikhov <lepihov@gmail.com>
Reviewed-by: Alexander Korotkov <aekorotkov@gmail.com>
Reviewed-by: Richard Guo <guofenglinux@gmail.com>

src/backend/optimizer/plan/analyzejoins.c

@@ -403,7 +403,12 @@ remove_rel_from_query(PlannerInfo *root, RelOptInfo *rel,
 
 	/*
 	 * Likewise remove references from PlaceHolderVar data structures,
-	 * removing any no-longer-needed placeholders entirely.
+	 * removing any no-longer-needed placeholders entirely.  We remove PHV
+	 * only for left-join removal.  With self-join elimination, PHVs already
+	 * get moved to the remaining relation, where they might still be needed.
+	 * It might also happen that we skip the removal of some PHVs that could
+	 * be removed.  However, the overhead of extra PHVs is small compared to
+	 * the complexity of analysis needed to remove them.
 	 *
 	 * Removal is a bit trickier than it might seem: we can remove PHVs that
 	 * are used at the target rel and/or in the join qual, but not those that
@@ -420,10 +425,16 @@ remove_rel_from_query(PlannerInfo *root, RelOptInfo *rel,
 		PlaceHolderInfo *phinfo = (PlaceHolderInfo *) lfirst(l);
 
 		Assert(sjinfo == NULL || !bms_is_member(relid, phinfo->ph_lateral));
-		if (bms_is_subset(phinfo->ph_needed, joinrelids) &&
+		if (sjinfo != NULL &&
+			bms_is_subset(phinfo->ph_needed, joinrelids) &&
 			bms_is_member(relid, phinfo->ph_eval_at) &&
-			(sjinfo == NULL || !bms_is_member(sjinfo->ojrelid, phinfo->ph_eval_at)))
+			!bms_is_member(sjinfo->ojrelid, phinfo->ph_eval_at))
 		{
+			/*
+			 * This code shouldn't be executed if one relation is substituted
+			 * with another: in this case, the placeholder may be employed in
+			 * a filter inside the scan node the SJE removes.
+			 */
 			root->placeholder_list = foreach_delete_current(root->placeholder_list,
 															l);
 			root->placeholder_array[phinfo->phid] = NULL;

src/test/regress/expected/join.out

@@ -7150,7 +7150,8 @@ on true;
                      ->  Seq Scan on emp1 t4
 (7 rows)
 
--- Check that SJE removes the whole PHVs correctly
+-- Try PHV, which could potentially be removed completely by SJE, but that's
+-- not implemented yet.
 explain (verbose, costs off)
 select 1 from emp1 t1 left join
     ((select 1 as x, * from emp1 t2) s1 inner join
@@ -7200,6 +7201,37 @@ on true;
          Output: t3.id, t1.id
 (7 rows)
 
+-- This is a degenerate case of PHV usage: it is evaluated and needed inside
+-- a baserel scan operation that the SJE removes.  The PHV in this test should
+-- be in the filter of parameterized Index Scan: the replace_nestloop_params()
+-- code will detect if the placeholder list doesn't have a reference to this
+-- parameter.
+--
+-- NOTE:  enable_hashjoin and enable_mergejoin must be disabled.
+CREATE TABLE tbl_phv(x int, y int PRIMARY KEY);
+CREATE INDEX tbl_phv_idx ON tbl_phv(x);
+INSERT INTO tbl_phv (x, y)
+  SELECT gs, gs FROM generate_series(1,100) AS gs;
+VACUUM ANALYZE tbl_phv;
+EXPLAIN (COSTS OFF, VERBOSE)
+SELECT 1 FROM tbl_phv t1 LEFT JOIN
+  (SELECT 1 extra, x, y FROM tbl_phv tl) t3 JOIN
+    (SELECT y FROM tbl_phv tr) t4
+  ON t4.y = t3.y
+ON true WHERE t3.extra IS NOT NULL AND t3.x = t1.x % 2;
+                       QUERY PLAN                        
+---------------------------------------------------------
+ Nested Loop
+   Output: 1
+   ->  Seq Scan on public.tbl_phv t1
+         Output: t1.x, t1.y
+   ->  Index Scan using tbl_phv_idx on public.tbl_phv tr
+         Output: tr.x, tr.y
+         Index Cond: (tr.x = (t1.x % 2))
+         Filter: (1 IS NOT NULL)
+(8 rows)
+
+DROP TABLE IF EXISTS tbl_phv;
 -- Check that SJE replaces join clauses involving the removed rel correctly
 explain (costs off)
 select * from emp1 t1

src/test/regress/sql/join.sql

@@ -2756,7 +2756,8 @@ select * from emp1 t1 left join
         on true)
 on true;
 
--- Check that SJE removes the whole PHVs correctly
+-- Try PHV, which could potentially be removed completely by SJE, but that's
+-- not implemented yet.
 explain (verbose, costs off)
 select 1 from emp1 t1 left join
     ((select 1 as x, * from emp1 t2) s1 inner join
@@ -2774,6 +2775,26 @@ select * from generate_series(1,10) t1(id) left join
     lateral (select t1.id as t1id, t2.id from emp1 t2 join emp1 t3 on t2.id = t3.id)
 on true;
 
+-- This is a degenerate case of PHV usage: it is evaluated and needed inside
+-- a baserel scan operation that the SJE removes.  The PHV in this test should
+-- be in the filter of parameterized Index Scan: the replace_nestloop_params()
+-- code will detect if the placeholder list doesn't have a reference to this
+-- parameter.
+--
+-- NOTE:  enable_hashjoin and enable_mergejoin must be disabled.
+CREATE TABLE tbl_phv(x int, y int PRIMARY KEY);
+CREATE INDEX tbl_phv_idx ON tbl_phv(x);
+INSERT INTO tbl_phv (x, y)
+  SELECT gs, gs FROM generate_series(1,100) AS gs;
+VACUUM ANALYZE tbl_phv;
+EXPLAIN (COSTS OFF, VERBOSE)
+SELECT 1 FROM tbl_phv t1 LEFT JOIN
+  (SELECT 1 extra, x, y FROM tbl_phv tl) t3 JOIN
+    (SELECT y FROM tbl_phv tr) t4
+  ON t4.y = t3.y
+ON true WHERE t3.extra IS NOT NULL AND t3.x = t1.x % 2;
+DROP TABLE IF EXISTS tbl_phv;
+
 -- Check that SJE replaces join clauses involving the removed rel correctly
 explain (costs off)
 select * from emp1 t1