Hi,
Off-list Ashutosh Bapat has suggested using a flag instead of counting
number of
dummy rels and then manipulating on it. That will be simple and smoother.
I agree with his suggestion and updated my patch accordingly.
Thanks
--
Jeevan Chalke
Technical Architect, Product Development
EnterpriseDB Corporation
The Enterprise PostgreSQL Company
From 41c885a31eb61538dc762c74f4d9782da1db9bc8 Mon Sep 17 00:00:00 2001
From: Jeevan Chalke <jeevan.cha...@enterprisedb.com>
Date: Fri, 22 Jun 2018 10:59:24 +0530
Subject: [PATCH] Make sure that we have live children before we append them.
Since 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 should not
try to create any append path by calling add_paths_to_append_rel().
Calling it when no child present to append will result in a server
crash. And appending only a few children will result into data loss.
Thus, don't try to create an append path at first place itself.
In passing, add an Assert() in add_paths_to_append_rel() to check
that it receives a valid live children list.
Jeevan Chalke, reviewed by Ashutosh Bapat.
---
src/backend/optimizer/path/allpaths.c | 3 +
src/backend/optimizer/plan/planner.c | 24 +++--
src/test/regress/expected/partition_aggregate.out | 105 ++++++++++++++++++++++
src/test/regress/sql/partition_aggregate.sql | 24 +++++
4 files changed, 148 insertions(+), 8 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..46128b4 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -7012,12 +7012,14 @@ create_partitionwise_grouping_paths(PlannerInfo *root,
List *grouped_live_children = NIL;
List *partially_grouped_live_children = NIL;
PathTarget *target = grouped_rel->reltarget;
+ bool found_partially_grouped_child;
Assert(patype != PARTITIONWISE_AGGREGATE_NONE);
Assert(patype != PARTITIONWISE_AGGREGATE_PARTIAL ||
partially_grouped_rel != NULL);
/* Add paths for partitionwise aggregation/grouping. */
+ found_partially_grouped_child = true;
for (cnt_parts = 0; cnt_parts < nparts; cnt_parts++)
{
RelOptInfo *child_input_rel = input_rel->part_rels[cnt_parts];
@@ -7091,6 +7093,8 @@ create_partitionwise_grouping_paths(PlannerInfo *root,
lappend(partially_grouped_live_children,
child_partially_grouped_rel);
}
+ else if (found_partially_grouped_child)
+ found_partially_grouped_child = false;
if (patype == PARTITIONWISE_AGGREGATE_FULL)
{
@@ -7103,20 +7107,20 @@ create_partitionwise_grouping_paths(PlannerInfo *root,
}
/*
- * All children can't be dummy at this point. If they are, then the parent
- * too marked as dummy.
- */
- Assert(grouped_live_children != NIL ||
- partially_grouped_live_children != NIL);
-
- /*
* Try to create append paths for partially grouped children. For full
* 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.
+ *
+ * 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 cannot create
+ * any append path.
*/
- if (partially_grouped_rel)
+ if (partially_grouped_rel && found_partially_grouped_child)
{
+ Assert(partially_grouped_live_children != NIL);
+
add_paths_to_append_rel(root, partially_grouped_rel,
partially_grouped_live_children);
@@ -7130,7 +7134,11 @@ create_partitionwise_grouping_paths(PlannerInfo *root,
/* If possible, create append paths for fully grouped children. */
if (patype == PARTITIONWISE_AGGREGATE_FULL)
+ {
+ Assert(grouped_live_children != NIL);
+
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..d286050 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_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_p3
+ -> Parallel Seq Scan on pagg_tab_para_p2
+(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_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
+ -> 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..6d8b739 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_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;
+
+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;
+
+-- 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
