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
Sorting: Allow _geo_distance to handle many to many geo point distance #7097
Conversation
Add computation of disyance to many geo points. Example request: ``` { "sort": [ { "_geo_distance": { "location": [ { "lat":1.2, "lon":3 }, { "lat":1.2, "lon":3 } ], "order": "desc", "unit": "km", "sort_mode": "max" } } ] } ``` closes elastic#3926
XContentBuilder parserCopy = jsonBuilder(); | ||
parserCopy.copyCurrentStructure(parser); | ||
String dummyField = "{\"dummy_field\":" + parserCopy.string() + "}"; | ||
XContentParser copiedParser = XContentHelper.createParser(new BytesArray(dummyField)); |
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.
hm, is there a cleaner possibility?
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.
+1 on cleaning this up
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 changed this now in commit "clean up parsing: don't copy structure and parse twice".
The problem was that GeoUtils.parseGeoPoint(..)
expects the opening bracket [
in case the geo point is in the format [number, number]
but the moment the array content is passed to the parse method this bracket was already read. I now added a special case parsing for this format.
Let me know if this is cleaner.
instead implement geo point parsing of foramt [number, number] explicitely
Thanks for the quick feedback! I added commits for each comment. |
* 1 2 3 4 5 6 7 | ||
*/ | ||
assertAcked(prepareCreate("index") | ||
.addMapping("type", "{\"type\": {\"properties\": {\"location\": {\"type\": \"geo_point\"}}}}")); |
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.
or more simply:
.addMapping("type", "location", "type=geo_point")
LGTM Could you please use the json builder instead of inlining json in tests? I find it hard to read with all the escaping that is required for new lines, quotes, etc. (if only java had multi-line strings). If you only change that part, feel free to push without asking me for another review. |
pushed to master fe86c8b |
Add computation of disyance to many geo points. Example request:
closes #3926