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

The following bug has been logged on the website:

Bug reference:      17700
Logged by:          xin wen
Email address:      xinwen@stu.scu.edu.cn
PostgreSQL version: 15.1
Operating system:   Ubuntu 20.04
Description:

When executing the following query:

WITH table3 ( column34 ) AS NOT MATERIALIZED ( SELECT 1 ) SELECT 1 FROM (
SELECT 1 FROM table3 WHERE column34 = ANY ( SELECT alias4 . column34 FROM
table3 FULL JOIN table3 AS alias4 ON TRUE )) AS alias5 ;

I get a failed assertion with the following stacktrace:

Core was generated by `postgres: postgres postgres [local] SELECT
'.
Program terminated with signal SIGABRT, Aborted.
#0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
#1  0x00007ff7f1a36859 in __GI_abort () at abort.c:79
#2  0x0000563be15a2a88 in ExceptionalCondition
(conditionName=conditionName@entry=0x563be1713818
"!find_dependent_phvs(root, varno)",
errorType=errorType@entry=0x563be1600498 "FailedAssertion", 
    fileName=fileName@entry=0x563be1713580
"/home/postgres/postgresql-15.1/bld_debugging/../src/backend/optimizer/prep/prepjointree.c",
lineNumber=lineNumber@entry=3301)
    at
/home/postgres/postgresql-15.1/bld_debugging/../src/backend/utils/error/assert.c:69
#3  0x0000563be13b15cb in remove_useless_results_recurse
(root=0x563be32a6cb8, jtnode=0x563be32a6158) at
/home/postgres/postgresql-15.1/bld_debugging/../src/backend/optimizer/prep/prepjointree.c:3301
#4  0x0000563be13b1304 in remove_useless_results_recurse
(root=0x563be32a6cb8, jtnode=0x563be32a6bd0) at
/home/postgres/postgresql-15.1/bld_debugging/../src/backend/optimizer/prep/prepjointree.c:3155
#5  0x0000563be13b15f6 in remove_useless_result_rtes
(root=root@entry=0x563be32a6cb8) at
/home/postgres/postgresql-15.1/bld_debugging/../src/backend/optimizer/prep/prepjointree.c:3095
#6  0x0000563be13a4fca in subquery_planner (glob=glob@entry=0x563be32a6000,
parse=parse@entry=0x563be31dee50, parent_root=parent_root@entry=0x0,
hasRecursion=hasRecursion@entry=false, 
    tuple_fraction=tuple_fraction@entry=0) at
/home/postgres/postgresql-15.1/bld_debugging/../src/backend/optimizer/plan/planner.c:1039
#7  0x0000563be13a52a3 in standard_planner (parse=0x563be31dee50,
query_string=<optimized out>, cursorOptions=2048, boundParams=<optimized
out>)
    at
/home/postgres/postgresql-15.1/bld_debugging/../src/backend/optimizer/plan/planner.c:406
#8  0x0000563be1476edc in pg_plan_query (querytree=0x563be31dee50, 
    query_string=0x563be31dd070 "WITH table3 ( column34 ) AS NOT
MATERIALIZED ( SELECT 1 ) SELECT 1 FROM ( SELECT 1 FROM table3 WHERE
column34 = ANY ( SELECT alias4 . column34 FROM table3 FULL JOIN table3 AS
alias4 ON TRUE )) AS alia"..., cursorOptions=2048, boundParams=0x0) at
/home/postgres/postgresql-15.1/bld_debugging/../src/backend/tcop/postgres.c:883
#9  0x0000563be1476fd1 in pg_plan_queries (querytrees=0x563be32a6c60, 
    query_string=query_string@entry=0x563be31dd070 "WITH table3 ( column34 )
AS NOT MATERIALIZED ( SELECT 1 ) SELECT 1 FROM ( SELECT 1 FROM table3 WHERE
column34 = ANY ( SELECT alias4 . column34 FROM table3 FULL JOIN table3 AS
alias4 ON TRUE )) AS alia"..., cursorOptions=cursorOptions@entry=2048,
boundParams=boundParams@entry=0x0)
    at
/home/postgres/postgresql-15.1/bld_debugging/../src/backend/tcop/postgres.c:975
#10 0x0000563be14773df in exec_simple_query (
    query_string=0x563be31dd070 "WITH table3 ( column34 ) AS NOT
MATERIALIZED ( SELECT 1 ) SELECT 1 FROM ( SELECT 1 FROM table3 WHERE
column34 = ANY ( SELECT alias4 . column34 FROM table3 FULL JOIN table3 AS
alias4 ON TRUE )) AS alia"...) at
/home/postgres/postgresql-15.1/bld_debugging/../src/backend/tcop/postgres.c:1169
#11 0x0000563be1478f8c in PostgresMain (dbname=<optimized out>,
username=<optimized out>) at
/home/postgres/postgresql-15.1/bld_debugging/../src/backend/tcop/postgres.c:4581
#12 0x0000563be13e5e8a in BackendRun (port=<optimized out>, port=<optimized
out>) at
/home/postgres/postgresql-15.1/bld_debugging/../src/backend/postmaster/postmaster.c:4504
#13 BackendStartup (port=<optimized out>) at
/home/postgres/postgresql-15.1/bld_debugging/../src/backend/postmaster/postmaster.c:4232
#14 ServerLoop () at
/home/postgres/postgresql-15.1/bld_debugging/../src/backend/postmaster/postmaster.c:1806
#15 0x0000563be13e6ffb in PostmasterMain (argc=<optimized out>,
argv=0x563be31d7370) at
/home/postgres/postgresql-15.1/bld_debugging/../src/backend/postmaster/postmaster.c:1478
#16 0x0000563be1111630 in main (argc=3, argv=0x563be31d7370) at
/home/postgres/postgresql-15.1/bld_debugging/../src/backend/main/main.c:202

I also find this assert failed in 12.13, 13.9 and 14.6 using the same
statement.

> On 28 Nov 2022, at 10:09, PG Bug reporting form <noreply@postgresql.org> wrote:

> When executing the following query:
>
> WITH table3 ( column34 ) AS NOT MATERIALIZED ( SELECT 1 ) SELECT 1 FROM (
> SELECT 1 FROM table3 WHERE column34 = ANY ( SELECT alias4 . column34 FROM
> table3 FULL JOIN table3 AS alias4 ON TRUE )) AS alias5 ;

The full join on the rhs of the semi join creates a PlaceHolderVar due to
table3 not being materialized. A slightly shorter repro is:

with table3 as not materialized (
  select 1 as column34
)
select 1 from table3
where column34 in (
  select alias4.column34 from table3
    full join table3 as alias4 on true
);

--
Daniel Gustafsson        https://vmware.com/

Richard Guo <guofenglinux@gmail.com> writes:
> I haven't got too much time looking into it.  But the comment near the
> assertion failure that says

>  * 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.

> I doubt this is true as a semijoin's qual can actually reference its
> RHS.  In this case the assertion failure happens because there is PHV in
> the join's qual.

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.

            regards, tom lane

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
 --

Richard Guo <guofenglinux@gmail.com> writes:
> BTW, for the test case

> +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
> );

> Actually we just need to keep 'c3' in a join's nullable side to have the
> PHV created.  So we don't have to use full join in the subquery.  A left
> join would do.

Actually, the planner reduces the full join to left join anyway;
if it did not, it wouldn't be able to reach the code in question.
I think this formulation is fine because it tests that step along
with the bug proper.

            regards, tom lane

Richard Guo <guofenglinux@gmail.com> writes:
> BTW, I wonder if we need the JOIN_RIGHT case here since JOIN_RIGHT
> should have been flipped around to become JOIN_LEFT.

Hmm, you're right, that's done in reduce_outer_joins which
precedes this step.

            regards, tom lane