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.
