From 8d3c4aa614e20375daeff0bb1b9f640b115f363e Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Mon, 22 Mar 2010 13:57:16 +0000
Subject: [PATCH] Fix an oversight in join-removal optimization: we have to
 check not only for plain Vars that are generated in the inner rel and used
 above the join, but also for PlaceHolderVars.  Per report from Oleg K.

---
 src/backend/optimizer/path/joinpath.c | 19 +++++++++--
 src/test/regress/expected/join.out    | 49 +++++++++++++++++++++++++++
 src/test/regress/sql/join.sql         | 23 +++++++++++++
 3 files changed, 89 insertions(+), 2 deletions(-)

diff --git a/src/backend/optimizer/path/joinpath.c b/src/backend/optimizer/path/joinpath.c
index 35c9353d2e2..f069f13fefb 100644
--- a/src/backend/optimizer/path/joinpath.c
+++ b/src/backend/optimizer/path/joinpath.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/optimizer/path/joinpath.c,v 1.130 2010/02/26 02:00:44 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/backend/optimizer/path/joinpath.c,v 1.131 2010/03/22 13:57:15 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -192,7 +192,9 @@ clause_sides_match_join(RestrictInfo *rinfo, RelOptInfo *outerrel,
  *
  * This is true for a left join for which the join condition cannot match
  * more than one inner-side row.  (There are other possibly interesting
- * cases, but we don't have the infrastructure to prove them.)
+ * cases, but we don't have the infrastructure to prove them.)  We also
+ * have to check that the inner side doesn't generate any variables needed
+ * above the join.
  *
  * Note: there is no need to consider the symmetrical case of duplicating the
  * right input, because add_paths_to_joinrel() will be called with each rel
@@ -245,6 +247,19 @@ join_is_removable(PlannerInfo *root,
 			return false;
 	}
 
+	/*
+	 * Similarly check that the inner rel doesn't produce any PlaceHolderVars
+	 * that will be used above the join.
+	 */
+	foreach(l, root->placeholder_list)
+	{
+		PlaceHolderInfo *phinfo = (PlaceHolderInfo *) lfirst(l);
+
+		if (bms_is_subset(phinfo->ph_eval_at, innerrel->relids) &&
+			!bms_is_subset(phinfo->ph_needed, joinrel->relids))
+			return false;
+	}
+
 	/*
 	 * Search for mergejoinable clauses that constrain the inner rel against
 	 * either the outer rel or a pseudoconstant.  If an operator is
diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out
index f2b346e5084..ad164e1c02b 100644
--- a/src/test/regress/expected/join.out
+++ b/src/test/regress/expected/join.out
@@ -2491,3 +2491,52 @@ select * from int4_tbl a full join int4_tbl b on false;
  -2147483647 |            
 (10 rows)
 
+--
+-- test join removal
+--
+create temp table parent (k int primary key, pd int);
+NOTICE:  CREATE TABLE / PRIMARY KEY will create implicit index "parent_pkey" for table "parent"
+create temp table child (k int unique, cd int);
+NOTICE:  CREATE TABLE / UNIQUE will create implicit index "child_k_key" for table "child"
+insert into parent values (1, 10), (2, 20), (3, 30);
+insert into child values (1, 100), (4, 400);
+-- this case is optimizable
+select p.* from parent p left join child c on (p.k = c.k);
+ k | pd 
+---+----
+ 1 | 10
+ 2 | 20
+ 3 | 30
+(3 rows)
+
+explain (costs off)
+  select p.* from parent p left join child c on (p.k = c.k);
+      QUERY PLAN      
+----------------------
+ Seq Scan on parent p
+(1 row)
+
+-- this case is not
+select p.*, linked from parent p
+  left join (select c.*, true as linked from child c) as ss
+  on (p.k = ss.k);
+ k | pd | linked 
+---+----+--------
+ 1 | 10 | t
+ 2 | 20 | 
+ 3 | 30 | 
+(3 rows)
+
+explain (costs off)
+  select p.*, linked from parent p
+    left join (select c.*, true as linked from child c) as ss
+    on (p.k = ss.k);
+           QUERY PLAN            
+---------------------------------
+ Hash Left Join
+   Hash Cond: (p.k = c.k)
+   ->  Seq Scan on parent p
+   ->  Hash
+         ->  Seq Scan on child c
+(5 rows)
+
diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql
index 977765551d5..b0a4ceccf0b 100644
--- a/src/test/regress/sql/join.sql
+++ b/src/test/regress/sql/join.sql
@@ -567,3 +567,26 @@ group by t1.q2 order by 1;
 --
 select * from int4_tbl a full join int4_tbl b on true;
 select * from int4_tbl a full join int4_tbl b on false;
+
+--
+-- test join removal
+--
+
+create temp table parent (k int primary key, pd int);
+create temp table child (k int unique, cd int);
+insert into parent values (1, 10), (2, 20), (3, 30);
+insert into child values (1, 100), (4, 400);
+
+-- this case is optimizable
+select p.* from parent p left join child c on (p.k = c.k);
+explain (costs off)
+  select p.* from parent p left join child c on (p.k = c.k);
+
+-- this case is not
+select p.*, linked from parent p
+  left join (select c.*, true as linked from child c) as ss
+  on (p.k = ss.k);
+explain (costs off)
+  select p.*, linked from parent p
+    left join (select c.*, true as linked from child c) as ss
+    on (p.k = ss.k);
-- 
GitLab