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

On Tue, Jun 4, 2024 at 6:51 PM Tender Wang <tndrwang@gmail.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.

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
Richard