On Thu, May 28, 2020 at 06:14:55PM -0700, Jeff Davis wrote:
On Thu, 2020-05-28 at 20:57 +0200, Tomas Vondra wrote:
Attached is a patch adding CP_SMALL_TLIST to create_agg_plan and
create_groupingsets_plan.
Looks good, except one question:
Why would aggstrategy ever be MIXED when in create_agg_path? Wouldn't
that only happen in create_groupingsets_path?
Ah, right. Yeah, we only need to check for AGG_HASH here. Moreover,
AGG_MIXED probably does not need the tlist tweak, because the input
should be pre-sorted as with AGG_SORTED.
And we should probably do similar check in the create_groupinsets_path,
I guess. At first I thought we can't do that before inspecting rollups,
which only happens later in the function, but now I see there's
aggstrategy in GroupingSetsPath too.
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
diff --git a/src/backend/optimizer/plan/createplan.c
b/src/backend/optimizer/plan/createplan.c
index 9941dfe65e..eab57fd74b 100644
--- a/src/backend/optimizer/plan/createplan.c
+++ b/src/backend/optimizer/plan/createplan.c
@@ -2113,12 +2113,21 @@ create_agg_plan(PlannerInfo *root, AggPath *best_path)
Plan *subplan;
List *tlist;
List *quals;
+ int flags;
/*
* Agg can project, so no need to be terribly picky about child tlist,
but
- * we do need grouping columns to be available
+ * we do need grouping columns to be available. We are a bit more
careful
+ * with hash aggregate, where we explicitly request small tlist to
minimize
+ * I/O needed for spilling (possibly not known already).
*/
- subplan = create_plan_recurse(root, best_path->subpath, CP_LABEL_TLIST);
+ flags = CP_LABEL_TLIST;
+
+ /* ensure small tlist for hash aggregate */
+ if (best_path->aggstrategy == AGG_HASHED)
+ flags |= CP_SMALL_TLIST;
+
+ subplan = create_plan_recurse(root, best_path->subpath, flags);
tlist = build_path_tlist(root, &best_path->path);
@@ -2200,6 +2209,7 @@ create_groupingsets_plan(PlannerInfo *root,
GroupingSetsPath *best_path)
int maxref;
List *chain;
ListCell *lc;
+ int flags;
/* Shouldn't get here without grouping sets */
Assert(root->parse->groupingSets);
@@ -2207,9 +2217,17 @@ create_groupingsets_plan(PlannerInfo *root,
GroupingSetsPath *best_path)
/*
* Agg can project, so no need to be terribly picky about child tlist,
but
- * we do need grouping columns to be available
+ * we do need grouping columns to be available. We are a bit more
careful
+ * with hash aggregate, where we explicitly request small tlist to
minimize
+ * I/O needed for spilling (possibly not known already).
*/
- subplan = create_plan_recurse(root, best_path->subpath, CP_LABEL_TLIST);
+ flags = CP_LABEL_TLIST;
+
+ /* ensure small tlist for hash aggregate */
+ if (best_path->aggstrategy == AGG_HASHED)
+ flags |= CP_SMALL_TLIST;
+
+ subplan = create_plan_recurse(root, best_path->subpath, flags);
/*
* Compute the mapping from tleSortGroupRef to column index in the
child's
