On Tue, 2024-04-23 at 16:59 +0800, Tender Wang wrote: > [ cut ] > > I have rebased master and fixed a plan diff case.
We (me, Paul Jungwirth, and Yuki Fujii) reviewed this patch at PgConf.dev Patch Review Workshop.
Thanks for reviewing this patch.
Here are our findings.
Patch tries to allow for using materialization together with parallel subqueries. It applies cleanly on 8fea1bd5411b793697a4c9087c403887e050c4ac (current HEAD). Tests pass locally on macOS and Linux in VM under Windows. Tests are also green in cfbot (for last 2 weeks; they were red previously, probably because of need to rebase).
Please add more tests. Especially please add some negative tests; current patch checks that it is safe to apply materialization. It would be helpful to add tests checking that materialization is not applied in both checked cases: 1. when inner join path is not parallel safe 2. when matpath is not parallel safe
I added a test case that inner rel is not parallel safe. Actually, matpath will not create
if inner rel is not parallel safe. So I didn't add test case for the second scenario.
This patch tries to apply materialization only when join type is not JOIN_UNIQUE_INNER. Comment mentions this, but does not explain why. So please either add comment describing reason for that or try enabling materialization in such a case.
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.