-
Notifications
You must be signed in to change notification settings - Fork 25.2k
Aggregations: Adds ability to sort on multiple criteria #7588
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@@ -239,6 +306,13 @@ public static InternalOrder readOrder(StreamInput in) throws IOException { | |||
return new InternalOrder.Aggregation(key + "." + in.readString(), asc); | |||
} | |||
return new InternalOrder.Aggregation(key, asc); | |||
case -1: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you use CompoundOrder.ID instead of -1?
@jpountz Didn't realise that you hadn't removed the review tag but have just pushed an update from your feedback. Sorry if that messes things up |
public static class CompoundOrderComparator implements Comparator<Terms.Bucket> { | ||
|
||
private List<Terms.Order> compoundOrder; | ||
private Aggregator aggregator; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you make them final?
LGTM |
The terms aggregation can now support sorting on multiple criteria by replacing the sort object with an array or sort object whose order signifies the priority of the sort. The existing syntax for sorting on a single criteria also still works. Contributes to #6917
These changes have been reverted as backward compatibility issues were found. A new PR will be raised when the issues have been resolved |
The terms aggregation can now support sorting on multiple criteria by replacing the sort object with an array or sort object whose order signifies the priority of the sort. The existing syntax for sorting on a single criteria also still works.
Contributes to #6917