Re: Clause accidentally pushed down ( Possible bug in Making Vars outer-join aware)

On Tue, May 16, 2023 at 8:03 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> D'oh. Here's a patchset with these issues addressed.

I'm reviewing the v5 patches and I find that the following change in
deconstruct_distribute_oj_quals is suspicious.

if (joins_below)
{
/* Reset serial counter for this version of the quals */
root->last_rinfo_serial = save_last_rinfo_serial;

/*
* Add lower joins' relids to the qual. We should add them to
* Vars coming from the current join's LHS: we want to transform
* the second form of OJ identity 3 to the first form, in which
* Vars of relation B will appear nulled by the
* syntactically-upper OJ within the Pbc clause, but those of
* relation C will not. (In the notation used by
* optimizer/README, we're converting a qual of the form Pbc to
* Pb*c.)
*/
quals = (List *)
add_nulling_relids((Node *) quals,
sjinfo->syn_lefthand,
joins_below);

I doubt this is always right to add joins_below to all the vars
belonging to sjinfo->syn_lefthand. What if the joins in joins_below
cannot commute with each other? As a counterexample, consider the query
below which causes assertion failure in search_indexed_tlist_for_var.
The query is designed so that t1/t2 join cannot commute with t2/t3 join
but can commute with t3/t4 join.

explain (costs off)
select * from t t1
left join t t2 on true
left join t t3 on true
left join t t4 on t2.a = t3.a;
server closed the connection unexpectedly

Also, it seems that this logic may cause us to miss join quals in the
final plan. Consider the query below.

explain (costs off)
select * from t t1
left join t t2 on true
left join t t3 on t2.a = t3.a
left join t t4 on t3.a != t4.a;
QUERY PLAN
------------------------------------------------
Nested Loop Left Join
-> Seq Scan on t t1
-> Materialize
-> Nested Loop Left Join
-> Hash Left Join
Hash Cond: (t2.a = t3.a)
-> Seq Scan on t t2
-> Hash
-> Seq Scan on t t3
-> Materialize
-> Seq Scan on t t4
(11 rows)

So we've missed join qual 't3.a != t4.a' in this plan shape. For this
join qual , deconstruct_distribute_oj_quals() generated two versions,
one with empty nullingrels, the other with nullingrels {t1/t2, t2/t3}.
Both are not applicable at join level (t2/t3)/t4. I think we should
have another version with nullingrels {t2/t3} for this qual.

Thanks
Richard