Add parsing methods to Range aggregations #24583
Conversation
cbuescher
left a comment
cbuescher
left a comment
There was a problem hiding this comment.
Looks good, but I found two edge cases around the range to/from value that might not be covered by tests yet and that might not be working as expected yet. Not entirely sure, but maybe worth taking another close look.
There was a problem hiding this comment.
This should probably be initialized to Double.NEGATIVE_INFINITY. If InternalRange.from is infinite, the "from" field won't be rendered in the xContent and I think the current parser would leave from == 0 in this case. Maybe we should also add tests for those edge cases.
There was a problem hiding this comment.
That's right, thanks! I added some random ranges with infinity values to the test.
There was a problem hiding this comment.
Same here but with Double.POSITIVE_INFINITY?
tlrx
force-pushed
the
add-parsing-for-range-aggs
branch
from
0879dbf to
13c2d12
Compare
|
@cbuescher I updated the code. Can you please have another look? Thanks |
|
Thanks @cbuescher @javanna |
3 participants
This pull request adds the parsing logic to InternalRange, InternalDateRange and InternalGeoDistance aggregations.