Skip to content
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

Closed
wants to merge 6 commits into from

Conversation

Projects
None yet
4 participants
@brwe
Copy link
Contributor

brwe commented Jul 30, 2014

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 #3926

_geo_distance sort: allow many to many geo point distance
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 #3926
XContentBuilder parserCopy = jsonBuilder();
parserCopy.copyCurrentStructure(parser);
String dummyField = "{\"dummy_field\":" + parserCopy.string() + "}";
XContentParser copiedParser = XContentHelper.createParser(new BytesArray(dummyField));

This comment has been minimized.

Copy link
@spinscale

spinscale Jul 31, 2014

Member

hm, is there a cleaner possibility?

This comment has been minimized.

Copy link
@jpountz

jpountz Jul 31, 2014

Contributor

+1 on cleaning this up

This comment has been minimized.

Copy link
@brwe

brwe Jul 31, 2014

Author Contributor

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.

* |___________________________
* 1 2 3 4 5 6 7
*/
assertAcked(prepareCreate("index")

This comment has been minimized.

Copy link
@spinscale
* @param points reference points.
*/
public GeoDistanceSortBuilder points(GeoPoint... points) {
this.points.addAll(Lists.newArrayList(points));

This comment has been minimized.

Copy link
@jpountz

jpountz Jul 31, 2014

Contributor

Very minor thing, but this.points.addAll(Arrays.asList(points)); would help copying the data only once instead of twice.

This comment has been minimized.

Copy link
@brwe

brwe Jul 31, 2014

Author Contributor

ok, changed also in geohash(String... geohashes)

builder.field(fieldName, points.get(0));
} else {
builder.field(fieldName, points);
}
}

This comment has been minimized.

Copy link
@jpountz

jpountz Jul 31, 2014

Contributor

Now that it supports several points, should it also work when both geo hashes and geo points are provided?

This comment has been minimized.

Copy link
@brwe

brwe Jul 31, 2014

Author Contributor

makes sense, changed that in "make sort work with arbitrary mixed geo_points formats"

brwe added some commits Jul 31, 2014

clean up parsing: don't copy structure and parse twice
instead implement geo point parsing of foramt [number, number] explicitely
@brwe

This comment has been minimized.

Copy link
Contributor Author

brwe commented Jul 31, 2014

Thanks for the quick feedback! I added commits for each comment.

@brwe brwe added the review label Jul 31, 2014

* 1 2 3 4 5 6 7
*/
assertAcked(prepareCreate("index")
.addMapping("type", "{\"type\": {\"properties\": {\"location\": {\"type\": \"geo_point\"}}}}"));

This comment has been minimized.

Copy link
@jpountz

jpountz Jul 31, 2014

Contributor

or more simply:

.addMapping("type", "location", "type=geo_point")
@jpountz

This comment has been minimized.

Copy link
Contributor

jpountz commented Jul 31, 2014

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.

@jpountz jpountz removed the review label Jul 31, 2014

@brwe

This comment has been minimized.

Copy link
Contributor Author

brwe commented Jul 31, 2014

pushed to master fe86c8b

@brwe brwe closed this Jul 31, 2014

@clintongormley clintongormley changed the title _geo_distance sort: allow many to many geo point distance Sorting: Allow _geo_distance to handle many to many geo point distance Sep 10, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.