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

On Fri, Jun 30, 2023 at 12:20 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> Richard Guo <guofenglinux(at)gmail(dot)com> writes:
> > On Wed, Jun 28, 2023 at 10:09 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)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.

Thanks
Richard