From: Tom Lane <tgl@sss.pgh.pa.us>
To: Richard Guo <guofenglinux@gmail.com>
cc: Daniel Gustafsson <daniel@yesql.se>, xinwen@stu.scu.edu.cn,
        pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #17700: An assert failed in prepjointree.c
In-reply-to: <556194.1669649617@sss.pgh.pa.us>
References: <17700-2b5c10d917c30687@postgresql.org>
 <CAMbWs4-VqpU4qBmO6Ktrt0Ur5OuV7OgZuDEx2GuVCajFrV6xAA@mail.gmail.com>
 <556194.1669649617@sss.pgh.pa.us>
Comments: In-reply-to Tom Lane <tgl@sss.pgh.pa.us>
	message dated "Mon, 28 Nov 2022 10:33:37 -0500"
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0"
Content-ID: <627496.1669652458.0@sss.pgh.pa.us>
Date: Mon, 28 Nov 2022 11:22:07 -0500
Message-ID: <627613.1669652527@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/627613.1669652527%40sss.pgh.pa.us>
Precedence: bulk

------- =_aaaaaaaaaa0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <627496.1669652458.1@sss.pgh.pa.us>

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


------- =_aaaaaaaaaa0
Content-Type: text/x-diff;
	name="remove-bogus-Assert-in-result-RTE-removal.patch";
	charset="us-ascii"
Content-ID: <627496.1669652458.2@sss.pgh.pa.us>
Content-Description: remove-bogus-Assert-in-result-RTE-removal.patch
Content-Transfer-Encoding: quoted-printable

diff --git a/src/backend/optimizer/prep/prepjointree.c b/src/backend/optim=
izer/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 =3D get_result_relid(root, j->rarg)) !=3D 0)
 				{
-					Assert(!find_dependent_phvs(root, varno));
 					remove_result_refs(root, varno, j->larg);
 					if (j->quals)
 						jtnode =3D (Node *)
diff --git a/src/test/regress/expected/join.out b/src/test/regress/expecte=
d/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 =3D 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
 --

------- =_aaaaaaaaaa0--