Horton

Thread

Commits

  1. Fix Memoize to work with partitionwise joining.

  1. MemoizePath fails to work for partitionwise join

    Richard Guo <guofenglinux@gmail.com> — 2022-12-05T10:43:15Z

    I happened to notice that currently MemoizePath cannot be generated for
partitionwise join.  I traced that and have found the problem.  One
condition we need to meet to generate MemoizePath is that inner path's
ppi_clauses should have the form "outer op inner" or "inner op outer",
as checked by clause_sides_match_join in paraminfo_get_equal_hashops.

Note that when are at this check, the inner path has not got the chance
to be re-parameterized (that is done later in try_nestloop_path), so if
it is parameterized, it is parameterized by the topmost parent of the
outer rel, not the outer rel itself.  Thus this check performed by
clause_sides_match_join could not succeed if we are joining two child
rels.

The fix is straightforward, just to use outerrel->top_parent if it is
not null and leave the reparameterization work to try_nestloop_path.  In
addition, I believe when reparameterizing MemoizePath we need to adjust
its param_exprs too.

Attach the patch for fix.

Thanks
Richard

  2. Re: MemoizePath fails to work for partitionwise join

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-12-05T17:42:57Z

    Richard Guo <guofenglinux@gmail.com> writes:
> The fix is straightforward, just to use outerrel->top_parent if it is
> not null and leave the reparameterization work to try_nestloop_path.  In
> addition, I believe when reparameterizing MemoizePath we need to adjust
> its param_exprs too.

Right you are.  I'd noticed the apparent omission in
reparameterize_path_by_child, but figured that we'd need a test case to
find any other holes before it'd be worth fixing.  Thanks for finding
a usable test case.

One small problem is that top_parent doesn't exist in the back branches,
so I had to substitute a much uglier lookup in order to make this work
there.  I'm surprised that we got away without top_parent for this long
TBH, but anyway this fix validates the wisdom of 2f17b5701.

So, pushed with some cosmetic adjustments and the modified back-branch
code.

			regards, tom lane

  3. Re: MemoizePath fails to work for partitionwise join

    Richard Guo <guofenglinux@gmail.com> — 2022-12-06T01:58:01Z

    On Tue, Dec 6, 2022 at 1:42 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:

> One small problem is that top_parent doesn't exist in the back branches,
> so I had to substitute a much uglier lookup in order to make this work
> there.  I'm surprised that we got away without top_parent for this long
> TBH, but anyway this fix validates the wisdom of 2f17b5701.
>
> So, pushed with some cosmetic adjustments and the modified back-branch
> code.


Thanks for the modifying and pushing and the back-patching.  I didn't
realize how the fix should look like if without top_parent.  Thanks to
the work in 2f17b5701, it makes life much easier.

Thanks
Richard