[Transform] Optimize composite agg execution using ordered groupings #75424
Conversation
…togram group by comes first. The order is only changed for execution, the provided config remains unchanged
|
Pinging @elastic/ml-core (Team:ML) |
| } | ||
|
|
||
| // Arrays.sort provides stable sort (to respect the input order if priorities match), Collections.sort not | ||
| UnmodifiableEntryWithPriority[] prioritizedGroups = new UnmodifiableEntryWithPriority[groups.size()]; |
There was a problem hiding this comment.
UnmodifiableEntryWithPriority could be replaced with Tuple<Entry<String, SingleGroupSource>, Integer>
Then you could do Comparitor.comparing(Tuple::v2). The static class seems like a ton of unnecessary code. But this is a minor quibble.
There was a problem hiding this comment.
I had something like that in one of the 1st versions (before realizing the problem with sort), however generic types and generic arrays don't work together (I could suppress it, but don't like to).
There was a problem hiding this comment.
however generic types and generic arrays don't work together
Very true.
...sform/src/main/java/org/elasticsearch/xpack/transform/transforms/pivot/GroupByOptimizer.java
Outdated
Show resolved
Hide resolved
| prioritizedGroups[index++] = new UnmodifiableEntryWithPriority(groupBy, priority); | ||
| } | ||
|
|
||
| Arrays.sort(prioritizedGroups, (a, b) -> b.compareTo(a)); |
There was a problem hiding this comment.
Could Collections.reverseOrder() be used as a comparator here?
There was a problem hiding this comment.
I am not reversing the order, I want to re-order according to the priority I calculated from inspecting the group by definition. In case the priority equals the order of the config should remain that's why I need a stable sort, it respects the insertion order.
Collections.sort does not provide stable sort. But re-thinking about it, I could solve that by boosting the 1st group with groups.size(), the 2nd with groups.size() - 1 and so on.
…transform/transforms/pivot/GroupByOptimizer.java Co-authored-by: Przemysław Witek <przemyslaw.witek@elastic.co>
…hs/elasticsearch into transform-optimizeGroupByOrder
|
I simplified |
…lastic#75424) Automatically reorder group_by for composite aggs, ensuring date histogram group by comes first. The order is only changed for execution, the provided config remains unchanged. In case of 2 group_by's of the same order type, the configuration order is respected. Script and runtime field based group_by's are penalized.
Automatically reorder group_by for composite aggs, ensuring date histogram
group by comes first. The order is only changed for execution, the provided
config remains unchanged.
In case of 2 group_by's of the same order type, the configuration order is
respected. Script and runtime field based group_by's are penalized.