Re: Assert !bms_overlap(joinrel->relids, required_outer)

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Richard Guo <guofenglinux(at)gmail(dot)com>
Cc: Michael Paquier <michael(at)paquier(dot)xyz>, Jaime Casanova <jcasanov(at)systemguards(dot)com(dot)ec>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Assert !bms_overlap(joinrel->relids, required_outer)
Date: 2023-06-27 22:28:58
Message-ID: 1059278.1687904938@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I wrote:
> So it looks to me like something further up should have rejected this
> path as not being usable here. Not sure what's dropping the ball.

After further digging, I've concluded that what usually stops us
from generating this bogus path for the t3/t4 join is the
"param_source_rels" heuristic in joinpath.c. It does stop it in
the simplified query

select 1 from t t2
inner join (t t3
left join (t t4 left join t t5 on t4.a = 1)
on t4.a = 1) on false
where t3.a = coalesce(t5.a,1);

However, once we add the lateral reference in s1, that heuristic
uncritically lets the path go through, and then later we have trouble.
Of course, that heuristic is only supposed to be a heuristic that
helps winnow valid paths, not a defense against invalid paths,
so it's not its fault that this goes wrong. (I think that the old
delay_upper_joins mechanism is what prevented this error before v16.)

For a real fix, I'm inclined to extend the loop that calculates
param_source_rels (in add_paths_to_joinrel) so that it also tracks
a set of incompatible relids that *must not* be present in the
parameterization of a proposed path. This would basically include
OJ relids of OJs that partially overlap the target joinrel; maybe
we should also include the min RHS of such OJs. Then we could
check that in try_nestloop_path. I've not tried to code this yet.

There's also the question of why we generated the bogus indexscan
in the first place; but it seems advisable to fix the join-level
issue before touching that, else we'll have nothing to test with.

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Thomas Munro 2023-06-27 23:03:46 Re: check_strxfrm_bug()
Previous Message Roberto Mello 2023-06-27 21:49:44 Re: PG 16 draft release notes ready