Re: Fix bogus Asserts in calc_non_nestloop_required_outer

On Mon, Jan 8, 2024 at 5:39 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> I think we're talking at cross-purposes. What I was wondering about
> (at least further down in the thread) was whether we shouldn't be
> checking *both* the "real" and the "parent" relids to make sure they
> don't overlap the parameterization sets. But it's probably overkill.
> After reading the code again I agree that the parent relids are more
> useful to check.

Hmm, OK.

But we could also find some way to assert that the parameterization
sets contain only top-most rels. That would be strictly stronger than
asserting that they don't contain any non-topmost rels that exist on
the other side.

Trying to say that more clearly, I think the rules here are:

1. outer_paramrels can't contain ANY non-top rels
2. outer_paramrels can't contain top rels that appear on the other
side of the join

What you're proposing to assert here is (2) plus a weaker version of
(1), namely "no non-top rels that appear on the other side of the
join". That's not wrong, but I don't see why it's better than checking
only (2) or checking exactly (1) and (2).

> However, I still don't like Richard's patch too much as-is, because
> the Asserts are difficult to read/understand and even more difficult
> to compare to the other code path. I think we want to keep the
> nestloop and not-nestloop paths as visually similar as possible,
> so I propose we do it more like the attached (where I also borrowed
> some of your wording for the comments).

I don't understand what this is parallel to;
calc_nestloop_required_outer does no similar dance.

If we don't set top_relids when it's the same as relids, then I agree
that doing this dance is warranted, but otherwise it's just clutter.

--
Robert Haas
EDB: http://www.enterprisedb.com