Hi Henson, Thanks for the careful review and for the reproducers. Both cases are fixed in the attached v3, and the tests you suggested are added.
> 1. Partial aggregation below a join goes away.
The window turned out to be incidental. The same thing happens with v2
and no window at all:
SELECT a.i1, a.i2, sum(a.x)
FROM ea a JOIN eb b ON a.i1 = b.k
GROUP BY a.i1, a.i2
ORDER BY a.i1, a.i2 DESC;
Here the parser already gives the GROUP BY item i2 the ORDER BY's DESC,
so 0002 plays no part. The cause is in 0001: it let the final
aggregation group straight off the (i1, i2) index regardless of
direction, but generate_grouped_paths() still required its input to be
sorted exactly by the grouping pathkeys. The partial aggregation
therefore still had to sort its whole input, and lost to the plan
without it, which had just become cheaper. 0002 merely brings the
window queries into the same situation by giving i2 the window's
direction.
v3 lets the partial aggregation take the sort direction from its input
path as well. Like the final aggregation, it compares the grouping
keys only for equality, and its output keeps the ordering of its
input, so nothing else changes: agg_info->group_clauses stays as is,
and only the pathkeys used to decide whether and how to sort the input
are adjusted, by a new function adjust_group_pathkeys_to_input().
With that, your query does the partial aggregation off the index
again, and a single Incremental Sort by (i1, i2 DESC) above the Merge
Join, over the ~100k partially aggregated rows, serves both the
Finalize GroupAggregate and the WindowAgg (excerpt):
WindowAgg
Window: w1 AS (PARTITION BY a.i1 ORDER BY a.i2)
-> Finalize GroupAggregate
Group Key: a.i1, a.i2
-> Gather Merge
Workers Planned: 2
-> Incremental Sort
Sort Key: a.i1, a.i2 DESC
Presorted Key: a.i1
-> Merge Join
Merge Cond: (a.i1 = b.k)
-> Partial GroupAggregate
Group Key: a.i1, a.i2
-> Parallel Index Scan using ea_i1_i2_idx
on ea a
-> Sort
Sort Key: b.k
-> Seq Scan on eb b
Timings on your data set (5 runs each, same -O2 build type for all):
master v2 v3
window query 209-240 ms 235-271 ms 99-107 ms
ORDER BY query 183-193 ms 218-255 ms 104-114 ms
The results match master for both queries.
> 2. GROUP BY fails on an operator class without the reverse operator.
Agreed, skipping the plan is better. In v3,
group_keys_reorder_by_pathkeys() looks up the operator before adding
the key to the reordered list. If the operator family lacks it, the
function stops matching there, just as it does on any other mismatch,
and the GROUP BY keys from that point on are sorted in their own
direction. Your query now returns 1000 for each color.
Thanks again for testing all those combinations of outer ORDER BY,
joins, outer windows and LIMIT.
Regards,
Zhenglong Li
v3-0001-Ignore-sort-direction-when-matching-GROUP-BY-keys.patch
Description: Binary data
v3-0002-Consider-window-functions-when-reordering-GROUP-B.patch
Description: Binary data
