Skip to content

Commit d8a2953

Browse files
committed
Strip off ORDER BY/DISTINCT aggregate pathkeys in create_agg_path
1349d27 added code to adjust the PlannerInfo.group_pathkeys so that ORDER BY / DISTINCT aggregate functions could obtain pre-sorted inputs to allow faster execution. That commit forgot to adjust the pathkeys in create_agg_path(). Some code in there assumed that it was always fine to make the AggPath's pathkeys the same as its subpath's. That seems to have been ok up until 1349d27, but since that commit adds pathkeys for columns which are within the aggregate function, those columns won't be available above the aggregate node. This can result in "could not find pathkey item to sort" during create_plan(). The fix here is to strip off the additional pathkeys added by adjust_group_pathkeys_for_groupagg(). It seems that the pathkeys here will only ever be group_pathkeys, so all we need to do is check if the length of the pathkey list is longer than the num_groupby_pathkeys and get rid of the additional ones only if we see extras. Reported-by: Justin Pryzby Reviewed-by: Richard Guo Discussion: https://postgr.es/m/ZQhYYRhUxpW3PSf9%40telsasoft.com Backpatch-through: 16, where 1349d27 was introduced
1 parent 77db132 commit d8a2953

File tree

1 file changed

+17
-1
lines changed

1 file changed

+17
-1
lines changed

src/backend/optimizer/util/pathnode.c

+17-1
Original file line numberDiff line numberDiff line change
@@ -3133,10 +3133,26 @@ create_agg_path(PlannerInfo *root,
31333133
pathnode->path.parallel_safe = rel->consider_parallel &&
31343134
subpath->parallel_safe;
31353135
pathnode->path.parallel_workers = subpath->parallel_workers;
3136+
31363137
if (aggstrategy == AGG_SORTED)
3137-
pathnode->path.pathkeys = subpath->pathkeys; /* preserves order */
3138+
{
3139+
/*
3140+
* Attempt to preserve the order of the subpath. Additional pathkeys
3141+
* may have been added in adjust_group_pathkeys_for_groupagg() to
3142+
* support ORDER BY / DISTINCT aggregates. Pathkeys added there
3143+
* belong to columns within the aggregate function, so we must strip
3144+
* these additional pathkeys off as those columns are unavailable
3145+
* above the aggregate node.
3146+
*/
3147+
if (list_length(subpath->pathkeys) > root->num_groupby_pathkeys)
3148+
pathnode->path.pathkeys = list_copy_head(subpath->pathkeys,
3149+
root->num_groupby_pathkeys);
3150+
else
3151+
pathnode->path.pathkeys = subpath->pathkeys; /* preserves order */
3152+
}
31383153
else
31393154
pathnode->path.pathkeys = NIL; /* output is unordered */
3155+
31403156
pathnode->subpath = subpath;
31413157

31423158
pathnode->aggstrategy = aggstrategy;

0 commit comments

Comments
 (0)