diff options
| -rw-r--r-- | src/backend/optimizer/plan/planner.c | 39 | ||||
| -rw-r--r-- | src/test/regress/expected/select_parallel.out | 18 | ||||
| -rw-r--r-- | src/test/regress/sql/select_parallel.sql | 6 |
3 files changed, 43 insertions, 20 deletions
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c index dad35e56f58..094c7082b14 100644 --- a/src/backend/optimizer/plan/planner.c +++ b/src/backend/optimizer/plan/planner.c @@ -1788,36 +1788,39 @@ grouping_planner(PlannerInfo *root, bool inheritance_update, Path *subpath = (Path *) lfirst(lc); Path *newpath; + /* Shouldn't have any parameterized paths anymore */ + Assert(subpath->param_info == NULL); + /* * We can't use apply_projection_to_path() here, because there * could already be pointers to these paths, and therefore we - * cannot modify them in place. Instead, we must use - * create_projection_path(). The good news is this won't - * actually insert a Result node into the final plan unless - * it's needed, but the bad news is that it will charge for - * the node whether it's needed or not. Therefore, if the - * target list is already what we need it to be, just leave - * this partial path alone. + * dare not modify them in place. Instead, we must use + * create_projection_path() unconditionally. */ - if (equal(scanjoin_target->exprs, subpath->pathtarget->exprs)) - continue; - - Assert(subpath->param_info == NULL); newpath = (Path *) create_projection_path(root, current_rel, subpath, scanjoin_target); - if (is_projection_capable_path(subpath)) + + /* + * Although create_projection_path() inserts a ProjectionPath + * unconditionally, create_projection_plan() will only insert + * a Result node if the subpath is not projection-capable, so + * we should discount the cost of that node if it will not + * actually get inserted. (This is pretty grotty but we can + * improve it later if it seems important.) + */ + if (equal(scanjoin_target->exprs, subpath->pathtarget->exprs)) { - /* - * Since the target lists differ, a projection path is - * essential, but it will disappear at plan creation time - * because the subpath is projection-capable. So avoid - * charging anything for the disappearing node. - */ + /* at most we need a relabeling of the subpath */ newpath->startup_cost = subpath->startup_cost; newpath->total_cost = subpath->total_cost; } + else if (is_projection_capable_path(subpath)) + { + /* need to project, but we don't need a Result */ + newpath->total_cost -= cpu_tuple_cost * subpath->rows; + } lfirst(lc) = newpath; } diff --git a/src/test/regress/expected/select_parallel.out b/src/test/regress/expected/select_parallel.out index 3c906407df2..2286fafab39 100644 --- a/src/test/regress/expected/select_parallel.out +++ b/src/test/regress/expected/select_parallel.out @@ -6,9 +6,10 @@ create or replace function parallel_restricted(int) returns int as -- Serializable isolation would disable parallel query, so explicitly use an -- arbitrary other level. begin isolation level repeatable read; --- setup parallel test +-- encourage use of parallel plans set parallel_setup_cost=0; set parallel_tuple_cost=0; +set min_parallel_relation_size=0; set max_parallel_workers_per_gather=4; explain (costs off) select count(*) from a_star; @@ -71,6 +72,21 @@ select length(stringu1) from tenk1 group by length(stringu1); 6 (1 row) +explain (costs off) + select stringu1, count(*) from tenk1 group by stringu1 order by stringu1; + QUERY PLAN +---------------------------------------------------- + Sort + Sort Key: stringu1 + -> Finalize HashAggregate + Group Key: stringu1 + -> Gather + Workers Planned: 4 + -> Partial HashAggregate + Group Key: stringu1 + -> Parallel Seq Scan on tenk1 +(9 rows) + -- test that parallel plan for aggregates is not selected when -- target list contains parallel restricted clause. explain (costs off) diff --git a/src/test/regress/sql/select_parallel.sql b/src/test/regress/sql/select_parallel.sql index a8cd9933329..38d3166742c 100644 --- a/src/test/regress/sql/select_parallel.sql +++ b/src/test/regress/sql/select_parallel.sql @@ -9,9 +9,10 @@ create or replace function parallel_restricted(int) returns int as -- arbitrary other level. begin isolation level repeatable read; --- setup parallel test +-- encourage use of parallel plans set parallel_setup_cost=0; set parallel_tuple_cost=0; +set min_parallel_relation_size=0; set max_parallel_workers_per_gather=4; explain (costs off) @@ -29,6 +30,9 @@ explain (costs off) select length(stringu1) from tenk1 group by length(stringu1); select length(stringu1) from tenk1 group by length(stringu1); +explain (costs off) + select stringu1, count(*) from tenk1 group by stringu1 order by stringu1; + -- test that parallel plan for aggregates is not selected when -- target list contains parallel restricted clause. explain (costs off) |