Remove bogus Assert and dead code in remove_useless_results_recurse().

The JOIN_SEMI case Assert'ed that there are no PlaceHolderVars that
need to be evaluated at the semijoin's RHS, which is wrong because
there could be some in the semijoin's qual condition.  However, there
could not be any references further up than that, and within the qual
there is not any way that such a PHV could have gone to null yet, so
we don't really need the PHV and there is no need to avoid making the
RHS-removal optimization.  The upshot is that there's no actual bug
in production code, and we ought to just remove this misguided Assert.

While we're here, also drop the JOIN_RIGHT case, which is dead code
because reduce_outer_joins() already got rid of JOIN_RIGHT.

Per bug #17700 from Xin Wen.  Uselessness of the JOIN_RIGHT case
pointed out by Richard Guo.  Back-patch to v12 where this code
was added.

Discussion: https://postgr.es/m/17700-2b5c10d917c30687@postgresql.org

diff --git a/src/backend/optimizer/prep/prepjointree.c b/src/backend/optimizer/prep/prepjointree.c
index 92a90e40f04dec3849b036af9a8733fa9ae33f05..1c20ef587cee7d271c9aa67994190d4011d4700f 100644 (file)
--- a/src/backend/optimizer/prep/prepjointree.c
+++ b/src/backend/optimizer/prep/prepjointree.c
@@ -3007,16 +3007,6 @@ remove_useless_results_recurse(PlannerInfo *root, Node *jtnode)
                                        jtnode = j->larg;
                                }
                                break;
-                       case JOIN_RIGHT:
-                               /* Mirror-image of the JOIN_LEFT case */
-                               if ((varno = get_result_relid(root, j->larg)) != 0 &&
-                                       (j->quals == NULL ||
-                                        !find_dependent_phvs(root, varno)))
-                               {
-                                       remove_result_refs(root, varno, j->rarg);
-                                       jtnode = j->rarg;
-                               }
-                               break;
                        case JOIN_SEMI:
 
                                /*
@@ -3025,14 +3015,17 @@ remove_useless_results_recurse(PlannerInfo *root, Node *jtnode)
                                 * LHS, since we should either return the LHS row or not.  For
                                 * simplicity we inject the filter qual into a new FromExpr.
                                 *
-                                * Unlike the LEFT/RIGHT cases, we just Assert that there are
-                                * no PHVs that need to be evaluated at the semijoin's RHS,
-                                * since the rest of the query couldn't reference any outputs
-                                * of the semijoin's RHS.
+                                * There is a fine point about PHVs that are supposed to be
+                                * evaluated at the RHS.  Such PHVs could only appear in the
+                                * semijoin's qual, since the rest of the query cannot
+                                * reference any outputs of the semijoin's RHS.  Therefore,
+                                * they can't actually go to null before being examined, and
+                                * it'd be OK to just remove the PHV wrapping.  We don't have
+                                * infrastructure for that, but remove_result_refs() will
+                                * relabel them as to be evaluated at the LHS, which is fine.
                                 */
                                if ((varno = get_result_relid(root, j->rarg)) != 0)
                                {
-                                       Assert(!find_dependent_phvs(root, varno));
                                        remove_result_refs(root, varno, j->larg);
                                        if (j->quals)
                                                jtnode = (Node *)
@@ -3046,6 +3039,7 @@ remove_useless_results_recurse(PlannerInfo *root, Node *jtnode)
                                /* We have no special smarts for these cases */
                                break;
                        default:
+                               /* Note: JOIN_RIGHT should be gone at this point */
                                elog(ERROR, "unrecognized join type: %d",
                                         (int) j->jointype);
                                break;

diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out
index 610e70d1cfdf8ed993101782a3790765600e10b9..580700fb9fbf311c45a407a156f3a67c7967a86b 100644 (file)
--- a/src/test/regress/expected/join.out
+++ b/src/test/regress/expected/join.out
@@ -3374,6 +3374,26 @@ where b;
  0 | t | t
 (2 rows)
 
+-- Test PHV in a semijoin qual, which confused useless-RTE removal (bug #17700)
+explain (verbose, costs off)
+with ctetable as not materialized ( select 1 as f1 )
+select * from ctetable c1
+where f1 in ( select c3.f1 from ctetable c2 full join ctetable c3 on true );
+         QUERY PLAN         
+----------------------------
+ Result
+   Output: 1
+   One-Time Filter: (1 = 1)
+(3 rows)
+
+with ctetable as not materialized ( select 1 as f1 )
+select * from ctetable c1
+where f1 in ( select c3.f1 from ctetable c2 full join ctetable c3 on true );
+ f1 
+----
+  1
+(1 row)
+
 --
 -- test extraction of restriction OR clauses from join OR clause
 -- (we used to only do this for indexable clauses)

diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql
index 2fff1a882e13d5a698f4483b6be332d2792109a9..4dd351b6a4579d6aca32004f4392dbdd4f5690a8 100644 (file)
--- a/src/test/regress/sql/join.sql
+++ b/src/test/regress/sql/join.sql
@@ -1080,6 +1080,16 @@ select * from
            select a as b) as t3
 where b;
 
+-- Test PHV in a semijoin qual, which confused useless-RTE removal (bug #17700)
+explain (verbose, costs off)
+with ctetable as not materialized ( select 1 as f1 )
+select * from ctetable c1
+where f1 in ( select c3.f1 from ctetable c2 full join ctetable c3 on true );
+
+with ctetable as not materialized ( select 1 as f1 )
+select * from ctetable c1
+where f1 in ( select c3.f1 from ctetable c2 full join ctetable c3 on true );
+
 --
 -- test extraction of restriction OR clauses from join OR clause
 -- (we used to only do this for indexable clauses)