From 84fa9ead4d19aceed4540ccd44c06c37fd629da2 Mon Sep 17 00:00:00 2001 From: Florian Schilling Date: Mon, 24 Jun 2013 18:35:12 +0200 Subject: [PATCH] The `geohash_cell` filter now adapts the format of other geo-filters. The oject fieldnames match the fieldnames document names automatically. This invalidates the `field` field in previeous versions. The value these fields value is a `geo_point` value (all formats supported) which is internally translated to a geohash. Since those points alway have a maximum precision (level 12) a `precision` definition has been included. This precision can either be defined as *length* of the geohash-string or as *distance*. It's assumed the a distance without any unit is a geohash-length. ``` GET 'http://127.0.0.1:9200/locations/_search?pretty=true' -d '{ "query": { "match_all":{} }, "filter": { "geohash_cell": { "pin": { "lat": 13.4080, "lon": 52.5186 }, "precision": 3, "neighbors": true } } }' ``` Closes #3229 --- .../common/geo/GeoHashUtils.java | 7 +- .../elasticsearch/common/geo/GeoPoint.java | 98 +++++++++++++++++++ .../index/query/FilterBuilders.java | 13 +++ .../index/query/GeohashFilter.java | 78 ++++++++++++--- .../search/geo/GeoFilterTests.java | 35 +++++-- 5 files changed, 210 insertions(+), 21 deletions(-) diff --git a/src/main/java/org/elasticsearch/common/geo/GeoHashUtils.java b/src/main/java/org/elasticsearch/common/geo/GeoHashUtils.java index 534b8c00566..21eb2501749 100644 --- a/src/main/java/org/elasticsearch/common/geo/GeoHashUtils.java +++ b/src/main/java/org/elasticsearch/common/geo/GeoHashUtils.java @@ -170,7 +170,12 @@ public class GeoHashUtils { if (nx >= 0 && nx <= xLimit && ny >= 0 && ny < yLimit) { return geohash.substring(0, level - 1) + encode(nx, ny); } else { - return neighbor(geohash, level - 1, dx, dy) + encode(nx, ny); + String neighbor = neighbor(geohash, level - 1, dx, dy); + if(neighbor != null) { + return neighbor + encode(nx, ny); + } else { + return null; + } } } } diff --git a/src/main/java/org/elasticsearch/common/geo/GeoPoint.java b/src/main/java/org/elasticsearch/common/geo/GeoPoint.java index 37d79150842..a95c0c71081 100644 --- a/src/main/java/org/elasticsearch/common/geo/GeoPoint.java +++ b/src/main/java/org/elasticsearch/common/geo/GeoPoint.java @@ -19,11 +19,21 @@ package org.elasticsearch.common.geo; +import java.io.IOException; + +import org.elasticsearch.ElasticSearchParseException; +import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.common.xcontent.XContentParser.Token; +import org.elasticsearch.index.mapper.geo.GeoPointFieldMapper; + /** * */ public class GeoPoint { + public static final String LATITUDE = GeoPointFieldMapper.Names.LAT; + public static final String LONGITUDE = GeoPointFieldMapper.Names.LON; + private double lat; private double lon; @@ -123,4 +133,92 @@ public class GeoPoint { public String toString() { return "[" + lat + ", " + lon + "]"; } + + /** + * Parse a {@link GeoPoint} with a {@link XContentParser}: + * + * @param parser {@link XContentParser} to parse the value from + * @return new {@link GeoPoint} parsed from the parse + * + * @throws IOException + * @throws ElasticSearchParseException + */ + public static GeoPoint parse(XContentParser parser) throws IOException, ElasticSearchParseException { + return parse(parser, new GeoPoint()); + } + + /** + * Parse a {@link GeoPoint} with a {@link XContentParser}. A geopoint has one of the following forms: + * + * + * + * @param parser {@link XContentParser} to parse the value from + * @param point A {@link GeoPoint} that will be reset by the values parsed + * @return new {@link GeoPoint} parsed from the parse + * + * @throws IOException + * @throws ElasticSearchParseException + */ + public static GeoPoint parse(XContentParser parser, GeoPoint point) throws IOException, ElasticSearchParseException { + if(parser.currentToken() == Token.START_OBJECT) { + while(parser.nextToken() != Token.END_OBJECT) { + if(parser.currentToken() == Token.FIELD_NAME) { + String field = parser.text(); + if(LATITUDE.equals(field)) { + if(parser.nextToken() == Token.VALUE_NUMBER) { + point.resetLat(parser.doubleValue()); + } else { + throw new ElasticSearchParseException("latitude must be a number"); + } + } else if (LONGITUDE.equals(field)) { + if(parser.nextToken() == Token.VALUE_NUMBER) { + point.resetLon(parser.doubleValue()); + } else { + throw new ElasticSearchParseException("latitude must be a number"); + } + } else { + throw new ElasticSearchParseException("field must be either '"+LATITUDE+"' or '"+LONGITUDE+"'"); + } + } else { + throw new ElasticSearchParseException("Token '"+parser.currentToken()+"' not allowed"); + } + } + return point; + } else if(parser.currentToken() == Token.START_ARRAY) { + int element = 0; + while(parser.nextToken() != Token.END_ARRAY) { + if(parser.currentToken() == Token.VALUE_NUMBER) { + element++; + if(element == 1) { + point.resetLon(parser.doubleValue()); + } else if(element == 2) { + point.resetLat(parser.doubleValue()); + } else { + throw new ElasticSearchParseException("only two values allowed"); + } + } else { + throw new ElasticSearchParseException("Numeric value expected"); + } + } + return point; + } else if(parser.currentToken() == Token.VALUE_STRING) { + String data = parser.text(); + int comma = data.indexOf(','); + if(comma > 0) { + double lat = Double.parseDouble(data.substring(0, comma).trim()); + double lon = Double.parseDouble(data.substring(comma+1).trim()); + return point.reset(lat, lon); + } else { + point.resetFromGeoHash(data); + return point; + } + } else { + throw new ElasticSearchParseException("geo_point expected"); + } + } } diff --git a/src/main/java/org/elasticsearch/index/query/FilterBuilders.java b/src/main/java/org/elasticsearch/index/query/FilterBuilders.java index dc8e66563e3..6a8e197ed2b 100644 --- a/src/main/java/org/elasticsearch/index/query/FilterBuilders.java +++ b/src/main/java/org/elasticsearch/index/query/FilterBuilders.java @@ -21,6 +21,7 @@ package org.elasticsearch.index.query; import com.spatial4j.core.shape.Shape; import org.elasticsearch.common.Nullable; +import org.elasticsearch.common.geo.GeoPoint; import org.elasticsearch.common.geo.ShapeRelation; /** @@ -372,6 +373,18 @@ public abstract class FilterBuilders { return new GeohashFilter.Builder(fieldname, geohash); } + /** + * A filter based on a bounding box defined by geohash. The field this filter is applied to + * must have {"type":"geo_point", "geohash":true} + * to work. + * + * @param fieldname The geopoint field name. + * @param point a geopoint within the geohash bucket + */ + public static GeohashFilter.Builder geoHashFilter(String fieldname, GeoPoint point) { + return new GeohashFilter.Builder(fieldname, point); + } + /** * A filter based on a bounding box defined by geohash. The field this filter is applied to * must have {"type":"geo_point", "geohash":true} diff --git a/src/main/java/org/elasticsearch/index/query/GeohashFilter.java b/src/main/java/org/elasticsearch/index/query/GeohashFilter.java index 10e53c141f0..980b2e7ac7f 100644 --- a/src/main/java/org/elasticsearch/index/query/GeohashFilter.java +++ b/src/main/java/org/elasticsearch/index/query/GeohashFilter.java @@ -26,7 +26,9 @@ import org.elasticsearch.common.Nullable; import org.elasticsearch.common.Strings; import org.elasticsearch.common.geo.GeoHashUtils; import org.elasticsearch.common.geo.GeoPoint; +import org.elasticsearch.common.geo.GeoUtils; import org.elasticsearch.common.inject.Inject; +import org.elasticsearch.common.unit.DistanceUnit; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.XContentParser.Token; @@ -56,9 +58,8 @@ import java.util.List; public class GeohashFilter { public static final String NAME = "geohash_cell"; - public static final String FIELDNAME = "field"; - public static final String GEOHASH = "geohash"; public static final String NEIGHBORS = "neighbors"; + public static final String PRECISION = "precision"; /** * Create a new geohash filter for a given set of geohashes. In general this method @@ -90,15 +91,23 @@ public class GeohashFilter { * false. */ public static class Builder extends BaseFilterBuilder { - + // we need to store the geohash rather than the corresponding point, + // because a transformation from a geohash to a point an back to the + // geohash will extend the accuracy of the hash to max precision + // i.e. by filing up with z's. private String fieldname; private String geohash; + private int levels = -1; private boolean neighbors; public Builder(String fieldname) { this(fieldname, null, false); } + public Builder(String fieldname, GeoPoint point) { + this(fieldname, point.geohash(), false); + } + public Builder(String fieldname, String geohash) { this(fieldname, geohash, false); } @@ -110,11 +119,31 @@ public class GeohashFilter { this.neighbors = neighbors; } + public Builder setPoint(GeoPoint point) { + this.geohash = point.getGeohash(); + return this; + } + + public Builder setPoint(double lat, double lon) { + this.geohash = GeoHashUtils.encode(lat, lon); + return this; + } + public Builder setGeohash(String geohash) { this.geohash = geohash; return this; } + public Builder setPrecision(int levels) { + this.levels = levels; + return this; + } + + public Builder setPrecision(String precision) { + double meters = DistanceUnit.parse(precision, DistanceUnit.METERS, DistanceUnit.METERS); + return setPrecision(GeoUtils.geoHashLevelsForPrecision(meters)); + } + public Builder setNeighbors(boolean neighbors) { this.neighbors = neighbors; return this; @@ -128,11 +157,14 @@ public class GeohashFilter { @Override protected void doXContent(XContentBuilder builder, Params params) throws IOException { builder.startObject(NAME); - builder.field(FIELDNAME, fieldname); - builder.field(GEOHASH, geohash); if (neighbors) { builder.field(NEIGHBORS, neighbors); } + if(levels > 0) { + builder.field(PRECISION, levels); + } + builder.field(fieldname, geohash); + builder.endObject(); } } @@ -154,6 +186,7 @@ public class GeohashFilter { String fieldName = null; String geohash = null; + int levels = -1; boolean neighbors = false; XContentParser.Token token; @@ -161,21 +194,35 @@ public class GeohashFilter { throw new ElasticSearchParseException(NAME + " must be an object"); } - while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) { + while ((token = parser.nextToken()) != Token.END_OBJECT) { if (token == Token.FIELD_NAME) { String field = parser.text(); - if (FIELDNAME.equals(field)) { - parser.nextToken(); - fieldName = parser.text(); - } else if (GEOHASH.equals(field)) { - parser.nextToken(); - geohash = parser.text(); + if (PRECISION.equals(field)) { + token = parser.nextToken(); + if(token == Token.VALUE_NUMBER) { + levels = parser.intValue(); + } else if(token == Token.VALUE_STRING) { + double meters = DistanceUnit.parse(parser.text(), DistanceUnit.METERS, DistanceUnit.METERS); + levels = GeoUtils.geoHashLevelsForPrecision(meters); + } } else if (NEIGHBORS.equals(field)) { parser.nextToken(); neighbors = parser.booleanValue(); } else { - throw new ElasticSearchParseException("unexpected field [" + field + "]"); + fieldName = field; + token = parser.nextToken(); + if(token == Token.VALUE_STRING) { + // A string indicates either a gehash or a lat/lon string + String location = parser.text(); + if(location.indexOf(",")>0) { + geohash = GeoPoint.parse(parser).geohash(); + } else { + geohash = location; + } + } else { + geohash = GeoPoint.parse(parser).geohash(); + } } } else { throw new ElasticSearchParseException("unexpected token [" + token + "]"); @@ -194,6 +241,11 @@ public class GeohashFilter { GeoPointFieldMapper geoMapper = ((GeoPointFieldMapper.GeoStringFieldMapper) mapper).geoMapper(); + if(levels > 0) { + int len = Math.min(levels, geohash.length()); + geohash = geohash.substring(0, len); + } + if (neighbors) { return create(parseContext, geoMapper, geohash, GeoHashUtils.neighbors(geohash)); } else { diff --git a/src/test/java/org/elasticsearch/test/integration/search/geo/GeoFilterTests.java b/src/test/java/org/elasticsearch/test/integration/search/geo/GeoFilterTests.java index 38e876b67c9..78b4122c71d 100644 --- a/src/test/java/org/elasticsearch/test/integration/search/geo/GeoFilterTests.java +++ b/src/test/java/org/elasticsearch/test/integration/search/geo/GeoFilterTests.java @@ -441,11 +441,14 @@ public class GeoFilterTests extends AbstractSharedClusterTest { @Test public void testGeoHashFilter() throws IOException { - String geohash = randomhash(12); - List neighbors = GeoHashUtils.neighbors(geohash); - + String geohash = randomhash(10); logger.info("Testing geohash boundingbox filter for [{}]", geohash); + + List neighbors = GeoHashUtils.neighbors(geohash); + List parentNeighbors = GeoHashUtils.neighbors(geohash.substring(0, geohash.length() - 1)); + logger.info("Neighbors {}", neighbors); + logger.info("Parent Neighbors {}", parentNeighbors); String mapping = XContentFactory.jsonBuilder() .startObject() @@ -477,20 +480,38 @@ public class GeoFilterTests extends AbstractSharedClusterTest { client().prepareIndex("locations", "location", "p").setCreate(true).setSource("{\"pin\":\"" + geohash.substring(0, geohash.length() - 1) + "\"}").execute().actionGet(); // index neighbors - List parentNeighbors = GeoHashUtils.neighbors(geohash.substring(0, geohash.length() - 1)); for (int i = 0; i < parentNeighbors.size(); i++) { client().prepareIndex("locations", "location", "p" + i).setCreate(true).setSource("{\"pin\":\"" + parentNeighbors.get(i) + "\"}").execute().actionGet(); } client().admin().indices().prepareRefresh("locations").execute().actionGet(); - // Result of this geohash search should contain the geohash only - SearchResponse results1 = client().prepareSearch("locations").setQuery(QueryBuilders.matchAllQuery()).setFilter("{\"geohash_cell\": {\"field\": \"pin\", \"geohash\": \"" + geohash + "\", \"neighbors\": false}}").execute().actionGet(); + // Result of this geohash search should contain the geohash only + SearchResponse results1 = client().prepareSearch("locations").setQuery(QueryBuilders.matchAllQuery()).setFilter("{\"geohash_cell\": {\"pin\": \"" + geohash + "\", \"neighbors\": false}}").execute().actionGet(); assertHitCount(results1, 1); - SearchResponse results2 = client().prepareSearch("locations").setQuery(QueryBuilders.matchAllQuery()).setFilter("{\"geohash_cell\": {\"field\": \"pin\", \"geohash\": \"" + geohash.substring(0, geohash.length() - 1) + "\", \"neighbors\": true}}").execute().actionGet(); // Result of the parent query should contain the parent it self, its neighbors, the child and all its neighbors + SearchResponse results2 = client().prepareSearch("locations").setQuery(QueryBuilders.matchAllQuery()).setFilter("{\"geohash_cell\": {\"pin\": \"" + geohash.substring(0, geohash.length() - 1) + "\", \"neighbors\": true}}").execute().actionGet(); assertHitCount(results2, 2 + neighbors.size() + parentNeighbors.size()); + + // Testing point formats and precision + GeoPoint point = GeoHashUtils.decode(geohash); + int precision = geohash.length(); + + logger.info("Testing lat/lon format"); + String pointTest1 = "{\"geohash_cell\": {\"pin\": {\"lat\": " + point.lat() + ",\"lon\": " + point.lon() + "},\"precision\": " + precision + ",\"neighbors\": true}}"; + SearchResponse results3 = client().prepareSearch("locations").setQuery(QueryBuilders.matchAllQuery()).setFilter(pointTest1).execute().actionGet(); + assertHitCount(results3, neighbors.size() + 1); + + logger.info("Testing String format"); + String pointTest2 = "{\"geohash_cell\": {\"pin\": \"" + point.lat() + "," + point.lon() + "\",\"precision\": " + precision + ",\"neighbors\": true}}"; + SearchResponse results4 = client().prepareSearch("locations").setQuery(QueryBuilders.matchAllQuery()).setFilter(pointTest2).execute().actionGet(); + assertHitCount(results4, neighbors.size() + 1); + + logger.info("Testing Array format"); + String pointTest3 = "{\"geohash_cell\": {\"pin\": [" + point.lon() + "," + point.lat() + "],\"precision\": " + precision + ",\"neighbors\": true}}"; + SearchResponse results5 = client().prepareSearch("locations").setQuery(QueryBuilders.matchAllQuery()).setFilter(pointTest3).execute().actionGet(); + assertHitCount(results5, neighbors.size() + 1); } @Test