Re: Should consider materializing the cheapest inner path in consider_parallel_nestloop()

From: Tender Wang <tndrwang(at)gmail(dot)com>
To: Richard Guo <guofenglinux(at)gmail(dot)com>
Cc: Tomasz Rybak <tomasz(dot)rybak(at)post(dot)pl>, pgsql-hackers(at)lists(dot)postgresql(dot)org, robertmhaas(at)gmail(dot)com, David Rowley <dgrowleyml(at)gmail(dot)com>, "Fujii(dot)Yuki(at)df(dot)MitsubishiElectric(dot)co(dot)jp" <Fujii(dot)Yuki(at)df(dot)mitsubishielectric(dot)co(dot)jp>
Subject: Re: Should consider materializing the cheapest inner path in consider_parallel_nestloop()
Date: 2024-06-19 02:55:25
Message-ID: CAHewXNkTh0RS7FMmVjMmPbMjG1=mUQ6uchxG3BuKsVH1=tNjag@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Richard Guo <guofenglinux(at)gmail(dot)com> 于2024年6月18日周二 17:24写道:

> On Tue, Jun 4, 2024 at 6:51 PM Tender Wang <tndrwang(at)gmail(dot)com> wrote:
> > Yeah, Richard commented the v1 patch about JOIN_UNIQUE_INNER in [1]
> >
> > * I think we should not consider materializing the cheapest inner path
> > if we're doing JOIN_UNIQUE_INNER, because in this case we have to
> > unique-ify the inner path.
> >
> > We don't consider material inner path if jointype is JOIN_UNIQUE_INNER
> in match_unsorted_order().
> > So here is as same logic as match_unsorted_order(). I added comments to
> explain why.
>
> I looked through the v4 patch and found an issue. For the plan diff:
>
> + -> Nested Loop
> + -> Parallel Seq Scan on prt1_p1 t1_1
> + -> Materialize
> + -> Sample Scan on prt1_p1 t2_1
> + Sampling: system (t1_1.a) REPEATABLE (t1_1.b)
> + Filter: (t1_1.a = a)
>
> This does not seem correct to me. The inner path is parameterized by
> the outer rel, in which case it does not make sense to add a Materialize
> node on top of it.
>

Yeah, you're right.

>
> I updated the patch to include a check in consider_parallel_nestloop
> ensuring that inner_cheapest_total is not parameterized by outerrel
> before materializing it. I also tweaked the comments, test cases and
> commit message.
>

Thanks for the work. Now it looks better.
I have changed the status from "need review" to "ready for commiters" on
the commitfest.

--
Tender Wang

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2024-06-19 03:07:27 Re: IPC::Run accepts bug reports
Previous Message Greg Sabino Mullane 2024-06-19 01:42:32 Re: Better error message when --single is not the first arg to postgres executable