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.

Sign up for GitHub

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

Jump to bottom

Sorting: Allow _geo_distance to handle many to many geo point distance #7097

Closed
wants to merge 6 commits into from
Closed

Sorting: Allow _geo_distance to handle many to many geo point distance #7097

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
172d832
_geo_distance sort: allow many to many geo point distance
brwe Jul 30, 2014
38baefb
clean up parsing: don't copy structure and parse twice
brwe Jul 31, 2014
79c6c3a
Lists.newArrayList(points) -> Arrays.asList(points)
brwe Jul 31, 2014
c221f92
make sort work with arbitrary mixed geo_points formats
brwe Jul 31, 2014
ded2a80
cleanuo imports and naming consistency
brwe Jul 31, 2014
1287ed8
strings to jsonBuilder and shorter add mapping
brwe Jul 31, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 16 additions & 0 deletions docs/reference/search/request/sort.asciidoc
View file
Open in desktop
Expand Up @@ -263,6 +263,22 @@ conform with http://geojson.org/[GeoJSON].
}
--------------------------------------------------


==== Multiple reference points

Multiple geo points can be passed as an array containing any `geo_point` format, for example

```
"pin.location" : [[-70, 40], [-71, 42]]
"pin.location" : [{"lat": -70, "lon": 40}, {"lat": -71, "lon": 42}]

```
and so forth.

The final distance for a document will then be `min`/`max` distance of all points contained in the document to all points given in the sort request.



==== Script Based Sorting

Allow to sort based on custom scripts, here is an example:
Expand Down
23 changes: 14 additions & 9 deletions src/main/java/org/elasticsearch/common/geo/GeoDistance.java
View file
Open in desktop
Expand Up @@ -25,6 +25,7 @@
import org.elasticsearch.common.unit.DistanceUnit;
import org.elasticsearch.index.fielddata.*;

import java.util.List;
import java.util.Locale;

/**
Expand Down Expand Up @@ -371,12 +372,13 @@ public double calculate(double targetLatitude, double targetLongitude) {
}
}


/**
* Return a {@link SortedNumericDoubleValues} instance that returns the distance to a given geo-point for each document.
* Return a {@link SortedNumericDoubleValues} instance that returns the distances to a list of geo-points for each document.
*/
public static SortedNumericDoubleValues distanceValues(final FixedSourceDistance distance, final MultiGeoPointValues geoPointValues) {
public static SortedNumericDoubleValues distanceValues(final MultiGeoPointValues geoPointValues, final FixedSourceDistance... distances) {
final GeoPointValues singleValues = FieldData.unwrapSingleton(geoPointValues);
if (singleValues != null) {
if (singleValues != null && distances.length == 1) {
final Bits docsWithField = FieldData.unwrapSingletonBits(geoPointValues);
return FieldData.singleton(new NumericDoubleValues() {

Expand All @@ -386,7 +388,7 @@ public double get(int docID) {
return 0d;
}
final GeoPoint point = singleValues.get(docID);
return distance.calculate(point.lat(), point.lon());
return distances[0].calculate(point.lat(), point.lon());
}

}, docsWithField);
Expand All @@ -396,15 +398,18 @@ public double get(int docID) {
@Override
public void setDocument(int doc) {
geoPointValues.setDocument(doc);
count = geoPointValues.count();
count = geoPointValues.count() * distances.length;
grow();
for (int i = 0; i < count; ++i) {
final GeoPoint point = geoPointValues.valueAt(i);
values[i] = distance.calculate(point.lat(), point.lon());
int valueCounter = 0;
for (FixedSourceDistance distance : distances) {
for (int i = 0; i < geoPointValues.count(); ++i) {
final GeoPoint point = geoPointValues.valueAt(i);
values[valueCounter] = distance.calculate(point.lat(), point.lon());
valueCounter++;
}
}
sort();
}

};
}
}
Expand Down
2 changes: 1 addition & 1 deletion ...ava/org/elasticsearch/search/aggregations/bucket/range/geodistance/GeoDistanceParser.java
View file
Open in desktop
Expand Up @@ -209,7 +209,7 @@ public DistanceSource(ValuesSource.GeoPoint source, GeoDistance distanceType, or
public void setNextReader(AtomicReaderContext reader) {
final MultiGeoPointValues geoValues = source.geoPointValues();
final FixedSourceDistance distance = distanceType.fixedSourceDistance(origin.getLat(), origin.getLon(), unit);
distanceValues = GeoDistance.distanceValues(distance, geoValues);
distanceValues = GeoDistance.distanceValues(geoValues, distance);
}

@Override
Expand Down
41 changes: 30 additions & 11 deletions src/main/java/org/elasticsearch/search/sort/GeoDistanceSortBuilder.java
View file
Open in desktop
Expand Up @@ -19,12 +19,16 @@

package org.elasticsearch.search.sort;

import com.google.common.collect.Lists;
import org.elasticsearch.common.geo.GeoDistance;
import org.elasticsearch.common.geo.GeoPoint;
import org.elasticsearch.common.unit.DistanceUnit;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.index.query.FilterBuilder;

import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
import java.util.Locale;

/**
Expand All @@ -33,10 +37,8 @@
public class GeoDistanceSortBuilder extends SortBuilder {

final String fieldName;

private double lat;
private double lon;
private String geohash;
private final List<GeoPoint> points = new ArrayList<>();
private final List<String> geohashes = new ArrayList<>();

private GeoDistance geoDistance;
private DistanceUnit unit;
Expand All @@ -61,16 +63,25 @@ public GeoDistanceSortBuilder(String fieldName) {
* @param lon longitude.
*/
public GeoDistanceSortBuilder point(double lat, double lon) {
this.lat = lat;
this.lon = lon;
points.add(new GeoPoint(lat, lon));
return this;
}

/**
* The point to create the range distance facets from.
*
* @param points reference points.
*/
public GeoDistanceSortBuilder points(GeoPoint... points) {
this.points.addAll(Lists.newArrayList(points));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

return this;
}

/**
* The geohash of the geo point to create the range distance facets from.
*/
public GeoDistanceSortBuilder geohash(String geohash) {
this.geohash = geohash;
public GeoDistanceSortBuilder geohash(String... geohash) {
this.geohashes.addAll(Lists.newArrayList(geohash));
return this;
}

Expand Down Expand Up @@ -138,10 +149,18 @@ public GeoDistanceSortBuilder setNestedPath(String nestedPath) {
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
builder.startObject("_geo_distance");

if (geohash != null) {
builder.field(fieldName, geohash);
if (geohashes.size() != 0) {
if (geohashes.size() == 1) {
builder.field(fieldName, geohashes.get(0));
} else {
builder.field(fieldName, geohashes);
}
} else {
builder.startArray(fieldName).value(lon).value(lat).endArray();
if (points.size() == 1) {
builder.field(fieldName, points.get(0));
} else {
builder.field(fieldName, points);
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


if (unit != null) {
Expand Down
55 changes: 49 additions & 6 deletions src/main/java/org/elasticsearch/search/sort/GeoDistanceSortParser.java
View file
Open in desktop
Expand Up @@ -26,12 +26,14 @@
import org.apache.lucene.search.SortField;
import org.apache.lucene.util.FixedBitSet;
import org.elasticsearch.ElasticsearchIllegalArgumentException;
import org.elasticsearch.ElasticsearchParseException;
import org.elasticsearch.common.bytes.BytesArray;
import org.elasticsearch.common.geo.GeoDistance;
import org.elasticsearch.common.geo.GeoDistance.FixedSourceDistance;
import org.elasticsearch.common.geo.GeoPoint;
import org.elasticsearch.common.geo.GeoUtils;
import org.elasticsearch.common.unit.DistanceUnit;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.common.xcontent.*;
import org.elasticsearch.index.fielddata.*;
import org.elasticsearch.index.fielddata.IndexFieldData.XFieldComparatorSource.Nested;
import org.elasticsearch.index.mapper.FieldMapper;
Expand All @@ -43,6 +45,10 @@
import org.elasticsearch.search.internal.SearchContext;

import java.io.IOException;
import java.util.ArrayList;
import java.util.List;

import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;

/**
*
Expand All @@ -57,7 +63,7 @@ public String[] names() {
@Override
public SortField parse(XContentParser parser, SearchContext context) throws Exception {
String fieldName = null;
GeoPoint point = new GeoPoint();
List<GeoPoint> geoPoints = new ArrayList<>();
DistanceUnit unit = DistanceUnit.DEFAULT;
GeoDistance geoDistance = GeoDistance.DEFAULT;
boolean reverse = false;
Expand All @@ -74,7 +80,8 @@ public SortField parse(XContentParser parser, SearchContext context) throws Exce
if (token == XContentParser.Token.FIELD_NAME) {
currentName = parser.currentName();
} else if (token == XContentParser.Token.START_ARRAY) {
GeoUtils.parseGeoPoint(parser, point);
parseGeoPoints(parser, geoPoints);

fieldName = currentName;
} else if (token == XContentParser.Token.START_OBJECT) {
// the json in the format of -> field : { lat : 30, lon : 12 }
Expand All @@ -83,7 +90,9 @@ public SortField parse(XContentParser parser, SearchContext context) throws Exce
nestedFilter = parsedFilter == null ? null : parsedFilter.filter();
} else {
fieldName = currentName;
GeoPoint point = new GeoPoint();
GeoUtils.parseGeoPoint(parser, point);
geoPoints.add(point);
}
} else if (token.isValue()) {
if ("reverse".equals(currentName)) {
Expand All @@ -102,14 +111,18 @@ public SortField parse(XContentParser parser, SearchContext context) throws Exce
} else if ("nested_path".equals(currentName) || "nestedPath".equals(currentName)) {
nestedPath = parser.text();
} else {
GeoPoint point = new GeoPoint();
point.resetFromString(parser.text());
geoPoints.add(point);
fieldName = currentName;
}
}
}

if (normalizeLat || normalizeLon) {
GeoUtils.normalizePoint(point, normalizeLat, normalizeLon);
for (GeoPoint point : geoPoints) {
GeoUtils.normalizePoint(point, normalizeLat, normalizeLon);
}
}

if (sortMode == null) {
Expand All @@ -126,7 +139,10 @@ public SortField parse(XContentParser parser, SearchContext context) throws Exce
}
final MultiValueMode finalSortMode = sortMode; // final reference for use in the anonymous class
final IndexGeoPointFieldData geoIndexFieldData = context.fieldData().getForField(mapper);
final FixedSourceDistance distance = geoDistance.fixedSourceDistance(point.lat(), point.lon(), unit);
final FixedSourceDistance[] distances = new FixedSourceDistance[geoPoints.size()];
for (int i = 0; i< geoPoints.size(); i++) {
distances[i] = geoDistance.fixedSourceDistance(geoPoints.get(i).lat(), geoPoints.get(i).lon(), unit);
}
ObjectMapper objectMapper;
if (nestedPath != null) {
ObjectMappers objectMappers = context.mapperService().objectMapper(nestedPath);
Expand Down Expand Up @@ -167,7 +183,7 @@ public FieldComparator<?> newComparator(String fieldname, int numHits, int sortP
@Override
protected Doubles getDoubleValues(AtomicReaderContext context, String field) throws IOException {
final MultiGeoPointValues geoPointValues = geoIndexFieldData.load(context).getGeoPointValues();
final SortedNumericDoubleValues distanceValues = GeoDistance.distanceValues(distance, geoPointValues);
final SortedNumericDoubleValues distanceValues = GeoDistance.distanceValues(geoPointValues, distances);
final NumericDoubleValues selectedValues;
if (nested == null) {
selectedValues = finalSortMode.select(distanceValues, Double.MAX_VALUE);
Expand All @@ -190,4 +206,31 @@ public double get(int docID) {

return new SortField(fieldName, geoDistanceComparatorSource, reverse);
}

private void parseGeoPoints(XContentParser parser, List<GeoPoint> geoPoints) throws IOException {

XContentBuilder parserCopy = jsonBuilder();
parserCopy.copyCurrentStructure(parser);
String dummyField = "{\"dummy_field\":" + parserCopy.string() + "}";
XContentParser copiedParser = XContentHelper.createParser(new BytesArray(dummyField));
Copy link
Contributor

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?

Copy link
Contributor

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

Copy link
Contributor Author

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.

try {
copiedParser.nextToken();
copiedParser.nextToken();
copiedParser.nextToken();
while (!copiedParser.nextToken().equals(XContentParser.Token.END_ARRAY)) {
GeoPoint point = new GeoPoint();
GeoUtils.parseGeoPoint(copiedParser, point);
geoPoints.add(point);
}
} catch (ElasticsearchParseException e) {
// it might be that the input was [number, number] in which case we must try to parse again
copiedParser = XContentHelper.createParser(new BytesArray(dummyField));
GeoPoint point = new GeoPoint();
copiedParser.nextToken();
copiedParser.nextToken();
copiedParser.nextToken();
GeoUtils.parseGeoPoint(copiedParser, point);
geoPoints.add(point);
}
}
}
68 changes: 68 additions & 0 deletions src/test/java/org/elasticsearch/search/sort/SimpleSortTests.java
View file
Open in desktop
Expand Up @@ -30,8 +30,11 @@
import org.elasticsearch.action.search.SearchResponse;
import org.elasticsearch.action.search.ShardSearchFailure;
import org.elasticsearch.cluster.metadata.IndexMetaData;
import org.elasticsearch.common.geo.GeoDistance;
import org.elasticsearch.common.geo.GeoPoint;
import org.elasticsearch.common.text.StringAndBytesText;
import org.elasticsearch.common.text.Text;
import org.elasticsearch.common.unit.DistanceUnit;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentFactory;
import org.elasticsearch.index.mapper.Uid;
Expand Down Expand Up @@ -1675,5 +1678,70 @@ public void testSortDuelBetweenSingleShardAndMultiShardIndex() throws Exception
}
}

public void testManyToManyGeoPoints() throws ExecutionException, InterruptedException {
/**
* | q | d1 | d2
* | | |
* | | |
* | | |
* |2 o| x | x
* | | |
* |1 o| x | x
* |___________________________
* 1 2 3 4 5 6 7
*/
assertAcked(prepareCreate("index")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:-)

.addMapping("type", "{\"type\": {\"properties\": {\"location\": {\"type\": \"geo_point\"}}}}"));
Copy link
Contributor

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")

String d1 = randomBoolean() ?
"{\"location\": [{\"lat\": \"3\", \"lon\": \"2\"}, {\"lat\": \"4\", \"lon\": \"1\"}]}" :
"{\"location\": [{\"lat\": \"4\", \"lon\": \"1\"}, {\"lat\": \"3\", \"lon\": \"2\"}]}";
String d2 = randomBoolean() ?
"{\"location\": [{\"lat\": \"5\", \"lon\": \"1\"}, {\"lat\": \"6\", \"lon\": \"2\"}]}" :
"{\"location\": [{\"lat\": \"6\", \"lon\": \"2\"}, {\"lat\": \"5\", \"lon\": \"1\"}]}";
indexRandom(true,
client().prepareIndex("index", "type", "d1").setSource(d1),
client().prepareIndex("index", "type", "d2").setSource(d2));
ensureYellow();
GeoPoint[] q = new GeoPoint[2];
if (randomBoolean()) {
q[0] = new GeoPoint(2, 1);
q[1] = new GeoPoint(2, 2);
} else {
q[1] = new GeoPoint(2, 2);
q[0] = new GeoPoint(2, 1);
}

SearchResponse searchResponse = client().prepareSearch()
.setQuery(matchAllQuery())
.addSort(new GeoDistanceSortBuilder("location").points(q).sortMode("min").order(SortOrder.ASC).geoDistance(GeoDistance.PLANE).unit(DistanceUnit.KILOMETERS))
.execute().actionGet();
assertOrderedSearchHits(searchResponse, "d1", "d2");
assertThat((Double) searchResponse.getHits().getAt(0).getSortValues()[0], equalTo(GeoDistance.PLANE.calculate(2, 2, 3, 2, DistanceUnit.KILOMETERS)));
assertThat((Double) searchResponse.getHits().getAt(1).getSortValues()[0], equalTo(GeoDistance.PLANE.calculate(2, 1, 5, 1, DistanceUnit.KILOMETERS)));

searchResponse = client().prepareSearch()
.setQuery(matchAllQuery())
.addSort(new GeoDistanceSortBuilder("location").points(q).sortMode("min").order(SortOrder.DESC).geoDistance(GeoDistance.PLANE).unit(DistanceUnit.KILOMETERS))
.execute().actionGet();
assertOrderedSearchHits(searchResponse, "d2", "d1");
assertThat((Double) searchResponse.getHits().getAt(0).getSortValues()[0], equalTo(GeoDistance.PLANE.calculate(2, 1, 5, 1, DistanceUnit.KILOMETERS)));
assertThat((Double) searchResponse.getHits().getAt(1).getSortValues()[0], equalTo(GeoDistance.PLANE.calculate(2, 2, 3, 2, DistanceUnit.KILOMETERS)));

searchResponse = client().prepareSearch()
.setQuery(matchAllQuery())
.addSort(new GeoDistanceSortBuilder("location").points(q).sortMode("max").order(SortOrder.ASC).geoDistance(GeoDistance.PLANE).unit(DistanceUnit.KILOMETERS))
.execute().actionGet();
assertOrderedSearchHits(searchResponse, "d1", "d2");
assertThat((Double) searchResponse.getHits().getAt(0).getSortValues()[0], equalTo(GeoDistance.PLANE.calculate(2, 2, 4, 1, DistanceUnit.KILOMETERS)));
assertThat((Double) searchResponse.getHits().getAt(1).getSortValues()[0], equalTo(GeoDistance.PLANE.calculate(2, 1, 6, 2, DistanceUnit.KILOMETERS)));

searchResponse = client().prepareSearch()
.setQuery(matchAllQuery())
.addSort(new GeoDistanceSortBuilder("location").points(q).sortMode("max").order(SortOrder.DESC).geoDistance(GeoDistance.PLANE).unit(DistanceUnit.KILOMETERS))
.execute().actionGet();
assertOrderedSearchHits(searchResponse, "d2", "d1");
assertThat((Double) searchResponse.getHits().getAt(0).getSortValues()[0], equalTo(GeoDistance.PLANE.calculate(2, 1, 6, 2, DistanceUnit.KILOMETERS)));
assertThat((Double) searchResponse.getHits().getAt(1).getSortValues()[0], equalTo(GeoDistance.PLANE.calculate(2, 2, 4, 1, DistanceUnit.KILOMETERS)));
}

}