Richard Guo <guofenglinux@gmail.com> writes:
> After some more thought, I think PHVs should not impose any constraints
> on removing self joins. If the removed rel is contained in the relids
> that a PHV is evaluated/needed at or laterally references, it can just
> be replaced with the rel that is kept.
I experimented with trying to break this patch using this test case:
create table t (id int primary key, f1 text, f2 text);
create table i (f0 int);
explain verbose
select * from i left join (
(select *, 1 as x from t tss1) t1 join
(select *, 2 as y from t tss2) t2
on t1.id = t2.id
) on i.f0 = t1.id;
It seems to successfully remove the self-join between tss1 and tss2,
but there is something wrong. If you examine the PlannerInfo just
after remove_useless_self_joins, you will notice that the two
PlaceHolderVars (for x and y) in the placeholder_list both have
:phrels (b 7)
:phnullingrels (b)
where beforehand x had
:phrels (b 6)
:phnullingrels (b 5)
and y had
:phrels (b 7)
:phnullingrels (b 5)
It's correct to move both phrels locations to rel 7 (the surviving
self-joined rel) but what became of the reference to nullingrel 5?
That seems clearly wrong, since we have not removed the left join.
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)".
It's not apparent to me why the cross-checks we have in setrefs.c
and elsewhere don't detect a problem with this. But it seems
clear that remove_useless_self_joins is still several bricks
shy of a load in terms of fully updating the data structure for a
join removal. Probably with some more work to add complication
to this test case, we could demonstrate an observable failure.
BTW, why is it that it seems to prefer to remove the first of
the two self-joined rels, rather than the second? That seems
jarringly bizarre.
regards, tom lane
