Richard Guo <guofenglinux@gmail.com> writes:
> On Fri, Jun 30, 2023 at 12:20 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> 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.
After studying this some more, I think that maybe it's the "run
parameterization tests on the parent relids" bit that is misguided.
I believe the way it's really working is that all paths arriving
here are parameterized by top parents, because that's the only thing
we generate to start with. A path can only become parameterized
by an otherrel when we apply reparameterize_path_by_child to it.
But the only place that happens is in try_nestloop_path itself
(or try_partial_nestloop_path), and then we immediately wrap it in
a nestloop join node, which becomes a child of an append that's
forming a partitionwise join. The partitionwise join as a
whole won't be parameterized by any child rels. So I think that
a path that's parameterized by a child rel can't exist "in the wild"
in a way that would allow it to get fed to one of the try_xxx_path
functions. This explains why the seeming oversights in the merge
and hash cases aren't causing a problem.
If this theory is correct, we could simplify try_nestloop_path a
bit. I doubt the code savings would matter, but maybe it's
worth changing for clarity's sake.
regards, tom lane
