Richard Guo <guofengli...@gmail.com> 于2023年9月5日周二 18:51写道：
>
> On Tue, Sep 5, 2023 at 4:52 PM tender wang <tndrw...@gmail.com> wrote:
>
>> I recently run benchmark[1] on master, but I found performance problem
>> as below:
>> ...
>>
>> I debug the code and find consider_parallel_nestloop() doesn't consider
>> materialized form of the cheapest inner path.
>>
>
> Yeah, this seems an omission in commit 45be99f8. I reviewed the patch
> and here are some comments.
>
> * 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.
>
That's right. The V2 patch has been fixed.
> * I think we can check if it'd be parallel safe before creating the
> material path, thus avoid the creation in unsafe cases.
>
Agreed.
> * I don't think the test case you added works for the code changes.
> Maybe a plan likes below is better:
>
Agreed.
explain (costs off)
> select * from tenk1, tenk2 where tenk1.two = tenk2.two;
> QUERY PLAN
> ----------------------------------------------
> Gather
> Workers Planned: 4
> -> Nested Loop
> Join Filter: (tenk1.two = tenk2.two)
> -> Parallel Seq Scan on tenk1
> -> Materialize
> -> Seq Scan on tenk2
> (7 rows)
>
> Thanks
> Richard
>
From 096c645d7c8d06df3a4e6a8aa7fc4edd1c0a3755 Mon Sep 17 00:00:00 2001
From: "tender.wang" <tender.w...@openpie.com>
Date: Thu, 7 Sep 2023 17:55:04 +0800
Subject: [PATCH v2] Parallel seq scan should consider materila inner path in
nestloop case.
---
src/backend/optimizer/path/joinpath.c | 19 +++++++++++++++
src/test/regress/expected/select_parallel.out | 24 +++++++++++++++++++
src/test/regress/sql/select_parallel.sql | 9 +++++++
3 files changed, 52 insertions(+)
diff --git a/src/backend/optimizer/path/joinpath.c
b/src/backend/optimizer/path/joinpath.c
index 821d282497..87111ad643 100644
--- a/src/backend/optimizer/path/joinpath.c
+++ b/src/backend/optimizer/path/joinpath.c
@@ -2004,10 +2004,25 @@ consider_parallel_nestloop(PlannerInfo *root,
{
JoinType save_jointype = jointype;
ListCell *lc1;
+ Path *matpath = NULL;
+ Path *inner_cheapest_total = innerrel->cheapest_total_path;
if (jointype == JOIN_UNIQUE_INNER)
jointype = JOIN_INNER;
+ /*
+ * Consider materializing the cheapest inner path, unless we're
+ * doing JOIN_UNIQUE_INNER or enable_material is off or the subpath
+ * is parallel unsafe or the path in question materializes its output
anyway.
+ */
+ if (save_jointype != JOIN_UNIQUE_INNER &&
+ enable_material &&
+ inner_cheapest_total != NULL &&
+ inner_cheapest_total->parallel_safe &&
+ !ExecMaterializesOutput(inner_cheapest_total->pathtype))
+ matpath = (Path *)
+ create_material_path(innerrel, inner_cheapest_total);
+
foreach(lc1, outerrel->partial_pathlist)
{
Path *outerpath = (Path *) lfirst(lc1);
@@ -2064,6 +2079,10 @@ consider_parallel_nestloop(PlannerInfo *root,
try_partial_nestloop_path(root, joinrel,
outerpath, mpath,
pathkeys, jointype, extra);
}
+ /* Also consider materialized form of the cheapest inner path */
+ if (matpath != NULL && matpath->parallel_safe)
+ try_partial_nestloop_path(root, joinrel, outerpath,
matpath,
+
pathkeys, jointype, extra);
}
}
diff --git a/src/test/regress/expected/select_parallel.out
b/src/test/regress/expected/select_parallel.out
index d88353d496..5b9f5c58cc 100644
--- a/src/test/regress/expected/select_parallel.out
+++ b/src/test/regress/expected/select_parallel.out
@@ -844,6 +844,30 @@ select * from
(12 rows)
reset enable_material;
+-- test materialized form of the cheapest inner path
+set min_parallel_table_scan_size = '512kB';
+explain(costs off)
+select count(*) from tenk1, int4_tbl where tenk1.two < int4_tbl.f1;
+ QUERY PLAN
+------------------------------------------------------------
+ Finalize Aggregate
+ -> Gather
+ Workers Planned: 4
+ -> Partial Aggregate
+ -> Nested Loop
+ Join Filter: (tenk1.two < int4_tbl.f1)
+ -> Parallel Seq Scan on tenk1
+ -> Materialize
+ -> Seq Scan on int4_tbl
+(9 rows)
+
+select count(*) from tenk1, int4_tbl where tenk1.two < int4_tbl.f1;
+ count
+-------
+ 20000
+(1 row)
+
+set min_parallel_table_scan_size = 0;
reset enable_hashagg;
-- check parallelized int8 aggregate (bug #14897)
explain (costs off)
diff --git a/src/test/regress/sql/select_parallel.sql
b/src/test/regress/sql/select_parallel.sql
index 80c914dc02..0afae0c750 100644
--- a/src/test/regress/sql/select_parallel.sql
+++ b/src/test/regress/sql/select_parallel.sql
@@ -312,6 +312,15 @@ select * from
reset enable_material;
+-- test materialized form of the cheapest inner path
+set min_parallel_table_scan_size = '512kB';
+
+explain(costs off)
+select count(*) from tenk1, int4_tbl where tenk1.two < int4_tbl.f1;
+
+select count(*) from tenk1, int4_tbl where tenk1.two < int4_tbl.f1;
+
+set min_parallel_table_scan_size = 0;
reset enable_hashagg;
-- check parallelized int8 aggregate (bug #14897)
--
2.25.1
