-
Notifications
You must be signed in to change notification settings - Fork 24.9k
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
Refactor Geohash Grid Aggregation #14138
Refactor Geohash Grid Aggregation #14138
Conversation
} | ||
|
||
if (requiredSize == 0) { | ||
requiredSize = Integer.MAX_VALUE; |
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.
I was curious why this is set to Integer.MAX_VALUE
? Would we not want to reset it to DEFAULT_MAX_NUM_CELLS
?
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.
So we have a loose convention that size: 0
means get all buckets. I personally don't like it but we have to keep it for now I guess as this refactor should be backwards compatible
78878b3
to
7e316ba
Compare
throw new SearchParseException(context, "Unexpected token " + token + " in [" + aggregationName + "].", | ||
parser.getTokenLocation()); | ||
Integer size = (Integer) otherOptions.get(GeoHashGridParams.FIELD_SIZE); | ||
if (size != null) { |
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.
indentation looks wrong?
LGTM, just found some indentation issues |
No description provided.