From 6d3b749c164934d6a0a50436f6c7c164088e3f8c Mon Sep 17 00:00:00 2001 From: Richard Guo Date: Tue, 19 Sep 2023 17:51:06 +0800 Subject: [PATCH v1] Include ordered aggregates' arguments in targetList In 1349d2790b we add pathkeys in ordered aggregates to root->group_pathkeys so that the executor does not need to perform a sort for these aggregates. With this change we also need to add the aggregates' arguments that are ORDER BY expressions to parse->targetList, otherwise prepare_sort_from_pathkeys would have problem computing sort column info. --- src/backend/optimizer/plan/planner.c | 33 +++++++++++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c index 44efb1f4eb..042151830b 100644 --- a/src/backend/optimizer/plan/planner.c +++ b/src/backend/optimizer/plan/planner.c @@ -3275,7 +3275,7 @@ adjust_group_pathkeys_for_groupagg(PlannerInfo *root) * Now process all the unprocessed_aggs to find the best set of pathkeys * for the given set of aggregates. * - * On the first outer loop here 'bestaggs' will be empty. We'll populate + * On the first outer loop here 'bestaggs' will be empty. We'll populate * this during the first loop using the pathkeys for the very first * AggInfo then taking any stronger pathkeys from any other AggInfos with * a more strict set of compatible pathkeys. Once the outer loop is @@ -3396,6 +3396,11 @@ adjust_group_pathkeys_for_groupagg(PlannerInfo *root) * there's no chance of a Hash Aggregate plan as create_grouping_paths * will not mark the GROUP BY as GROUPING_CAN_USE_HASH due to the presence * of ordered aggregates. + * + * We also need to add resjunk target entries to the targetList for the + * aggregates' arguments that are ORDER BY expressions, otherwise + * prepare_sort_from_pathkeys would have problem computing sort column + * info. */ i = -1; while ((i = bms_next_member(bestaggs, i)) >= 0) @@ -3405,8 +3410,34 @@ adjust_group_pathkeys_for_groupagg(PlannerInfo *root) foreach(lc, agginfo->aggrefs) { Aggref *aggref = lfirst_node(Aggref, lc); + ListCell *l; aggref->aggpresorted = true; + + foreach(l, aggref->args) + { + TargetEntry *aggtle = lfirst_node(TargetEntry, l); + TargetEntry *tle; + + /* + * No need to consider arguments that are not ORDER BY + * expressions. + */ + if (aggtle->ressortgroupref == 0) + continue; + + if (tlist_member(aggtle->expr, root->parse->targetList)) + continue; /* already got it */ + + /* + * Add resjunk entry to targetList. + */ + tle = makeTargetEntry(copyObject(aggtle->expr), + list_length(root->parse->targetList) + 1, + NULL, + true); + root->parse->targetList = lappend(root->parse->targetList, tle); + } } } } -- 2.31.0