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

On Tue, Nov 28, 2023 at 3:03 PM Richard Guo <guofenglinux(at)gmail(dot)com> wrote:

> On Tue, Nov 28, 2023 at 1:42 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
>> Much worse, if you look around elsewhere in the data structure,
>> particularly at the processed_tlist, you find instances of the
>> PlaceHolderVars that have not been updated and still have the
>> old values such as ":phrels (b 6)".
>
>
> Good catch! We fail to handle PlaceHolderVar case in sje_walker and
> replace_varno_walker, which is wrong. Fixed in v3 patch.
>

I noticed that this issue exists on master even without the
Don't-constrain-SJE-due-to-PHVs patch, and it can be illustrated with a
query from regression test.

-- Test that placeholders are updated correctly after join removal
explain (costs off)
select * from (values (1)) x
left join (select coalesce(y.q1, 1) from int8_tbl y
right join sj j1 inner join sj j2 on j1.a = j2.a
on true) z
on true;

In this query it successfully removes the self-join between j1 (6) and
j2 (7). But in the processed_tlist, you can still find PlaceHolderVar
with ":phrels (b 5 6 7 9)". So I think we need to handle PlaceHolderVar
case in sje_walker and replace_varno_walker, whether or not we
incorporate the Don't-constrain-SJE-due-to-PHVs patch.

Also, it seems to me that sje_walker is redundant. Currently it is used
to walk the query tree for varno replacement, a task that can be
fulfilled with replace_varno_walker.

0001 adds the handling of PlaceHolderVar case in replace_varno_walker,
and meanwhile retires sje_walker.

0002 is the original Don't-constrain-SJE-due-to-PHVs patch.

0003 is a fix of the comment for remove_self_joins_recurse, as described
upthread.

Thanks
Richard