On Fri, Jun 30, 2023 at 12:20 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Richard Guo <guofenglinux@gmail.com> writes: > On Wed, Jun 28, 2023 at 10:09 PM Tom Lane <tgl@sss.pgh.pa.us> wrote: >> Those cases will go through calc_non_nestloop_required_outer >> which has >> /* neither path can require rels from the other */ >> Assert(!bms_overlap(outer_paramrels, inner_path->parent->relids)); >> Assert(!bms_overlap(inner_paramrels, outer_path->parent->relids));
> Looking at these two assertions it occurred to me that shouldn't we > check against top_parent_relids for an otherrel since paths are > parameterized by top-level parents? We do that in try_nestloop_path.
Yeah, while looking at this I was wondering why try_mergejoin_path and try_hashjoin_path don't do the same "Paths are parameterized by top-level parents, so run parameterization tests on the parent relids" dance that try_nestloop_path does. This omission is consistent with that, but it's not obvious why it'd be okay to skip it for non-nestloop joins. I guess we'd have noticed by now if it wasn't okay, but ...
I think it just makes these two assertions meaningless to skip it for non-nestloop joins if the input paths are for otherrels, because paths would never be parameterized by the member relations. So these two assertions would always be true for otherrel paths. I think this is why we have not noticed any problem by now.
However, this is not what we want. What we want is to verify that neither input path for the joinrel can require rels from the other, even for otherrel paths. So I think the current code is not right for that. We need to check against top_parent_relids for otherrel paths, and that would make these assertions meaningful.