Re: Fix bogus Asserts in calc_non_nestloop_required_outer

Robert Haas <robertmhaas@gmail.com> writes:
> On Fri, Nov 3, 2023 at 2:47 AM Richard Guo <guofenglinux@gmail.com> wrote:
>> Comment is now added above the Asserts.  Thanks for taking an interest
>> in this.

> I'd like to suggest rewording this comment a little more. Here's my proposal:

> Both of the paths passed to this function are still parameterized by
> the topmost parent, because reparameterize_path_by_child() hasn't been
> called yet. So, these sanity checks use the parent relids.

> What do you think?

I don't think I believe this code change, let alone any of the
explanations for it.  The point of these Asserts is to be sure that
we don't form an alleged parameterization set that includes any rels
that are included in the new join, because that would be obviously
wrong.  They do check that as they stand.  I'm not sure what invariant
the proposed revision would be enforcing.

There might be an argument for leaving the existing Asserts in
place and *also* checking

    Assert(!bms_overlap(outer_paramrels,
                        inner_path->parent->top_parent_relids));
    Assert(!bms_overlap(inner_paramrels,
                        outer_path->parent->top_parent_relids));

but I'm not quite convinced what the point of that is.

            regards, tom lane