Geo: Allow numeric parameters enclosed in quotes for 'geohash_grid' aggregation #14440
Conversation
danielmitterdorfer
added
>bug
:Analytics/Geo
|
Can you please have a look @markharwood, @jpountz? The PR basically allows to quote parameters of the geohash_grid aggregation and enforces the documented range of [1,12] for precision as stated in the docs. You can find an example for reproduction in the corresponding ticket #13132. |
There was a problem hiding this comment.
I tend to find the '==false' way easier to read
There was a problem hiding this comment.
I am used to the !x notation but I am ok with reverting that.
|
Thanks @danielmitterdorfer I left some comments. You might want to coordinate with @colings86 on this since he has an open PR to refactor this aggregation to parse on the coordinating node (#14138). |
|
Thanks for the heads up on @colings86 work. I'll check with him. @jpountz: Can you look again at my changes? |
|
The PR looks good to me now, thanks! |
|
Ok, I'll check next with @colings86 on how to proceed regarding #14138. |
There was a problem hiding this comment.
Should we just use parser.isValue() here instead? I think we do this in other places that we are coercing the value into a number
There was a problem hiding this comment.
We cannot use #isValue() here because it would also return true on other types such as booleans which we want to exclude.
|
LGTM |
…ggregation Originally, only numeric values were allowed for parameters of the 'geohash_grid' aggregation in contrast to other places in the REST API. With this commit we also allow that parameters are enclosed in quotes (i.e. as JSON strings). Additionally, with this commit the valid range for 'precision' is enforced for the Java API and the REST API (the latter was previously missing the check). Closes elastic#13132
danielmitterdorfer
force-pushed
the
fix/13132
branch
from
5be23a0 to
7b62267
Compare
3 participants
Originally, only numeric values were allowed for parameters of the
'geohash_grid' aggregation in contrast to other places in the REST
API.
With this commit we also allow that parameters are enclosed in quotes (i.e.
as JSON strings). Additionally, with this commit the valid range for
'precision' is enforced for the Java API and the REST API (the latter was
previously missing the check).
Closes #13132