public inbox for [email protected]
help / color / mirror / Atom feedFrom: Antonin Houska <[email protected]>
Subject: [PATCH 3/3] Use also partial paths as the input for grouped paths.
Date: Thu, 17 Nov 2022 10:41:12 +0100
---
src/backend/optimizer/path/allpaths.c | 44 +++++-
src/backend/optimizer/util/relnode.c | 46 +++---
src/test/regress/expected/agg_pushdown.out | 156 +++++++++++++++++++++
src/test/regress/sql/agg_pushdown.sql | 65 +++++++++
4 files changed, 281 insertions(+), 30 deletions(-)
diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c
index 6638311ead..50f0cf8365 100644
--- a/src/backend/optimizer/path/allpaths.c
+++ b/src/backend/optimizer/path/allpaths.c
@@ -130,7 +130,7 @@ static void set_worktable_pathlist(PlannerInfo *root, RelOptInfo *rel,
RangeTblEntry *rte);
static void add_grouped_path(PlannerInfo *root, RelOptInfo *rel,
Path *subpath, AggStrategy aggstrategy,
- RelAggInfo *agg_info);
+ RelAggInfo *agg_info, bool partial);
static RelOptInfo *make_rel_from_joinlist(PlannerInfo *root, List *joinlist);
static bool subquery_is_pushdown_safe(Query *subquery, Query *topquery,
pushdown_safety_info *safetyInfo);
@@ -3343,6 +3343,7 @@ generate_grouping_paths(PlannerInfo *root, RelOptInfo *rel_grouped,
RelOptInfo *rel_plain, RelAggInfo *agg_info)
{
ListCell *lc;
+ Path *path;
if (IS_DUMMY_REL(rel_plain))
{
@@ -3352,7 +3353,7 @@ generate_grouping_paths(PlannerInfo *root, RelOptInfo *rel_grouped,
foreach(lc, rel_plain->pathlist)
{
- Path *path = (Path *) lfirst(lc);
+ path = (Path *) lfirst(lc);
/*
* Since the path originates from the non-grouped relation which is
@@ -3366,7 +3367,8 @@ generate_grouping_paths(PlannerInfo *root, RelOptInfo *rel_grouped,
* add_grouped_path() will check whether the path has suitable
* pathkeys.
*/
- add_grouped_path(root, rel_grouped, path, AGG_SORTED, agg_info);
+ add_grouped_path(root, rel_grouped, path, AGG_SORTED, agg_info,
+ false);
/*
* Repeated creation of hash table (for new parameter values) should
@@ -3374,12 +3376,38 @@ generate_grouping_paths(PlannerInfo *root, RelOptInfo *rel_grouped,
* efficiency.
*/
if (path->param_info == NULL)
- add_grouped_path(root, rel_grouped, path, AGG_HASHED, agg_info);
+ add_grouped_path(root, rel_grouped, path, AGG_HASHED, agg_info,
+ false);
}
/* Could not generate any grouped paths? */
if (rel_grouped->pathlist == NIL)
+ {
mark_dummy_rel(rel_grouped);
+ return;
+ }
+
+ /*
+ * Almost the same for partial paths.
+ *
+ * The difference is that parameterized paths are never created, see
+ * add_partial_path() for explanation.
+ */
+ foreach(lc, rel_plain->partial_pathlist)
+ {
+ path = (Path *) lfirst(lc);
+
+ if (path->param_info != NULL)
+ continue;
+
+ path = (Path *) create_projection_path(root, rel_grouped, path,
+ agg_info->agg_input);
+
+ add_grouped_path(root, rel_grouped, path, AGG_SORTED, agg_info,
+ true);
+ add_grouped_path(root, rel_grouped, path, AGG_HASHED, agg_info,
+ true);
+ }
}
/*
@@ -3387,7 +3415,8 @@ generate_grouping_paths(PlannerInfo *root, RelOptInfo *rel_grouped,
*/
static void
add_grouped_path(PlannerInfo *root, RelOptInfo *rel, Path *subpath,
- AggStrategy aggstrategy, RelAggInfo *agg_info)
+ AggStrategy aggstrategy, RelAggInfo *agg_info,
+ bool partial)
{
Path *agg_path;
@@ -3410,7 +3439,10 @@ add_grouped_path(PlannerInfo *root, RelOptInfo *rel, Path *subpath,
return;
/* Add the grouped path to the list of grouped base paths. */
- add_path(rel, (Path *) agg_path);
+ if (!partial)
+ add_path(rel, (Path *) agg_path);
+ else
+ add_partial_path(rel, (Path *) agg_path);
}
/*
diff --git a/src/backend/optimizer/util/relnode.c b/src/backend/optimizer/util/relnode.c
index 556f25bece..17aa8804ff 100644
--- a/src/backend/optimizer/util/relnode.c
+++ b/src/backend/optimizer/util/relnode.c
@@ -948,33 +948,12 @@ build_join_rel(PlannerInfo *root,
build_joinrel_partition_info(joinrel, outer_rel, inner_rel,
restrictlist, sjinfo->jointype);
+ /*
+ * Set estimates of the joinrel's size.
+ */
if (!grouped)
- {
- /*
- * Set estimates of the joinrel's size.
- */
set_joinrel_size_estimates(root, joinrel, outer_rel, inner_rel,
sjinfo, restrictlist);
-
- /*
- * Set the consider_parallel flag if this joinrel could potentially be
- * scanned within a parallel worker. If this flag is false for either
- * inner_rel or outer_rel, then it must be false for the joinrel also.
- * Even if both are true, there might be parallel-restricted
- * expressions in the targetlist or quals.
- *
- * Note that if there are more than two rels in this relation, they
- * could be divided between inner_rel and outer_rel in any arbitrary
- * way. We assume this doesn't matter, because we should hit all the
- * same baserels and joinclauses while building up to this joinrel no
- * matter which we take; therefore, we should make the same decision
- * here however we get here.
- */
- if (inner_rel->consider_parallel && outer_rel->consider_parallel &&
- is_parallel_safe(root, (Node *) restrictlist) &&
- is_parallel_safe(root, (Node *) joinrel->reltarget->exprs))
- joinrel->consider_parallel = true;
- }
else
{
/*
@@ -990,6 +969,25 @@ build_join_rel(PlannerInfo *root,
agg_info->input_rows, NULL, NULL);
}
+ /*
+ * Set the consider_parallel flag if this joinrel could potentially be
+ * scanned within a parallel worker. If this flag is false for either
+ * inner_rel or outer_rel, then it must be false for the joinrel also.
+ * Even if both are true, there might be parallel-restricted expressions
+ * in the targetlist or quals.
+ *
+ * Note that if there are more than two rels in this relation, they could
+ * be divided between inner_rel and outer_rel in any arbitrary way. We
+ * assume this doesn't matter, because we should hit all the same baserels
+ * and joinclauses while building up to this joinrel no matter which we
+ * take; therefore, we should make the same decision here however we get
+ * here.
+ */
+ if (inner_rel->consider_parallel && outer_rel->consider_parallel &&
+ is_parallel_safe(root, (Node *) restrictlist) &&
+ is_parallel_safe(root, (Node *) joinrel->reltarget->exprs))
+ joinrel->consider_parallel = true;
+
/* Add the joinrel to the PlannerInfo. */
if (!grouped)
add_join_rel(root, joinrel);
diff --git a/src/test/regress/expected/agg_pushdown.out b/src/test/regress/expected/agg_pushdown.out
index 03a5ccf571..66d36d122e 100644
--- a/src/test/regress/expected/agg_pushdown.out
+++ b/src/test/regress/expected/agg_pushdown.out
@@ -214,3 +214,159 @@ c2.parent = p.i WHERE c1.j = c2.k GROUP BY p.i;
-> Index Only Scan using agg_pushdown_parent_pkey on agg_pushdown_parent p
(13 rows)
+-- Most of the tests above with parallel query processing enforced.
+SET min_parallel_index_scan_size = 0;
+SET min_parallel_table_scan_size = 0;
+SET parallel_setup_cost = 0;
+SET parallel_tuple_cost = 0;
+-- Partially aggregate a single relation.
+--
+-- Nestloop join.
+SET enable_nestloop TO on;
+SET enable_hashjoin TO off;
+SET enable_mergejoin TO off;
+EXPLAIN (COSTS off)
+SELECT p.x, avg(c1.v) FROM agg_pushdown_parent AS p JOIN agg_pushdown_child1
+AS c1 ON c1.parent = p.i GROUP BY p.i;
+ QUERY PLAN
+------------------------------------------------------------------------------------------------------------
+ Finalize GroupAggregate
+ Group Key: p.i
+ -> Gather Merge
+ Workers Planned: 1
+ -> Nested Loop
+ -> Partial GroupAggregate
+ Group Key: c1.parent
+ -> Parallel Index Scan using agg_pushdown_child1_parent_idx on agg_pushdown_child1 c1
+ -> Index Scan using agg_pushdown_parent_pkey on agg_pushdown_parent p
+ Index Cond: (i = c1.parent)
+(10 rows)
+
+-- Hash join.
+SET enable_nestloop TO off;
+SET enable_hashjoin TO on;
+EXPLAIN (COSTS off)
+SELECT p.i, avg(c1.v) FROM agg_pushdown_parent AS p JOIN agg_pushdown_child1
+AS c1 ON c1.parent = p.i GROUP BY p.i;
+ QUERY PLAN
+------------------------------------------------------------------------------------------------------------------
+ Finalize GroupAggregate
+ Group Key: p.i
+ -> Sort
+ Sort Key: p.i
+ -> Gather
+ Workers Planned: 1
+ -> Parallel Hash Join
+ Hash Cond: (c1.parent = p.i)
+ -> Partial GroupAggregate
+ Group Key: c1.parent
+ -> Parallel Index Scan using agg_pushdown_child1_parent_idx on agg_pushdown_child1 c1
+ -> Parallel Hash
+ -> Parallel Index Only Scan using agg_pushdown_parent_pkey on agg_pushdown_parent p
+(13 rows)
+
+-- Merge join.
+SET enable_hashjoin TO off;
+SET enable_mergejoin TO on;
+EXPLAIN (COSTS off)
+SELECT p.i, avg(c1.v) FROM agg_pushdown_parent AS p JOIN agg_pushdown_child1
+AS c1 ON c1.parent = p.i GROUP BY p.i;
+ QUERY PLAN
+------------------------------------------------------------------------------------------------------------
+ Finalize GroupAggregate
+ Group Key: p.i
+ -> Gather Merge
+ Workers Planned: 1
+ -> Merge Join
+ Merge Cond: (c1.parent = p.i)
+ -> Partial GroupAggregate
+ Group Key: c1.parent
+ -> Parallel Index Scan using agg_pushdown_child1_parent_idx on agg_pushdown_child1 c1
+ -> Index Only Scan using agg_pushdown_parent_pkey on agg_pushdown_parent p
+(10 rows)
+
+SET enable_nestloop TO on;
+SET enable_hashjoin TO on;
+-- Perform nestloop join between agg_pushdown_child1 and agg_pushdown_child2
+-- and aggregate the result.
+SET enable_nestloop TO on;
+SET enable_hashjoin TO off;
+SET enable_mergejoin TO off;
+EXPLAIN (COSTS off)
+SELECT p.i, avg(c1.v + c2.v) FROM agg_pushdown_parent AS p JOIN
+agg_pushdown_child1 AS c1 ON c1.parent = p.i JOIN agg_pushdown_child2 AS c2 ON
+c2.parent = p.i WHERE c1.j = c2.k GROUP BY p.i;
+ QUERY PLAN
+------------------------------------------------------------------------------------------------------------------
+ Finalize GroupAggregate
+ Group Key: p.i
+ -> Gather Merge
+ Workers Planned: 2
+ -> Sort
+ Sort Key: p.i
+ -> Nested Loop
+ -> Partial HashAggregate
+ Group Key: c1.parent
+ -> Nested Loop
+ -> Parallel Index Scan using agg_pushdown_child1_pkey on agg_pushdown_child1 c1
+ -> Index Scan using agg_pushdown_child2_pkey on agg_pushdown_child2 c2
+ Index Cond: ((k = c1.j) AND (parent = c1.parent))
+ -> Index Only Scan using agg_pushdown_parent_pkey on agg_pushdown_parent p
+ Index Cond: (i = c1.parent)
+(15 rows)
+
+-- The same for hash join.
+SET enable_nestloop TO off;
+SET enable_hashjoin TO on;
+EXPLAIN (COSTS off)
+SELECT p.i, avg(c1.v + c2.v) FROM agg_pushdown_parent AS p JOIN
+agg_pushdown_child1 AS c1 ON c1.parent = p.i JOIN agg_pushdown_child2 AS c2 ON
+c2.parent = p.i WHERE c1.j = c2.k GROUP BY p.i;
+ QUERY PLAN
+------------------------------------------------------------------------------------------------------------------------
+ Finalize GroupAggregate
+ Group Key: p.i
+ -> Gather Merge
+ Workers Planned: 1
+ -> Sort
+ Sort Key: p.i
+ -> Parallel Hash Join
+ Hash Cond: (c1.parent = p.i)
+ -> Partial HashAggregate
+ Group Key: c1.parent
+ -> Parallel Hash Join
+ Hash Cond: ((c1.parent = c2.parent) AND (c1.j = c2.k))
+ -> Parallel Index Scan using agg_pushdown_child1_parent_idx on agg_pushdown_child1 c1
+ -> Parallel Hash
+ -> Parallel Index Scan using agg_pushdown_child2_pkey on agg_pushdown_child2 c2
+ -> Parallel Hash
+ -> Parallel Index Only Scan using agg_pushdown_parent_pkey on agg_pushdown_parent p
+(17 rows)
+
+-- The same for merge join.
+SET enable_hashjoin TO off;
+SET enable_mergejoin TO on;
+SET enable_seqscan TO off;
+EXPLAIN (COSTS off)
+SELECT p.i, avg(c1.v + c2.v) FROM agg_pushdown_parent AS p JOIN
+agg_pushdown_child1 AS c1 ON c1.parent = p.i JOIN agg_pushdown_child2 AS c2 ON
+c2.parent = p.i WHERE c1.j = c2.k GROUP BY p.i;
+ QUERY PLAN
+------------------------------------------------------------------------------------------------------------------
+ Finalize GroupAggregate
+ Group Key: p.i
+ -> Gather Merge
+ Workers Planned: 2
+ -> Merge Join
+ Merge Cond: (c1.parent = p.i)
+ -> Sort
+ Sort Key: c1.parent
+ -> Partial HashAggregate
+ Group Key: c1.parent
+ -> Merge Join
+ Merge Cond: ((c1.j = c2.k) AND (c1.parent = c2.parent))
+ -> Parallel Index Scan using agg_pushdown_child1_pkey on agg_pushdown_child1 c1
+ -> Index Scan using agg_pushdown_child2_pkey on agg_pushdown_child2 c2
+ -> Index Only Scan using agg_pushdown_parent_pkey on agg_pushdown_parent p
+(15 rows)
+
diff --git a/src/test/regress/sql/agg_pushdown.sql b/src/test/regress/sql/agg_pushdown.sql
index 0a4614592b..49ba6dd67c 100644
--- a/src/test/regress/sql/agg_pushdown.sql
+++ b/src/test/regress/sql/agg_pushdown.sql
@@ -113,3 +113,68 @@ EXPLAIN (COSTS off)
SELECT p.i, avg(c1.v + c2.v) FROM agg_pushdown_parent AS p JOIN
agg_pushdown_child1 AS c1 ON c1.parent = p.i JOIN agg_pushdown_child2 AS c2 ON
c2.parent = p.i WHERE c1.j = c2.k GROUP BY p.i;
+
+-- Most of the tests above with parallel query processing enforced.
+SET min_parallel_index_scan_size = 0;
+SET min_parallel_table_scan_size = 0;
+SET parallel_setup_cost = 0;
+SET parallel_tuple_cost = 0;
+
+-- Partially aggregate a single relation.
+--
+-- Nestloop join.
+SET enable_nestloop TO on;
+SET enable_hashjoin TO off;
+SET enable_mergejoin TO off;
+EXPLAIN (COSTS off)
+SELECT p.x, avg(c1.v) FROM agg_pushdown_parent AS p JOIN agg_pushdown_child1
+AS c1 ON c1.parent = p.i GROUP BY p.i;
+
+-- Hash join.
+SET enable_nestloop TO off;
+SET enable_hashjoin TO on;
+
+EXPLAIN (COSTS off)
+SELECT p.i, avg(c1.v) FROM agg_pushdown_parent AS p JOIN agg_pushdown_child1
+AS c1 ON c1.parent = p.i GROUP BY p.i;
+
+-- Merge join.
+SET enable_hashjoin TO off;
+SET enable_mergejoin TO on;
+
+EXPLAIN (COSTS off)
+SELECT p.i, avg(c1.v) FROM agg_pushdown_parent AS p JOIN agg_pushdown_child1
+AS c1 ON c1.parent = p.i GROUP BY p.i;
+
+SET enable_nestloop TO on;
+SET enable_hashjoin TO on;
+
+-- Perform nestloop join between agg_pushdown_child1 and agg_pushdown_child2
+-- and aggregate the result.
+SET enable_nestloop TO on;
+SET enable_hashjoin TO off;
+SET enable_mergejoin TO off;
+
+EXPLAIN (COSTS off)
+SELECT p.i, avg(c1.v + c2.v) FROM agg_pushdown_parent AS p JOIN
+agg_pushdown_child1 AS c1 ON c1.parent = p.i JOIN agg_pushdown_child2 AS c2 ON
+c2.parent = p.i WHERE c1.j = c2.k GROUP BY p.i;
+
+-- The same for hash join.
+SET enable_nestloop TO off;
+SET enable_hashjoin TO on;
+
+EXPLAIN (COSTS off)
+SELECT p.i, avg(c1.v + c2.v) FROM agg_pushdown_parent AS p JOIN
+agg_pushdown_child1 AS c1 ON c1.parent = p.i JOIN agg_pushdown_child2 AS c2 ON
+c2.parent = p.i WHERE c1.j = c2.k GROUP BY p.i;
+
+-- The same for merge join.
+SET enable_hashjoin TO off;
+SET enable_mergejoin TO on;
+SET enable_seqscan TO off;
+
+EXPLAIN (COSTS off)
+SELECT p.i, avg(c1.v + c2.v) FROM agg_pushdown_parent AS p JOIN
+agg_pushdown_child1 AS c1 ON c1.parent = p.i JOIN agg_pushdown_child2 AS c2 ON
+c2.parent = p.i WHERE c1.j = c2.k GROUP BY p.i;
--
2.31.1
--=-=-=
Content-Type: text/x-diff
Content-Disposition: inline; filename=v21-fixes.patch
diff --git a/src/backend/optimizer/README b/src/backend/optimizer/README
index 2fd1a96269..db97bd254d 100644
--- a/src/backend/optimizer/README
+++ b/src/backend/optimizer/README
@@ -1186,14 +1186,34 @@ Thus the join above the partial aggregate node receives fewer input rows, and
so the number of outer-to-inner pairs of tuples to be checked can be
significantly lower, which can in turn lead to considerably lower join cost.
-Note that there's often no GROUP BY expression to be used for the partial
-aggregation, so we use equivalence classes to derive grouping expression: in
-the example above, the grouping key "b.j" was derived from "a.i".
-
-Also note that in this case the partial aggregate uses the "b.j" as grouping
-column although the column does not appear in the query target list. The point
-is that "b.j" is needed to evaluate the join condition, and there's no other
-way for the partial aggregate to emit its values.
+Note that the GROUP BY expression might not be useful for the partial
+aggregate. In the example above, the aggregate avg(b.y) references table "b",
+but the GROUP BY expression mentions "a". However, the equivalence class {a.i,
+b.j} allows us to use the b.j column as a grouping key for the partial
+aggregation of the "b" table. The equivalence class mechanism is suitable
+because it's designed to derive join clauses, and at the same time the join
+clauses determine the choice of grouping columns of the partial aggregate: the
+only way for the partial aggregate to provide upper join(s) with input values
+is to have the join input expression(s) in the grouping key: besides grouping
+columns, the partial aggregate can only produce the transient states of the
+aggregate functions, but aggregate functions cannot be referenced by the JOIN
+clauses.
+
+Regarding correctness, join node considers the output of the partial aggregate
+to be equivalent to the output of a plain (non-aggregated) relation scan. That
+is, a group (i.e. a row of the partial aggregate output) matches the other
+side of the join if and only if each row of the non-aggregate relation
+does. In other words, all rows belonging to the same group have the same value
+of the join columns (As mentioned above, a join cannot reference other output
+expressions of the partial aggregate than the grouping expressions.).
+
+However, there's a restriction from the aggregate's perspective: the aggregate
+cannot be pushed down if any column referenced by either grouping expression
+or aggregate function can be set to NULL by an outer join above the relation
+to which we want to apply the partiall aggregation. The point is that those
+NULL values would not appear on the input of the pushed-down aggregate, so it could
+either put the rows into groups in a different way than the aggregate at the
+top of the plan, or it could compute wrong values of the aggregate functions.
Besides base relation, the aggregation can also be pushed down to join:
diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c
index 32b3dedc71..50f0cf8365 100644
--- a/src/backend/optimizer/path/allpaths.c
+++ b/src/backend/optimizer/path/allpaths.c
@@ -197,8 +197,7 @@ make_one_rel(PlannerInfo *root, List *joinlist)
* Now that the sizes are known, we can estimate the sizes of the grouped
* relations.
*/
- if (root->grouped_var_list)
- setup_base_grouped_rels(root);
+ setup_base_grouped_rels(root);
/*
* We should now have size estimates for every actual table involved in
@@ -341,7 +340,7 @@ set_base_rel_sizes(PlannerInfo *root)
}
/*
- * setup_based_grouped_rels
+ * setup_base_grouped_rels
* For each "plain" relation build a grouped relation if aggregate pushdown
* is possible and if this relation is suitable for partial aggregation.
*/
@@ -350,6 +349,11 @@ setup_base_grouped_rels(PlannerInfo *root)
{
Index rti;
+ /* If there are no grouping expressions, no aggregate push-down. */
+ if (!root->grouped_var_list)
+ return;
+
+
for (rti = 1; rti < root->simple_rel_array_size; rti++)
{
RelOptInfo *brel = root->simple_rel_array[rti];
@@ -362,6 +366,10 @@ setup_base_grouped_rels(PlannerInfo *root)
Assert(brel->relid == rti); /* sanity check on array */
+ /* ignore RTEs that are "other rels" */
+ if (brel->reloptkind != RELOPT_BASEREL)
+ continue;
+
/*
* The aggregate push-down feature only makes sense if there are
* multiple base rels in the query.
@@ -369,16 +377,13 @@ setup_base_grouped_rels(PlannerInfo *root)
if (!bms_nonempty_difference(root->all_baserels, brel->relids))
continue;
- /* ignore RTEs that are "other rels" */
- if (brel->reloptkind != RELOPT_BASEREL)
+ rel_grouped = build_simple_grouped_rel(root, brel->relid, &agg_info);
+ /* Couldn't any aggregate be pushed down to this relation? */
+ if (!rel_grouped)
continue;
- rel_grouped = build_simple_grouped_rel(root, brel->relid, &agg_info);
- if (rel_grouped)
- {
- /* Make the relation available for joining. */
- add_grouped_rel(root, rel_grouped, agg_info);
- }
+ /* Make the relation available for joining. */
+ add_grouped_rel(root, rel_grouped, agg_info);
}
}
@@ -554,21 +559,8 @@ set_rel_pathlist(PlannerInfo *root, RelOptInfo *rel,
}
else
{
- RelOptInfo *rel_grouped;
- RelAggInfo *agg_info;
-
/* Plain relation */
set_plain_rel_pathlist(root, rel, rte);
-
- /* Add paths to the grouped relation if one exists. */
- rel_grouped = find_grouped_rel(root, rel->relids,
- &agg_info);
- if (rel_grouped)
- {
- generate_grouping_paths(root, rel_grouped, rel,
- agg_info);
- set_cheapest(rel_grouped);
- }
}
break;
case RTE_SUBQUERY:
@@ -836,6 +828,8 @@ static void
set_plain_rel_pathlist(PlannerInfo *root, RelOptInfo *rel, RangeTblEntry *rte)
{
Relids required_outer;
+ RelOptInfo *rel_grouped;
+ RelAggInfo *agg_info;
/*
* We don't support pushing join clauses into the quals of a seqscan, but
@@ -856,6 +850,14 @@ set_plain_rel_pathlist(PlannerInfo *root, RelOptInfo *rel, RangeTblEntry *rte)
/* Consider TID scans */
create_tidscan_paths(root, rel);
+
+ /* Add paths to the grouped relation if one exists. */
+ rel_grouped = find_grouped_rel(root, rel->relids, &agg_info);
+ if (!rel_grouped)
+ return;
+
+ generate_grouping_paths(root, rel_grouped, rel, agg_info);
+ set_cheapest(rel_grouped);
}
/*
@@ -3428,14 +3430,19 @@ add_grouped_path(PlannerInfo *root, RelOptInfo *rel, Path *subpath,
else
elog(ERROR, "unexpected strategy %d", aggstrategy);
+ /*
+ * Bail out if we failed to create a suitable aggregated path. This can
+ * happen e.g. then the path does not support hashing (for AGG_HASHED),
+ * or when the input path is not sorted.
+ */
+ if (agg_path == NULL)
+ return;
+
/* Add the grouped path to the list of grouped base paths. */
- if (agg_path != NULL)
- {
- if (!partial)
- add_path(rel, (Path *) agg_path);
- else
- add_partial_path(rel, (Path *) agg_path);
- }
+ if (!partial)
+ add_path(rel, (Path *) agg_path);
+ else
+ add_partial_path(rel, (Path *) agg_path);
}
/*
@@ -3579,7 +3586,6 @@ standard_join_search(PlannerInfo *root, int levels_needed, List *initial_rels)
for (lev = 2; lev <= levels_needed; lev++)
{
- RelOptInfo *rel_grouped;
ListCell *lc;
/*
@@ -3601,6 +3607,8 @@ standard_join_search(PlannerInfo *root, int levels_needed, List *initial_rels)
*/
foreach(lc, root->join_rel_level[lev])
{
+ RelOptInfo *rel_grouped;
+
rel = (RelOptInfo *) lfirst(lc);
/* Create paths for partitionwise joins. */
diff --git a/src/backend/optimizer/path/costsize.c b/src/backend/optimizer/path/costsize.c
index b34ad90d08..4688f561f0 100644
--- a/src/backend/optimizer/path/costsize.c
+++ b/src/backend/optimizer/path/costsize.c
@@ -4999,6 +4999,7 @@ set_baserel_size_estimates(PlannerInfo *root, RelOptInfo *rel)
0,
JOIN_INNER,
NULL);
+
rel->rows = clamp_row_est(nrows);
cost_qual_eval(&rel->baserestrictcost, rel->baserestrictinfo, root);
diff --git a/src/backend/optimizer/plan/initsplan.c b/src/backend/optimizer/plan/initsplan.c
index 8e913c92d8..8dc39765f2 100644
--- a/src/backend/optimizer/plan/initsplan.c
+++ b/src/backend/optimizer/plan/initsplan.c
@@ -355,7 +355,8 @@ create_aggregate_grouped_var_infos(PlannerInfo *root)
Assert(root->grouped_var_list == NIL);
tlist_exprs = pull_var_clause((Node *) root->processed_tlist,
- PVC_INCLUDE_AGGREGATES);
+ PVC_INCLUDE_AGGREGATES |
+ PVC_RECURSE_WINDOWFUNCS);
/*
* Although GroupingFunc is related to root->parse->groupingSets, this
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index 0ada3ba3eb..3292b4b419 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -6847,6 +6847,12 @@ create_partial_grouping_paths(PlannerInfo *root,
* push-down.
*/
partially_grouped_rel = find_grouped_rel(root, input_rel->relids, NULL);
+
+ /*
+ * If the relation already exists, it must have been created by aggregate
+ * pushdown. We can't check how exactly it got created, but we can at
+ * least check that aggregate pushdown is enabled.
+ */
Assert(enable_agg_pushdown || partially_grouped_rel == NULL);
/*
@@ -6871,17 +6877,25 @@ create_partial_grouping_paths(PlannerInfo *root,
/*
* If we can't partially aggregate partial paths, and we can't partially
* aggregate non-partial paths, then don't bother creating the new
- * RelOptInfo at all, unless the caller specified force_rel_creation.
+ * RelOptInfo at all, unless the caller specified force_rel_creation. However
*/
if (cheapest_total_path == NULL &&
cheapest_partial_path == NULL &&
- !force_rel_creation &&
- partially_grouped_rel == NULL)
- return NULL;
+ !force_rel_creation)
+ {
+ /*
+ * If partially_grouped_rel exists, it should contain paths generated
+ * by the aggregate push-down feature, so the caller is interested in
+ * it.
+ */
+ return partially_grouped_rel;
+ }
/*
* Build a new upper relation to represent the result of partially
- * aggregating the rows from the input relation.
+ * aggregating the rows from the input relation. The relation may already
+ * exist due to aggregate pushdown, in which case we don't need to create
+ * it.
*/
if (partially_grouped_rel == NULL)
partially_grouped_rel = fetch_upper_rel(root,
@@ -6903,6 +6917,11 @@ create_partial_grouping_paths(PlannerInfo *root,
*
* If the target was already created for the sake of aggregate push-down,
* it should be compatible with what we'd create here.
+ *
+ * XXX If fetch_upper_rel() had to create a new relation (i.e. aggregate
+ * push-down generated no paths), it created an empty target. Should we
+ * change the convention and have it assign NULL to reltarget instead? Or
+ * should we introduce a function like is_pathtarget_empty()?
*/
if (partially_grouped_rel->reltarget->exprs == NIL)
partially_grouped_rel->reltarget =
diff --git a/src/backend/optimizer/util/pathnode.c b/src/backend/optimizer/util/pathnode.c
index 7025ebf94b..2627e2f252 100644
--- a/src/backend/optimizer/util/pathnode.c
+++ b/src/backend/optimizer/util/pathnode.c
@@ -3163,6 +3163,9 @@ create_agg_path(PlannerInfo *root,
}
/*
+ * create_agg_sorted_path
+ * Creates a pathnode performing sorted aggregation/grouping
+ *
* Apply AGG_SORTED aggregation path to subpath if it's suitably sorted.
*
* NULL is returned if sorting of subpath output is not suitable.
@@ -3176,45 +3179,32 @@ create_agg_sorted_path(PlannerInfo *root, RelOptInfo *rel, Path *subpath,
AggClauseCosts agg_costs;
PathTarget *target;
double dNumGroups;
- ListCell *lc1;
- List *key_subset = NIL;
AggPath *result = NULL;
aggsplit = AGGSPLIT_INITIAL_SERIAL;
agg_exprs = agg_info->agg_exprs;
target = agg_info->target;
- if (subpath->pathkeys == NIL)
- return NULL;
-
- if (!grouping_is_sortable(root->parse->groupClause))
+ /* group_pathkeys are necessary to evaluate the sorting. */
+ if (agg_info->group_pathkeys == NIL)
return NULL;
/*
- * Find all query pathkeys that our relation does affect.
+ * The input path must be sorted in a specific way, but if it's not sorted
+ * at all, it's not useful for AGG_SORTED.
*/
- foreach(lc1, root->group_pathkeys)
- {
- PathKey *gkey = castNode(PathKey, lfirst(lc1));
- ListCell *lc2;
-
- foreach(lc2, subpath->pathkeys)
- {
- PathKey *skey = castNode(PathKey, lfirst(lc2));
-
- if (skey == gkey)
- {
- key_subset = lappend(key_subset, gkey);
- break;
- }
- }
- }
+ if (subpath->pathkeys == NIL)
+ return NULL;
- if (key_subset == NIL)
+ /* Are the grouping clauses suitable for sorted aggregation? */
+ if (!grouping_is_sortable(agg_info->group_clauses))
return NULL;
- /* Check if AGG_SORTED is useful for the whole query. */
- if (!pathkeys_contained_in(key_subset, subpath->pathkeys))
+ /*
+ * Is the input path sorted enough for this grouping? TODO Consider using
+ * incremental sort if the sorting is "almost sufficient".
+ */
+ if (!pathkeys_contained_in(agg_info->group_pathkeys, subpath->pathkeys))
return NULL;
MemSet(&agg_costs, 0, sizeof(AggClauseCosts));
@@ -3231,7 +3221,7 @@ create_agg_sorted_path(PlannerInfo *root, RelOptInfo *rel, Path *subpath,
result = create_agg_path(root, rel, subpath, target,
AGG_SORTED, aggsplit,
agg_info->group_clauses,
- NIL,
+ NIL, /* qual for HAVING clause */
&agg_costs,
dNumGroups);
@@ -3254,7 +3244,6 @@ create_agg_hashed_path(PlannerInfo *root, RelOptInfo *rel,
AggClauseCosts agg_costs;
PathTarget *target;
double dNumGroups;
- double hashaggtablesize;
Query *parse = root->parse;
AggPath *result = NULL;
@@ -3279,25 +3268,18 @@ create_agg_hashed_path(PlannerInfo *root, RelOptInfo *rel,
dNumGroups = estimate_num_groups(root, agg_info->group_exprs,
subpath->rows, NULL, NULL);
- hashaggtablesize = estimate_hashagg_tablesize(root, subpath,
- &agg_costs,
- dNumGroups);
-
- if (hashaggtablesize < work_mem * 1024L)
- {
- /*
- * qual is NIL because the HAVING clause cannot be evaluated until
- * the final value of the aggregate is known.
- */
- result = create_agg_path(root, rel, subpath,
- target,
- AGG_HASHED,
- aggsplit,
- agg_info->group_clauses,
- NIL,
- &agg_costs,
- dNumGroups);
- }
+ /*
+ * qual is NIL because the HAVING clause cannot be evaluated until the
+ * final value of the aggregate is known.
+ */
+ result = create_agg_path(root, rel, subpath,
+ target,
+ AGG_HASHED,
+ aggsplit,
+ agg_info->group_clauses,
+ NIL, /* qual for HAVING clause */
+ &agg_costs,
+ dNumGroups);
}
return result;
diff --git a/src/backend/optimizer/util/relnode.c b/src/backend/optimizer/util/relnode.c
index ce2e267e91..fcec58f10a 100644
--- a/src/backend/optimizer/util/relnode.c
+++ b/src/backend/optimizer/util/relnode.c
@@ -2330,15 +2330,16 @@ create_rel_agg_info(PlannerInfo *root, RelOptInfo *rel)
List *grp_exprs_extra = NIL;
List *group_clauses_final;
int i;
+ bool pk_found, pk_missing;
/*
* The function shouldn't have been called if there's no opportunity for
- * aggregation push-down.
+ * aggregate push-down.
*/
Assert(root->grouped_var_list != NIL);
/*
- * The current implementation of aggregation push-down cannot handle
+ * The current implementation of aggregate push-down cannot handle
* PlaceHolderVar (PHV).
*
* If we knew that the PHV should be evaluated in this target (and of
@@ -2608,11 +2609,13 @@ create_rel_agg_info(PlannerInfo *root, RelOptInfo *rel)
*/
i = 0;
result = makeNode(RelAggInfo);
+ pk_missing = false;
foreach(lc, target->exprs)
{
Index sortgroupref = 0;
SortGroupClause *cl;
Expr *texpr;
+ ListCell *lc2;
texpr = (Expr *) lfirst(lc);
@@ -2631,9 +2634,9 @@ create_rel_agg_info(PlannerInfo *root, RelOptInfo *rel)
/*
* Besides being an aggregate, the target expression should have no
- * other reason then being a column of a relation functionally
- * dependent on the GROUP BY clause. So it's not actually a grouping
- * column.
+ * other reason to be there than being a column of a relation
+ * functionally dependent on the GROUP BY clause. So it's not actually
+ * a grouping column.
*/
if (sortgroupref == 0)
continue;
@@ -2654,6 +2657,52 @@ create_rel_agg_info(PlannerInfo *root, RelOptInfo *rel)
*/
result->group_exprs = list_append_unique(result->group_exprs,
texpr);
+
+ /*
+ * Try to find PathKey for the expression, but don't if we already saw
+ * an expression w/o the PathKey.
+ */
+ if (pk_missing)
+ continue;
+
+ pk_found = false;
+ foreach(lc2, root->group_pathkeys)
+ {
+ PathKey *pkey = lfirst_node(PathKey, lc2);
+ EquivalenceClass *ec = pkey->pk_eclass;
+ ListCell *lc3;
+
+ foreach(lc3, ec->ec_members)
+ {
+ EquivalenceMember *em = lfirst_node(EquivalenceMember, lc3);
+
+ if (equal(texpr, em->em_expr))
+ {
+ result->group_pathkeys = lappend(result->group_pathkeys,
+ pkey);
+ pk_found = true;
+ break;
+ }
+ }
+ if (pk_found)
+ break;
+ }
+
+ /*
+ * If no PathKey was found, the expression was probably generated out
+ * of grp_exprs_extra. If we don't have a single PathKey,
+ * group_pathkeys is not useful, so clear it.
+ */
+ if (!pk_found)
+ {
+ list_free(result->group_pathkeys);
+ result->group_pathkeys = NIL;
+ /*
+ * Do not spend cycles looking for the PathKey for other
+ * expressions.
+ */
+ pk_missing = true;
+ }
}
/*
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index 89f944d83a..310c5ff774 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -389,6 +389,7 @@
#enable_seqscan = on
#enable_sort = on
#enable_tidscan = on
+#enable_agg_pushdown = on
# - Planner Cost Constants -
diff --git a/src/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.h
index 07459c423f..38e105b6de 100644
--- a/src/include/nodes/pathnodes.h
+++ b/src/include/nodes/pathnodes.h
@@ -1065,8 +1065,9 @@ typedef struct RelOptInfo
* actually just a workspace for users of the structure, i.e. not initialized
* when instance of the structure is created.
*
- * "group_clauses" and "group_exprs" are lists of SortGroupClause and the
- * corresponding grouping expressions respectively.
+ * "group_clauses", "group_exprs" and "group_pathkeys" are lists of
+ * SortGroupClause, the corresponding grouping expressions and PathKey
+ * respectively.
*
* "agg_exprs" is a list of Aggref nodes for the aggregation of the relation's
* paths.
@@ -1090,6 +1091,7 @@ typedef struct RelAggInfo
List *group_clauses;
List *group_exprs;
+ List *group_pathkeys;
List *agg_exprs; /* Aggref expressions. */
diff --git a/src/test/regress/expected/sysviews.out b/src/test/regress/expected/sysviews.out
index 442f7f9b41..da67f3a901 100644
--- a/src/test/regress/expected/sysviews.out
+++ b/src/test/regress/expected/sysviews.out
@@ -111,7 +111,7 @@ select count(*) = 0 as ok from pg_stat_wal_receiver;
select name, setting from pg_settings where name like 'enable%';
name | setting
--------------------------------+---------
- enable_agg_pushdown | off
+ enable_agg_pushdown | on
enable_async_append | on
enable_bitmapscan | on
enable_gathermerge | on
--=-=-=--
view thread (8+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected]
Subject: Re: [PATCH 3/3] Use also partial paths as the input for grouped paths.
In-Reply-To: <no-message-id-1859120@localhost>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox