implement group by for single-column aggregates #144
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Implements most of the functionality for the capability
query.aggregates.group_by. There are still a couple of things to follow up on.Counts are not implemented for group by queries yet. I'll follow up on those in ENG-1568.
There is a bug involving multiple references to the same relationship that should be resolved. I'll follow up in ENG-1569.
While working on this I removed the custom "count" aggregation - it is redundant, and I've been meaning to do that for a while. Users can use the standard count aggregations instead.
There is a change in here that explicitly converts aggregate result values for "average" and "sum" aggregations to the result types declared in the schema. This is necessary to avoid errors in response serialization for groups when aggregating over 128-bit decimal values. I applied the same type conversion for group and for root aggregates for consistency. This does mean there will be some loss of precision in those cases. But it also means we won't get back a JSON string in some cases, and a JSON number in others.