From dd9951b3fd92a61d1b9fb02019a3af5b4d8e3b93 Mon Sep 17 00:00:00 2001 From: Jeevan Chalke Date: Wed, 20 Jun 2018 18:47:12 +0530 Subject: [PATCH] Make sure that we have live children before we append them. In create_partitionwise_grouping_paths(), we were calling add_paths_to_append_rel() on empty live children which causing an Assertion failure inside it. Thus, prevent calling add_paths_to_append_rel() when there are no live children. In passing, add an Assert() in add_paths_to_append_rel() to check that it receives a valid live children list. Also, it is very well possible that we could not have a partially_grouped_rel for some of the children for which aggregation in partial is not feasible. In such cases, we will end up having fewer children in the partially_grouped_live_children list. Don't create append rel in that case. Jeevan Chalke --- src/backend/optimizer/path/allpaths.c | 3 + src/backend/optimizer/plan/planner.c | 25 +++++- src/test/regress/expected/partition_aggregate.out | 105 ++++++++++++++++++++++ src/test/regress/sql/partition_aggregate.sql | 24 +++++ 4 files changed, 155 insertions(+), 2 deletions(-) diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c index 3ada379..9f3d725 100644 --- a/src/backend/optimizer/path/allpaths.c +++ b/src/backend/optimizer/path/allpaths.c @@ -1391,6 +1391,9 @@ add_paths_to_append_rel(PlannerInfo *root, RelOptInfo *rel, bool build_partitioned_rels = false; double partial_rows = -1; + /* We should end-up here only when we have at least one child. */ + Assert(live_childrels != NIL); + /* If appropriate, consider parallel append */ pa_subpaths_valid = enable_parallel_append && rel->consider_parallel; diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c index 67a2c7a..9459b3c 100644 --- a/src/backend/optimizer/plan/planner.c +++ b/src/backend/optimizer/plan/planner.c @@ -7011,12 +7011,14 @@ create_partitionwise_grouping_paths(PlannerInfo *root, int cnt_parts; List *grouped_live_children = NIL; List *partially_grouped_live_children = NIL; + int dummy_children_cnt; PathTarget *target = grouped_rel->reltarget; Assert(patype != PARTITIONWISE_AGGREGATE_NONE); Assert(patype != PARTITIONWISE_AGGREGATE_PARTIAL || partially_grouped_rel != NULL); + dummy_children_cnt = 0; /* Add paths for partitionwise aggregation/grouping. */ for (cnt_parts = 0; cnt_parts < nparts; cnt_parts++) { @@ -7075,6 +7077,8 @@ create_partitionwise_grouping_paths(PlannerInfo *root, if (IS_DUMMY_REL(child_input_rel)) { mark_dummy_rel(child_grouped_rel); + /* Count dummy children. */ + dummy_children_cnt++; continue; } @@ -7114,8 +7118,16 @@ create_partitionwise_grouping_paths(PlannerInfo *root, * partitionwise aggregation, we might have paths in the partial_pathlist * if parallel aggregation is possible. For partial partitionwise * aggregation, we may have paths in both pathlist and partial_pathlist. - */ - if (partially_grouped_rel) + * + * However, it is very well possible that we could not have a + * partially_grouped_rel for some of (or all) the children for which + * aggregation in partial is not feasible. In that case, we might have + * no child or fewer children to append. In case of no child, we cannot + * create any append path. Whereas when there are fewer children, then + * the children count should be number of partitions - dummy children. + */ + if (partially_grouped_rel && partially_grouped_live_children != NIL && + list_length(partially_grouped_live_children) == nparts - dummy_children_cnt) { add_paths_to_append_rel(root, partially_grouped_rel, partially_grouped_live_children); @@ -7130,7 +7142,16 @@ create_partitionwise_grouping_paths(PlannerInfo *root, /* If possible, create append paths for fully grouped children. */ if (patype == PARTITIONWISE_AGGREGATE_FULL) + { + /* + * In case of full partitionwise aggregation we should have grouped rel + * for each non-dummy child. + */ + Assert(grouped_live_children != NIL && + list_length(grouped_live_children) == nparts - dummy_children_cnt); + add_paths_to_append_rel(root, grouped_rel, grouped_live_children); + } } /* diff --git a/src/test/regress/expected/partition_aggregate.out b/src/test/regress/expected/partition_aggregate.out index 76a8209..498592f 100644 --- a/src/test/regress/expected/partition_aggregate.out +++ b/src/test/regress/expected/partition_aggregate.out @@ -1394,3 +1394,108 @@ SELECT y, sum(x), avg(x), count(*) FROM pagg_tab_para GROUP BY y HAVING avg(x) < 11 | 16500 | 11.0000000000000000 | 1500 (4 rows) +-- Test when parent can produce parallel paths but not any (or some) of its children +ALTER TABLE pagg_tab_para_p1 SET (parallel_workers = 0); +ALTER TABLE pagg_tab_para_p2 SET (parallel_workers = 0); +ANALYZE pagg_tab_para; +EXPLAIN (COSTS OFF) +SELECT x, sum(y), avg(y), count(*) FROM pagg_tab_para GROUP BY x HAVING avg(y) < 7 ORDER BY 1, 2, 3; + QUERY PLAN +-------------------------------------------------------------------------------------- + Sort + Sort Key: pagg_tab_para_p1.x, (sum(pagg_tab_para_p1.y)), (avg(pagg_tab_para_p1.y)) + -> Finalize GroupAggregate + Group Key: pagg_tab_para_p1.x + Filter: (avg(pagg_tab_para_p1.y) < '7'::numeric) + -> Gather Merge + Workers Planned: 2 + -> Sort + Sort Key: pagg_tab_para_p1.x + -> Partial HashAggregate + Group Key: pagg_tab_para_p1.x + -> Parallel Append + -> Seq Scan on pagg_tab_para_p1 + -> Seq Scan on pagg_tab_para_p2 + -> Parallel Seq Scan on pagg_tab_para_p3 +(15 rows) + +SELECT x, sum(y), avg(y), count(*) FROM pagg_tab_para GROUP BY x HAVING avg(y) < 7 ORDER BY 1, 2, 3; + x | sum | avg | count +----+------+--------------------+------- + 0 | 5000 | 5.0000000000000000 | 1000 + 1 | 6000 | 6.0000000000000000 | 1000 + 10 | 5000 | 5.0000000000000000 | 1000 + 11 | 6000 | 6.0000000000000000 | 1000 + 20 | 5000 | 5.0000000000000000 | 1000 + 21 | 6000 | 6.0000000000000000 | 1000 +(6 rows) + +ALTER TABLE pagg_tab_para_p3 SET (parallel_workers = 0); +ANALYZE pagg_tab_para; +EXPLAIN (COSTS OFF) +SELECT x, sum(y), avg(y), count(*) FROM pagg_tab_para GROUP BY x HAVING avg(y) < 7 ORDER BY 1, 2, 3; + QUERY PLAN +-------------------------------------------------------------------------------------- + Sort + Sort Key: pagg_tab_para_p1.x, (sum(pagg_tab_para_p1.y)), (avg(pagg_tab_para_p1.y)) + -> Finalize GroupAggregate + Group Key: pagg_tab_para_p1.x + Filter: (avg(pagg_tab_para_p1.y) < '7'::numeric) + -> Gather Merge + Workers Planned: 2 + -> Sort + Sort Key: pagg_tab_para_p1.x + -> Partial HashAggregate + Group Key: pagg_tab_para_p1.x + -> Parallel Append + -> Seq Scan on pagg_tab_para_p1 + -> Seq Scan on pagg_tab_para_p2 + -> Seq Scan on pagg_tab_para_p3 +(15 rows) + +SELECT x, sum(y), avg(y), count(*) FROM pagg_tab_para GROUP BY x HAVING avg(y) < 7 ORDER BY 1, 2, 3; + x | sum | avg | count +----+------+--------------------+------- + 0 | 5000 | 5.0000000000000000 | 1000 + 1 | 6000 | 6.0000000000000000 | 1000 + 10 | 5000 | 5.0000000000000000 | 1000 + 11 | 6000 | 6.0000000000000000 | 1000 + 20 | 5000 | 5.0000000000000000 | 1000 + 21 | 6000 | 6.0000000000000000 | 1000 +(6 rows) + +-- Reset parallelism parameters to get partitionwise aggregation plan. +RESET min_parallel_table_scan_size; +RESET parallel_setup_cost; +EXPLAIN (COSTS OFF) +SELECT x, sum(y), avg(y), count(*) FROM pagg_tab_para GROUP BY x HAVING avg(y) < 7 ORDER BY 1, 2, 3; + QUERY PLAN +-------------------------------------------------------------------------------------- + Sort + Sort Key: pagg_tab_para_p1.x, (sum(pagg_tab_para_p1.y)), (avg(pagg_tab_para_p1.y)) + -> Append + -> HashAggregate + Group Key: pagg_tab_para_p1.x + Filter: (avg(pagg_tab_para_p1.y) < '7'::numeric) + -> Seq Scan on pagg_tab_para_p1 + -> HashAggregate + Group Key: pagg_tab_para_p2.x + Filter: (avg(pagg_tab_para_p2.y) < '7'::numeric) + -> Seq Scan on pagg_tab_para_p2 + -> HashAggregate + Group Key: pagg_tab_para_p3.x + Filter: (avg(pagg_tab_para_p3.y) < '7'::numeric) + -> Seq Scan on pagg_tab_para_p3 +(15 rows) + +SELECT x, sum(y), avg(y), count(*) FROM pagg_tab_para GROUP BY x HAVING avg(y) < 7 ORDER BY 1, 2, 3; + x | sum | avg | count +----+------+--------------------+------- + 0 | 5000 | 5.0000000000000000 | 1000 + 1 | 6000 | 6.0000000000000000 | 1000 + 10 | 5000 | 5.0000000000000000 | 1000 + 11 | 6000 | 6.0000000000000000 | 1000 + 20 | 5000 | 5.0000000000000000 | 1000 + 21 | 6000 | 6.0000000000000000 | 1000 +(6 rows) + diff --git a/src/test/regress/sql/partition_aggregate.sql b/src/test/regress/sql/partition_aggregate.sql index c60d7d2..53654f3 100644 --- a/src/test/regress/sql/partition_aggregate.sql +++ b/src/test/regress/sql/partition_aggregate.sql @@ -294,3 +294,27 @@ SELECT x, sum(y), avg(y), count(*) FROM pagg_tab_para GROUP BY x HAVING avg(y) < EXPLAIN (COSTS OFF) SELECT y, sum(x), avg(x), count(*) FROM pagg_tab_para GROUP BY y HAVING avg(x) < 12 ORDER BY 1, 2, 3; SELECT y, sum(x), avg(x), count(*) FROM pagg_tab_para GROUP BY y HAVING avg(x) < 12 ORDER BY 1, 2, 3; + +-- Test when parent can produce parallel paths but not any (or some) of its children +ALTER TABLE pagg_tab_para_p1 SET (parallel_workers = 0); +ALTER TABLE pagg_tab_para_p2 SET (parallel_workers = 0); +ANALYZE pagg_tab_para; + +EXPLAIN (COSTS OFF) +SELECT x, sum(y), avg(y), count(*) FROM pagg_tab_para GROUP BY x HAVING avg(y) < 7 ORDER BY 1, 2, 3; +SELECT x, sum(y), avg(y), count(*) FROM pagg_tab_para GROUP BY x HAVING avg(y) < 7 ORDER BY 1, 2, 3; + +ALTER TABLE pagg_tab_para_p3 SET (parallel_workers = 0); +ANALYZE pagg_tab_para; + +EXPLAIN (COSTS OFF) +SELECT x, sum(y), avg(y), count(*) FROM pagg_tab_para GROUP BY x HAVING avg(y) < 7 ORDER BY 1, 2, 3; +SELECT x, sum(y), avg(y), count(*) FROM pagg_tab_para GROUP BY x HAVING avg(y) < 7 ORDER BY 1, 2, 3; + +-- Reset parallelism parameters to get partitionwise aggregation plan. +RESET min_parallel_table_scan_size; +RESET parallel_setup_cost; + +EXPLAIN (COSTS OFF) +SELECT x, sum(y), avg(y), count(*) FROM pagg_tab_para GROUP BY x HAVING avg(y) < 7 ORDER BY 1, 2, 3; +SELECT x, sum(y), avg(y), count(*) FROM pagg_tab_para GROUP BY x HAVING avg(y) < 7 ORDER BY 1, 2, 3; -- 1.8.3.1