remove_rel_from_query() must clean up PlaceHolderVar.phrels fields.

While we got away with this sloppiness before, it's not okay now
that fee7b77b9 caused build_joinrel_tlist() to make use of phrels.
Per report from Robins Tharakan.

Richard Guo (some cosmetic tweaks by me)

Discussion: https://postgr.es/m/CAMbWs4_ngw9sKxpTE8hqk=-ooVX_CQP3DarA4HzkRMz_JKpTrA@mail.gmail.com

diff --git a/src/backend/optimizer/plan/analyzejoins.c b/src/backend/optimizer/plan/analyzejoins.c
index 5bcf95cef3e0e85e31be48a73830598167f44faa..283539e53c404eb5748de0e244c98ec6f35739aa 100644 (file)
--- a/src/backend/optimizer/plan/analyzejoins.c
+++ b/src/backend/optimizer/plan/analyzejoins.c
@@ -316,12 +316,8 @@ join_is_removable(PlannerInfo *root, SpecialJoinInfo *sjinfo)
  * Remove the target relid from the planner's data structures, having
  * determined that there is no need to include it in the query.
  *
- * We are not terribly thorough here.  We must make sure that the rel is
- * no longer treated as a baserel, and that attributes of other baserels
- * are no longer marked as being needed at joins involving this rel.
- * Also, join quals involving the rel have to be removed from the joininfo
- * lists, but only if they belong to the outer join identified by ojrelid
- * and joinrelids.
+ * We are not terribly thorough here.  We only bother to update parts of
+ * the planner's data structures that will actually be consulted later.
  */
 static void
 remove_rel_from_query(PlannerInfo *root, int relid, int ojrelid,
@@ -429,11 +425,18 @@ remove_rel_from_query(PlannerInfo *root, int relid, int ojrelid,
        }
        else
        {
+           PlaceHolderVar *phv = phinfo->ph_var;
+
            phinfo->ph_eval_at = bms_del_member(phinfo->ph_eval_at, relid);
            phinfo->ph_eval_at = bms_del_member(phinfo->ph_eval_at, ojrelid);
-           Assert(!bms_is_empty(phinfo->ph_eval_at));
+           Assert(!bms_is_empty(phinfo->ph_eval_at));  /* checked previously */
            phinfo->ph_needed = bms_del_member(phinfo->ph_needed, relid);
            phinfo->ph_needed = bms_del_member(phinfo->ph_needed, ojrelid);
+           /* ph_needed might or might not become empty */
+           phv->phrels = bms_del_member(phv->phrels, relid);
+           phv->phrels = bms_del_member(phv->phrels, ojrelid);
+           Assert(!bms_is_empty(phv->phrels));
+           Assert(phv->phnullingrels == NULL); /* no need to adjust */
        }
    }
 

diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out
index 18b5e8f750bbab88c7ad369c95a264ca4d170e80..75f03fcf30c52eabbce416a0a8c5906e0aba002c 100644 (file)
--- a/src/test/regress/expected/join.out
+++ b/src/test/regress/expected/join.out
@@ -5052,6 +5052,22 @@ from int4_tbl as t1
          One-Time Filter: false
 (5 rows)
 
+-- per further discussion of bug #17781
+explain (costs off)
+select ss1.x
+from (select f1/2 as x from int4_tbl i4 left join a on a.id = i4.f1) ss1
+     right join int8_tbl i8 on true
+where current_user is not null;  -- this is to add a Result node
+                  QUERY PLAN                   
+-----------------------------------------------
+ Result
+   One-Time Filter: (CURRENT_USER IS NOT NULL)
+   ->  Nested Loop Left Join
+         ->  Seq Scan on int8_tbl i8
+         ->  Materialize
+               ->  Seq Scan on int4_tbl i4
+(6 rows)
+
 -- check that join removal works for a left join when joining a subquery
 -- that is guaranteed to be unique by its GROUP BY clause
 explain (costs off)

diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql
index 7806c910b4d6b8ac572dec25affb95263e3feb71..2e764cdd51d2ce8c332305d19fd4fcaee1d5812b 100644 (file)
--- a/src/test/regress/sql/join.sql
+++ b/src/test/regress/sql/join.sql
@@ -1813,6 +1813,13 @@ from int4_tbl as t1
                on t5.q1 = t7.q2)
     on false;
 
+-- per further discussion of bug #17781
+explain (costs off)
+select ss1.x
+from (select f1/2 as x from int4_tbl i4 left join a on a.id = i4.f1) ss1
+     right join int8_tbl i8 on true
+where current_user is not null;  -- this is to add a Result node
+
 -- check that join removal works for a left join when joining a subquery
 -- that is guaranteed to be unique by its GROUP BY clause
 explain (costs off)