Re: BUG #17700: An assert failed in prepjointree.c

I wrote:
> Yeah, the possibility of a PHV in the qual refutes that argument.
> So we need to make the test honestly, as in the other arms of that
> switch.  Will fix.

Actually ... it seems like we could just drop that Assert, as per
revised argument in the comment.  This'd explain the lack of field
complaints: there's no bug in a production build.

            regards, tom lane

diff --git a/src/backend/optimizer/prep/prepjointree.c b/src/backend/optimizer/prep/prepjointree.c
index f4cdb879c2..b156b1e42e 100644
--- a/src/backend/optimizer/prep/prepjointree.c
+++ b/src/backend/optimizer/prep/prepjointree.c
@@ -3269,14 +3269,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 *)
diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out
index 21ca7d04bc..b8d43e4c14 100644
--- a/src/test/regress/expected/join.out
+++ b/src/test/regress/expected/join.out
@@ -3550,6 +3550,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 inlining of immutable functions
 --
diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql
index 50c19d822b..65aab85c35 100644
--- a/src/test/regress/sql/join.sql
+++ b/src/test/regress/sql/join.sql
@@ -1156,6 +1156,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 inlining of immutable functions
 --