Re: BUG #18187: Unexpected error: "variable not found in subplan target lists" triggered by JOIN

On Thu, Nov 9, 2023 at 11:54 AM Richard Guo <guofenglinux(at)gmail(dot)com> wrote:

> I've looked into this a little bit. I think it's caused by the SJE
> logic not properly removing references from PHVs.
>

Looking closer at the codes, I think we still have some loose ends
regarding how SJE handles PHVs.

1) When we check PHVs in remove_self_joins_one_group() to see if there
is any PHV that prevents us from removing the self join, we do not check
ph_lateral. This is dangerous because it leaves us with no guarantee of
what the Assert in remove_rel_from_query() asserts:

Assert(!bms_is_member(relid, phinfo->ph_lateral));

To illustrate this problem, look at the query below which would trigger
this Assert.

create table t (a int primary key, b int);

explain (costs off)
select * from t t1 join t t2 on t1.a = t2.a left join
lateral (select t1.a as t1a, * from generate_series(1,1) t3) s on true;
server closed the connection unexpectedly

2) Currently remove_self_joins_one_group() checks PHVs as below

/* there isn't any other place to eval PHV */
if (bms_is_subset(phinfo->ph_eval_at, joinrelids) ||
bms_is_subset(phinfo->ph_needed, joinrelids))
break;

I'm wondering if we can relax this restriction because it seems to me
that a PHV evaluated/needed at or below the self join should not have
problem if we remove the self join. For instance,

explain (costs off)
select * from generate_series(1,10) t1(a) left join lateral
(select t1.a as t1a, t2.a from t t2 join t t3 on t2.a = t3.a) on true;
QUERY PLAN
-------------------------------------------
Nested Loop Left Join
-> Function Scan on generate_series t1
-> Hash Join
Hash Cond: (t2.a = t3.a)
-> Seq Scan on t t2
-> Hash
-> Seq Scan on t t3
(7 rows)

I think the t2/t3 join can actually be removed.

And now I have a vague feeling that PHVs should not impose any
constraints on removing self joins. But I'm not sure.

It would be great if we could have Tom's perspective on this. So I've
added Tom in the cc list.

FWIW, attached is a band-aid fix for the Assert failure issue in 1), in
case we want to fix it first before we discuss this topic further.

Thanks
Richard