From 7a4fca2c1a60e86ebabeddec90ef17fa57776603 Mon Sep 17 00:00:00 2001 From: Shay Banon Date: Fri, 19 Aug 2011 07:07:02 +0300 Subject: [PATCH] Geo Distance Filter Bounding Box Optimization, closes #1261. --- .idea/dictionaries/kimchy.xml | 1 + .../geo/GeoDistanceSearchBenchmark.java | 39 ++++++--- .../mapper/geo/GeoPointDocFieldData.java | 8 ++ .../query/GeoBoundingBoxFilterBuilder.java | 10 +-- .../query/GeoBoundingBoxFilterParser.java | 11 +-- .../index/query/GeoDistanceFilterBuilder.java | 10 +++ .../index/query/GeoDistanceFilterParser.java | 5 +- .../query/GeoDistanceRangeFilterParser.java | 5 +- .../index/query/GeoPolygonFilterBuilder.java | 8 +- .../index/query/GeoPolygonFilterParser.java | 11 +-- .../search/geo/GeoBoundingBoxFilter.java | 9 +- .../index/search/geo/GeoDistance.java | 82 +++++++++++++++++++ .../index/search/geo/GeoDistanceFilter.java | 31 +++++-- .../search/geo/GeoDistanceRangeFilter.java | 34 ++++++-- .../index/search/geo/GeoPolygonFilter.java | 13 --- .../elasticsearch/index/search/geo/Point.java | 63 ++++++++++++++ .../index/search/geo/GeoDistanceTests.java | 50 +++++++++++ 17 files changed, 321 insertions(+), 69 deletions(-) create mode 100644 modules/elasticsearch/src/main/java/org/elasticsearch/index/search/geo/Point.java create mode 100644 modules/elasticsearch/src/test/java/org/elasticsearch/index/search/geo/GeoDistanceTests.java diff --git a/.idea/dictionaries/kimchy.xml b/.idea/dictionaries/kimchy.xml index 56220be22aa..9d480269696 100644 --- a/.idea/dictionaries/kimchy.xml +++ b/.idea/dictionaries/kimchy.xml @@ -12,6 +12,7 @@ attr auth banon + bbox bindhost birthdate bitset diff --git a/modules/benchmark/micro/src/main/java/org/elasticsearch/benchmark/search/geo/GeoDistanceSearchBenchmark.java b/modules/benchmark/micro/src/main/java/org/elasticsearch/benchmark/search/geo/GeoDistanceSearchBenchmark.java index 156d9be73b9..402b125d73f 100644 --- a/modules/benchmark/micro/src/main/java/org/elasticsearch/benchmark/search/geo/GeoDistanceSearchBenchmark.java +++ b/modules/benchmark/micro/src/main/java/org/elasticsearch/benchmark/search/geo/GeoDistanceSearchBenchmark.java @@ -104,46 +104,63 @@ public class GeoDistanceSearchBenchmark { client.admin().indices().prepareRefresh().execute().actionGet(); } - System.err.println("--> Warming up (ARC)"); + System.err.println("--> Warming up (ARC) - optimize_bbox"); long start = System.currentTimeMillis(); for (int i = 0; i < NUM_WARM; i++) { - run(client, GeoDistance.ARC); + run(client, GeoDistance.ARC, true); } long totalTime = System.currentTimeMillis() - start; - System.out.println("--> Warmup (ARC) " + (totalTime / NUM_WARM) + "ms"); + System.err.println("--> Warmup (ARC) - optimize_bbox " + (totalTime / NUM_WARM) + "ms"); - System.err.println("--> Perf (ARC)"); + System.err.println("--> Perf (ARC) - optimize_bbox"); start = System.currentTimeMillis(); for (int i = 0; i < NUM_RUNS; i++) { - run(client, GeoDistance.ARC); + run(client, GeoDistance.ARC, true); } totalTime = System.currentTimeMillis() - start; - System.out.println("--> Perf (ARC) " + (totalTime / NUM_RUNS) + "ms"); + System.err.println("--> Perf (ARC) - optimize_bbox " + (totalTime / NUM_RUNS) + "ms"); + + System.err.println("--> Warming up (ARC) - no optimize_bbox"); + start = System.currentTimeMillis(); + for (int i = 0; i < NUM_WARM; i++) { + run(client, GeoDistance.ARC, false); + } + totalTime = System.currentTimeMillis() - start; + System.err.println("--> Warmup (ARC) - no optimize_bbox " + (totalTime / NUM_WARM) + "ms"); + + System.err.println("--> Perf (ARC) - no optimize_bbox"); + start = System.currentTimeMillis(); + for (int i = 0; i < NUM_RUNS; i++) { + run(client, GeoDistance.ARC, false); + } + totalTime = System.currentTimeMillis() - start; + System.err.println("--> Perf (ARC) - no optimize_bbox " + (totalTime / NUM_RUNS) + "ms"); System.err.println("--> Warming up (PLANE)"); start = System.currentTimeMillis(); for (int i = 0; i < NUM_WARM; i++) { - run(client, GeoDistance.PLANE); + run(client, GeoDistance.PLANE, true); } totalTime = System.currentTimeMillis() - start; - System.out.println("--> Warmup (PLANE) " + (totalTime / NUM_WARM) + "ms"); + System.err.println("--> Warmup (PLANE) " + (totalTime / NUM_WARM) + "ms"); System.err.println("--> Perf (PLANE)"); start = System.currentTimeMillis(); for (int i = 0; i < NUM_RUNS; i++) { - run(client, GeoDistance.PLANE); + run(client, GeoDistance.PLANE, true); } totalTime = System.currentTimeMillis() - start; - System.out.println("--> Perf (PLANE) " + (totalTime / NUM_RUNS) + "ms"); + System.err.println("--> Perf (PLANE) " + (totalTime / NUM_RUNS) + "ms"); node.close(); } - public static void run(Client client, GeoDistance geoDistance) { + public static void run(Client client, GeoDistance geoDistance, boolean optimizeBbox) { client.prepareSearch() // from NY .setSearchType(SearchType.COUNT) .setQuery(filteredQuery(matchAllQuery(), geoDistanceFilter("location") .distance("2km") + .optimizeBbox(optimizeBbox) .geoDistance(geoDistance) .point(40.7143528, -74.0059731))) .execute().actionGet(); diff --git a/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/geo/GeoPointDocFieldData.java b/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/geo/GeoPointDocFieldData.java index 6f02c33bb87..db8c1e819e9 100644 --- a/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/geo/GeoPointDocFieldData.java +++ b/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/geo/GeoPointDocFieldData.java @@ -43,6 +43,14 @@ public class GeoPointDocFieldData extends DocFieldData { return fieldData.factorDistance(docId, DistanceUnit.MILES, lat, lon); } + public double factorDistance02(double lat, double lon) { + return fieldData.factorDistance(docId, DistanceUnit.MILES, lat, lon) + 1; + } + + public double factorDistance13(double lat, double lon) { + return fieldData.factorDistance(docId, DistanceUnit.MILES, lat, lon) + 2; + } + public double arcDistance(double lat, double lon) { return fieldData.arcDistance(docId, DistanceUnit.MILES, lat, lon); } diff --git a/modules/elasticsearch/src/main/java/org/elasticsearch/index/query/GeoBoundingBoxFilterBuilder.java b/modules/elasticsearch/src/main/java/org/elasticsearch/index/query/GeoBoundingBoxFilterBuilder.java index 0e5ebbe4728..506666d9f26 100644 --- a/modules/elasticsearch/src/main/java/org/elasticsearch/index/query/GeoBoundingBoxFilterBuilder.java +++ b/modules/elasticsearch/src/main/java/org/elasticsearch/index/query/GeoBoundingBoxFilterBuilder.java @@ -20,7 +20,7 @@ package org.elasticsearch.index.query; import org.elasticsearch.common.xcontent.XContentBuilder; -import org.elasticsearch.index.search.geo.GeoBoundingBoxFilter; +import org.elasticsearch.index.search.geo.Point; import java.io.IOException; @@ -31,11 +31,11 @@ public class GeoBoundingBoxFilterBuilder extends BaseFilterBuilder { private final String name; - private GeoBoundingBoxFilter.Point topLeft; + private Point topLeft; private String topLeftGeohash; - private GeoBoundingBoxFilter.Point bottomRight; + private Point bottomRight; private String bottomRightGeohash; @@ -55,7 +55,7 @@ public class GeoBoundingBoxFilterBuilder extends BaseFilterBuilder { * @param lon The longitude */ public GeoBoundingBoxFilterBuilder topLeft(double lat, double lon) { - topLeft = new GeoBoundingBoxFilter.Point(); + topLeft = new Point(); topLeft.lat = lat; topLeft.lon = lon; return this; @@ -68,7 +68,7 @@ public class GeoBoundingBoxFilterBuilder extends BaseFilterBuilder { * @param lon The longitude */ public GeoBoundingBoxFilterBuilder bottomRight(double lat, double lon) { - bottomRight = new GeoBoundingBoxFilter.Point(); + bottomRight = new Point(); bottomRight.lat = lat; bottomRight.lon = lon; return this; diff --git a/modules/elasticsearch/src/main/java/org/elasticsearch/index/query/GeoBoundingBoxFilterParser.java b/modules/elasticsearch/src/main/java/org/elasticsearch/index/query/GeoBoundingBoxFilterParser.java index 4bdf935fd99..b02b2c3a6b5 100644 --- a/modules/elasticsearch/src/main/java/org/elasticsearch/index/query/GeoBoundingBoxFilterParser.java +++ b/modules/elasticsearch/src/main/java/org/elasticsearch/index/query/GeoBoundingBoxFilterParser.java @@ -29,6 +29,7 @@ import org.elasticsearch.index.mapper.geo.GeoPointFieldDataType; import org.elasticsearch.index.mapper.geo.GeoPointFieldMapper; import org.elasticsearch.index.search.geo.GeoBoundingBoxFilter; import org.elasticsearch.index.search.geo.GeoHashUtils; +import org.elasticsearch.index.search.geo.Point; import java.io.IOException; @@ -54,8 +55,8 @@ public class GeoBoundingBoxFilterParser implements FilterParser { boolean cache = false; CacheKeyFilter.Key cacheKey = null; String fieldName = null; - GeoBoundingBoxFilter.Point topLeft = new GeoBoundingBoxFilter.Point(); - GeoBoundingBoxFilter.Point bottomRight = new GeoBoundingBoxFilter.Point(); + Point topLeft = new Point(); + Point bottomRight = new Point(); String filterName = null; String currentFieldName = null; @@ -70,7 +71,7 @@ public class GeoBoundingBoxFilterParser implements FilterParser { if (token == XContentParser.Token.FIELD_NAME) { currentFieldName = parser.currentName(); } else if (token == XContentParser.Token.START_ARRAY) { - GeoBoundingBoxFilter.Point point = null; + Point point = null; if ("top_left".equals(currentFieldName) || "topLeft".equals(currentFieldName)) { point = topLeft; } else if ("bottom_right".equals(currentFieldName) || "bottomRight".equals(currentFieldName)) { @@ -87,7 +88,7 @@ public class GeoBoundingBoxFilterParser implements FilterParser { } } } else if (token == XContentParser.Token.START_OBJECT) { - GeoBoundingBoxFilter.Point point = null; + Point point = null; if ("top_left".equals(currentFieldName) || "topLeft".equals(currentFieldName)) { point = topLeft; } else if ("bottom_right".equals(currentFieldName) || "bottomRight".equals(currentFieldName)) { @@ -115,7 +116,7 @@ public class GeoBoundingBoxFilterParser implements FilterParser { if ("field".equals(currentFieldName)) { fieldName = parser.text(); } else { - GeoBoundingBoxFilter.Point point = null; + Point point = null; if ("top_left".equals(currentFieldName) || "topLeft".equals(currentFieldName)) { point = topLeft; } else if ("bottom_right".equals(currentFieldName) || "bottomRight".equals(currentFieldName)) { diff --git a/modules/elasticsearch/src/main/java/org/elasticsearch/index/query/GeoDistanceFilterBuilder.java b/modules/elasticsearch/src/main/java/org/elasticsearch/index/query/GeoDistanceFilterBuilder.java index 3ce12218f6d..f4295cc32ae 100644 --- a/modules/elasticsearch/src/main/java/org/elasticsearch/index/query/GeoDistanceFilterBuilder.java +++ b/modules/elasticsearch/src/main/java/org/elasticsearch/index/query/GeoDistanceFilterBuilder.java @@ -42,6 +42,8 @@ public class GeoDistanceFilterBuilder extends BaseFilterBuilder { private GeoDistance geoDistance; + private Boolean optimizeBbox; + private Boolean cache; private String cacheKey; @@ -87,6 +89,11 @@ public class GeoDistanceFilterBuilder extends BaseFilterBuilder { return this; } + public GeoDistanceFilterBuilder optimizeBbox(boolean optimizeBbox) { + this.optimizeBbox = optimizeBbox; + return this; + } + /** * Sets the filter name for the filter that can be used when searching for matched_filters per hit. */ @@ -119,6 +126,9 @@ public class GeoDistanceFilterBuilder extends BaseFilterBuilder { if (geoDistance != null) { builder.field("distance_type", geoDistance.name().toLowerCase()); } + if (optimizeBbox != null) { + builder.field("optimize_bbox", optimizeBbox.booleanValue()); + } if (filterName != null) { builder.field("_name", filterName); } diff --git a/modules/elasticsearch/src/main/java/org/elasticsearch/index/query/GeoDistanceFilterParser.java b/modules/elasticsearch/src/main/java/org/elasticsearch/index/query/GeoDistanceFilterParser.java index ee3ea4311e6..41bd9fd3e46 100644 --- a/modules/elasticsearch/src/main/java/org/elasticsearch/index/query/GeoDistanceFilterParser.java +++ b/modules/elasticsearch/src/main/java/org/elasticsearch/index/query/GeoDistanceFilterParser.java @@ -73,6 +73,7 @@ public class GeoDistanceFilterParser implements FilterParser { Object vDistance = null; DistanceUnit unit = DistanceUnit.KILOMETERS; // default unit GeoDistance geoDistance = GeoDistance.ARC; + boolean optimizeBbox = true; while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) { if (token == XContentParser.Token.FIELD_NAME) { currentFieldName = parser.currentName(); @@ -132,6 +133,8 @@ public class GeoDistanceFilterParser implements FilterParser { cache = parser.booleanValue(); } else if ("_cache_key".equals(currentFieldName) || "_cacheKey".equals(currentFieldName)) { cacheKey = new CacheKeyFilter.Key(parser.text()); + } else if ("optimize_bbox".equals(currentFieldName) || "optimizeBbox".equals(currentFieldName)) { + optimizeBbox = parser.booleanValue(); } else { // assume the value is the actual value String value = parser.text(); @@ -166,7 +169,7 @@ public class GeoDistanceFilterParser implements FilterParser { } fieldName = mapper.names().indexName(); - Filter filter = new GeoDistanceFilter(lat, lon, distance, geoDistance, fieldName, parseContext.indexCache().fieldData()); + Filter filter = new GeoDistanceFilter(lat, lon, distance, geoDistance, fieldName, parseContext.indexCache().fieldData(), optimizeBbox); if (cache) { filter = parseContext.cacheFilter(filter, cacheKey); } diff --git a/modules/elasticsearch/src/main/java/org/elasticsearch/index/query/GeoDistanceRangeFilterParser.java b/modules/elasticsearch/src/main/java/org/elasticsearch/index/query/GeoDistanceRangeFilterParser.java index a4a9540576b..4e5163207c2 100644 --- a/modules/elasticsearch/src/main/java/org/elasticsearch/index/query/GeoDistanceRangeFilterParser.java +++ b/modules/elasticsearch/src/main/java/org/elasticsearch/index/query/GeoDistanceRangeFilterParser.java @@ -75,6 +75,7 @@ public class GeoDistanceRangeFilterParser implements FilterParser { boolean includeUpper = true; DistanceUnit unit = DistanceUnit.KILOMETERS; // default unit GeoDistance geoDistance = GeoDistance.ARC; + boolean optimizeBbox = true; while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) { if (token == XContentParser.Token.FIELD_NAME) { currentFieldName = parser.currentName(); @@ -181,6 +182,8 @@ public class GeoDistanceRangeFilterParser implements FilterParser { cache = parser.booleanValue(); } else if ("_cache_key".equals(currentFieldName) || "_cacheKey".equals(currentFieldName)) { cacheKey = new CacheKeyFilter.Key(parser.text()); + } else if ("optimize_bbox".equals(currentFieldName) || "optimizeBbox".equals(currentFieldName)) { + optimizeBbox = parser.booleanValue(); } else { // assume the value is the actual value String value = parser.text(); @@ -223,7 +226,7 @@ public class GeoDistanceRangeFilterParser implements FilterParser { } fieldName = mapper.names().indexName(); - Filter filter = new GeoDistanceRangeFilter(lat, lon, from, to, includeLower, includeUpper, geoDistance, fieldName, parseContext.indexCache().fieldData()); + Filter filter = new GeoDistanceRangeFilter(lat, lon, from, to, includeLower, includeUpper, geoDistance, fieldName, parseContext.indexCache().fieldData(), optimizeBbox); if (cache) { filter = parseContext.cacheFilter(filter, cacheKey); } diff --git a/modules/elasticsearch/src/main/java/org/elasticsearch/index/query/GeoPolygonFilterBuilder.java b/modules/elasticsearch/src/main/java/org/elasticsearch/index/query/GeoPolygonFilterBuilder.java index 43d8292fb9d..1d6ea246d63 100644 --- a/modules/elasticsearch/src/main/java/org/elasticsearch/index/query/GeoPolygonFilterBuilder.java +++ b/modules/elasticsearch/src/main/java/org/elasticsearch/index/query/GeoPolygonFilterBuilder.java @@ -22,7 +22,7 @@ package org.elasticsearch.index.query; import org.elasticsearch.common.collect.Lists; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.index.search.geo.GeoHashUtils; -import org.elasticsearch.index.search.geo.GeoPolygonFilter; +import org.elasticsearch.index.search.geo.Point; import java.io.IOException; import java.util.List; @@ -34,7 +34,7 @@ public class GeoPolygonFilterBuilder extends BaseFilterBuilder { private final String name; - private final List points = Lists.newArrayList(); + private final List points = Lists.newArrayList(); private Boolean cache; private String cacheKey; @@ -53,7 +53,7 @@ public class GeoPolygonFilterBuilder extends BaseFilterBuilder { * @return */ public GeoPolygonFilterBuilder addPoint(double lat, double lon) { - points.add(new GeoPolygonFilter.Point(lat, lon)); + points.add(new Point(lat, lon)); return this; } @@ -88,7 +88,7 @@ public class GeoPolygonFilterBuilder extends BaseFilterBuilder { builder.startObject(name); builder.startArray("points"); - for (GeoPolygonFilter.Point point : points) { + for (Point point : points) { builder.startArray().value(point.lon).value(point.lat).endArray(); } builder.endArray(); diff --git a/modules/elasticsearch/src/main/java/org/elasticsearch/index/query/GeoPolygonFilterParser.java b/modules/elasticsearch/src/main/java/org/elasticsearch/index/query/GeoPolygonFilterParser.java index 9e3d00760ba..4cf471b6d63 100644 --- a/modules/elasticsearch/src/main/java/org/elasticsearch/index/query/GeoPolygonFilterParser.java +++ b/modules/elasticsearch/src/main/java/org/elasticsearch/index/query/GeoPolygonFilterParser.java @@ -30,6 +30,7 @@ import org.elasticsearch.index.mapper.geo.GeoPointFieldDataType; import org.elasticsearch.index.mapper.geo.GeoPointFieldMapper; import org.elasticsearch.index.search.geo.GeoHashUtils; import org.elasticsearch.index.search.geo.GeoPolygonFilter; +import org.elasticsearch.index.search.geo.Point; import java.io.IOException; import java.util.List; @@ -67,7 +68,7 @@ public class GeoPolygonFilterParser implements FilterParser { boolean cache = false; CacheKeyFilter.Key cacheKey = null; String fieldName = null; - List points = Lists.newArrayList(); + List points = Lists.newArrayList(); String filterName = null; @@ -89,7 +90,7 @@ public class GeoPolygonFilterParser implements FilterParser { if (token == XContentParser.Token.FIELD_NAME) { currentFieldName = parser.currentName(); } else if (token == XContentParser.Token.START_ARRAY) { - GeoPolygonFilter.Point point = new GeoPolygonFilter.Point(); + Point point = new Point(); token = parser.nextToken(); point.lon = parser.doubleValue(); token = parser.nextToken(); @@ -99,7 +100,7 @@ public class GeoPolygonFilterParser implements FilterParser { } points.add(point); } else if (token == XContentParser.Token.START_OBJECT) { - GeoPolygonFilter.Point point = new GeoPolygonFilter.Point(); + Point point = new Point(); while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) { if (token == XContentParser.Token.FIELD_NAME) { currentFieldName = parser.currentName(); @@ -117,7 +118,7 @@ public class GeoPolygonFilterParser implements FilterParser { } points.add(point); } else if (token.isValue()) { - GeoPolygonFilter.Point point = new GeoPolygonFilter.Point(); + Point point = new Point(); String value = parser.text(); int comma = value.indexOf(','); if (comma != -1) { @@ -159,7 +160,7 @@ public class GeoPolygonFilterParser implements FilterParser { } fieldName = mapper.names().indexName(); - Filter filter = new GeoPolygonFilter(points.toArray(new GeoPolygonFilter.Point[points.size()]), fieldName, parseContext.indexCache().fieldData()); + Filter filter = new GeoPolygonFilter(points.toArray(new Point[points.size()]), fieldName, parseContext.indexCache().fieldData()); if (cache) { filter = parseContext.cacheFilter(filter, cacheKey); } diff --git a/modules/elasticsearch/src/main/java/org/elasticsearch/index/search/geo/GeoBoundingBoxFilter.java b/modules/elasticsearch/src/main/java/org/elasticsearch/index/search/geo/GeoBoundingBoxFilter.java index 1f74e0ea4cc..ea3cc61c0bc 100644 --- a/modules/elasticsearch/src/main/java/org/elasticsearch/index/search/geo/GeoBoundingBoxFilter.java +++ b/modules/elasticsearch/src/main/java/org/elasticsearch/index/search/geo/GeoBoundingBoxFilter.java @@ -102,7 +102,7 @@ public class GeoBoundingBoxFilter extends Filter { for (int i = 0; i < lats.length; i++) { double lat = lats[i]; double lon = lons[i]; - if (((topLeft.lon <= lon && 180 >= lon) || (-180 <= lon && bottomRight.lon >= lon)) && + if (((topLeft.lon <= lon || bottomRight.lon >= lon)) && (topLeft.lat >= lat && bottomRight.lat <= lat)) { return true; } @@ -111,7 +111,7 @@ public class GeoBoundingBoxFilter extends Filter { double lat = fieldData.latValue(doc); double lon = fieldData.lonValue(doc); - if (((topLeft.lon <= lon && 180 >= lon) || (-180 <= lon && bottomRight.lon >= lon)) && + if (((topLeft.lon <= lon || bottomRight.lon >= lon)) && (topLeft.lat >= lat && bottomRight.lat <= lat)) { return true; } @@ -165,9 +165,4 @@ public class GeoBoundingBoxFilter extends Filter { return false; } } - - public static class Point { - public double lat; - public double lon; - } } diff --git a/modules/elasticsearch/src/main/java/org/elasticsearch/index/search/geo/GeoDistance.java b/modules/elasticsearch/src/main/java/org/elasticsearch/index/search/geo/GeoDistance.java index 760ae078ba6..57fef026765 100644 --- a/modules/elasticsearch/src/main/java/org/elasticsearch/index/search/geo/GeoDistance.java +++ b/modules/elasticsearch/src/main/java/org/elasticsearch/index/search/geo/GeoDistance.java @@ -101,6 +101,44 @@ public enum GeoDistance { public abstract FixedSourceDistance fixedSourceDistance(double sourceLatitude, double sourceLongitude, DistanceUnit unit); + private static final double MIN_LAT = Math.toRadians(-90d); // -PI/2 + private static final double MAX_LAT = Math.toRadians(90d); // PI/2 + private static final double MIN_LON = Math.toRadians(-180d); // -PI + private static final double MAX_LON = Math.toRadians(180d); // PI + + public static DistanceBoundingCheck distanceBoundingCheck(double sourceLatitude, double sourceLongitude, double distance, DistanceUnit unit) { + // angular distance in radians on a great circle + double radDist = distance / unit.getEarthRadius(); + + double radLat = Math.toRadians(sourceLatitude); + double radLon = Math.toRadians(sourceLongitude); + + double minLat = radLat - radDist; + double maxLat = radLat + radDist; + + double minLon, maxLon; + if (minLat > MIN_LAT && maxLat < MAX_LAT) { + double deltaLon = Math.asin(Math.sin(radDist) / Math.cos(radLat)); + minLon = radLon - deltaLon; + if (minLon < MIN_LON) minLon += 2d * Math.PI; + maxLon = radLon + deltaLon; + if (maxLon > MAX_LON) maxLon -= 2d * Math.PI; + } else { + // a pole is within the distance + minLat = Math.max(minLat, MIN_LAT); + maxLat = Math.min(maxLat, MAX_LAT); + minLon = MIN_LON; + maxLon = MAX_LON; + } + + Point left = new Point(Math.toDegrees(minLat), Math.toDegrees(minLon)); + Point right = new Point(Math.toDegrees(maxLat), Math.toDegrees(maxLon)); + if (minLon > maxLon) { + return new Meridian180DistanceBoundingCheck(left, right); + } + return new SimpleDistanceBoundingCheck(left, right); + } + public static GeoDistance fromString(String s) { if ("plane".equals(s)) { return PLANE; @@ -117,6 +155,50 @@ public enum GeoDistance { double calculate(double targetLatitude, double targetLongitude); } + public static interface DistanceBoundingCheck { + + boolean isWithin(double targetLatitude, double targetLongitude); + } + + public static AlwaysDistanceBoundingCheck ALWAYS_INSTANCE = new AlwaysDistanceBoundingCheck(); + + private static class AlwaysDistanceBoundingCheck implements DistanceBoundingCheck { + @Override public boolean isWithin(double targetLatitude, double targetLongitude) { + return true; + } + } + + public static class Meridian180DistanceBoundingCheck implements DistanceBoundingCheck { + + private final Point left; + private final Point right; + + public Meridian180DistanceBoundingCheck(Point left, Point right) { + this.left = left; + this.right = right; + } + + @Override public boolean isWithin(double targetLatitude, double targetLongitude) { + return (targetLatitude >= left.lat && targetLatitude <= right.lat) && + (targetLongitude >= left.lon || targetLongitude <= right.lon); + } + } + + public static class SimpleDistanceBoundingCheck implements DistanceBoundingCheck { + private final Point left; + private final Point right; + + public SimpleDistanceBoundingCheck(Point left, Point right) { + this.left = left; + this.right = right; + } + + @Override public boolean isWithin(double targetLatitude, double targetLongitude) { + return (targetLatitude >= left.lat && targetLatitude <= right.lat) && + (targetLongitude >= left.lon && targetLongitude <= right.lon); + } + } + public static class PlaneFixedSourceDistance implements FixedSourceDistance { private final double sourceLatitude; diff --git a/modules/elasticsearch/src/main/java/org/elasticsearch/index/search/geo/GeoDistanceFilter.java b/modules/elasticsearch/src/main/java/org/elasticsearch/index/search/geo/GeoDistanceFilter.java index 55893b46460..0605d825f96 100644 --- a/modules/elasticsearch/src/main/java/org/elasticsearch/index/search/geo/GeoDistanceFilter.java +++ b/modules/elasticsearch/src/main/java/org/elasticsearch/index/search/geo/GeoDistanceFilter.java @@ -48,8 +48,10 @@ public class GeoDistanceFilter extends Filter { private final FieldDataCache fieldDataCache; private final GeoDistance.FixedSourceDistance fixedSourceDistance; + private final GeoDistance.DistanceBoundingCheck distanceBoundingCheck; - public GeoDistanceFilter(double lat, double lon, double distance, GeoDistance geoDistance, String fieldName, FieldDataCache fieldDataCache) { + public GeoDistanceFilter(double lat, double lon, double distance, GeoDistance geoDistance, String fieldName, FieldDataCache fieldDataCache, + boolean optimizeBbox) { this.lat = lat; this.lon = lon; this.distance = distance; @@ -58,6 +60,7 @@ public class GeoDistanceFilter extends Filter { this.fieldDataCache = fieldDataCache; this.fixedSourceDistance = geoDistance.fixedSourceDistance(lat, lon, DistanceUnit.MILES); + this.distanceBoundingCheck = optimizeBbox ? GeoDistance.distanceBoundingCheck(lat, lon, distance, DistanceUnit.MILES) : GeoDistance.ALWAYS_INSTANCE; } public double lat() { @@ -82,7 +85,7 @@ public class GeoDistanceFilter extends Filter { @Override public DocIdSet getDocIdSet(IndexReader reader) throws IOException { final GeoPointFieldData fieldData = (GeoPointFieldData) fieldDataCache.cache(GeoPointFieldDataType.TYPE, reader, fieldName); - return new GeoDistanceDocSet(reader.maxDoc(), fieldData, fixedSourceDistance, distance); + return new GeoDistanceDocSet(reader.maxDoc(), fieldData, fixedSourceDistance, distanceBoundingCheck, distance); } @Override @@ -120,11 +123,14 @@ public class GeoDistanceFilter extends Filter { private final double distance; // in miles private final GeoPointFieldData fieldData; private final GeoDistance.FixedSourceDistance fixedSourceDistance; + private final GeoDistance.DistanceBoundingCheck distanceBoundingCheck; - public GeoDistanceDocSet(int maxDoc, GeoPointFieldData fieldData, GeoDistance.FixedSourceDistance fixedSourceDistance, double distance) { + public GeoDistanceDocSet(int maxDoc, GeoPointFieldData fieldData, GeoDistance.FixedSourceDistance fixedSourceDistance, GeoDistance.DistanceBoundingCheck distanceBoundingCheck, + double distance) { super(maxDoc); this.fieldData = fieldData; this.fixedSourceDistance = fixedSourceDistance; + this.distanceBoundingCheck = distanceBoundingCheck; this.distance = distance; } @@ -144,16 +150,25 @@ public class GeoDistanceFilter extends Filter { double[] lats = fieldData.latValues(doc); double[] lons = fieldData.lonValues(doc); for (int i = 0; i < lats.length; i++) { - double d = fixedSourceDistance.calculate(lats[i], lons[i]); - if (d < distance) { - return true; + double lat = lats[i]; + double lon = lons[i]; + if (distanceBoundingCheck.isWithin(lat, lon)) { + double d = fixedSourceDistance.calculate(lat, lon); + if (d < distance) { + return true; + } } } return false; } else { - double d = fixedSourceDistance.calculate(fieldData.latValue(doc), fieldData.lonValue(doc)); - return d < distance; + double lat = fieldData.latValue(doc); + double lon = fieldData.lonValue(doc); + if (distanceBoundingCheck.isWithin(lat, lon)) { + double d = fixedSourceDistance.calculate(lat, lon); + return d < distance; + } } + return false; } } } diff --git a/modules/elasticsearch/src/main/java/org/elasticsearch/index/search/geo/GeoDistanceRangeFilter.java b/modules/elasticsearch/src/main/java/org/elasticsearch/index/search/geo/GeoDistanceRangeFilter.java index 3f268dee836..1e3c87eb2e2 100644 --- a/modules/elasticsearch/src/main/java/org/elasticsearch/index/search/geo/GeoDistanceRangeFilter.java +++ b/modules/elasticsearch/src/main/java/org/elasticsearch/index/search/geo/GeoDistanceRangeFilter.java @@ -44,12 +44,14 @@ public class GeoDistanceRangeFilter extends Filter { private final GeoDistance geoDistance; private final GeoDistance.FixedSourceDistance fixedSourceDistance; + private final GeoDistance.DistanceBoundingCheck distanceBoundingCheck; private final String fieldName; private final FieldDataCache fieldDataCache; - public GeoDistanceRangeFilter(double lat, double lon, Double lowerVal, Double upperVal, boolean includeLower, boolean includeUpper, GeoDistance geoDistance, String fieldName, FieldDataCache fieldDataCache) { + public GeoDistanceRangeFilter(double lat, double lon, Double lowerVal, Double upperVal, boolean includeLower, boolean includeUpper, GeoDistance geoDistance, String fieldName, FieldDataCache fieldDataCache, + boolean optimizeBbox) { this.lat = lat; this.lon = lon; this.geoDistance = geoDistance; @@ -71,7 +73,10 @@ public class GeoDistanceRangeFilter extends Filter { inclusiveUpperPoint = NumericUtils.sortableLongToDouble(includeUpper ? i : (i - 1L)); } else { inclusiveUpperPoint = Double.POSITIVE_INFINITY; + optimizeBbox = false; } + + this.distanceBoundingCheck = optimizeBbox ? GeoDistance.distanceBoundingCheck(lat, lon, inclusiveUpperPoint, DistanceUnit.MILES) : GeoDistance.ALWAYS_INSTANCE; } public double lat() { @@ -92,7 +97,7 @@ public class GeoDistanceRangeFilter extends Filter { @Override public DocIdSet getDocIdSet(IndexReader reader) throws IOException { final GeoPointFieldData fieldData = (GeoPointFieldData) fieldDataCache.cache(GeoPointFieldDataType.TYPE, reader, fieldName); - return new GeoDistanceRangeDocSet(reader.maxDoc(), fieldData, fixedSourceDistance, inclusiveLowerPoint, inclusiveUpperPoint); + return new GeoDistanceRangeDocSet(reader.maxDoc(), fieldData, fixedSourceDistance, distanceBoundingCheck, inclusiveLowerPoint, inclusiveUpperPoint); } @Override @@ -133,13 +138,16 @@ public class GeoDistanceRangeFilter extends Filter { private final GeoPointFieldData fieldData; private final GeoDistance.FixedSourceDistance fixedSourceDistance; + private final GeoDistance.DistanceBoundingCheck distanceBoundingCheck; private final double inclusiveLowerPoint; // in miles private final double inclusiveUpperPoint; // in miles - public GeoDistanceRangeDocSet(int maxDoc, GeoPointFieldData fieldData, GeoDistance.FixedSourceDistance fixedSourceDistance, double inclusiveLowerPoint, double inclusiveUpperPoint) { + public GeoDistanceRangeDocSet(int maxDoc, GeoPointFieldData fieldData, GeoDistance.FixedSourceDistance fixedSourceDistance, GeoDistance.DistanceBoundingCheck distanceBoundingCheck, + double inclusiveLowerPoint, double inclusiveUpperPoint) { super(maxDoc); this.fieldData = fieldData; this.fixedSourceDistance = fixedSourceDistance; + this.distanceBoundingCheck = distanceBoundingCheck; this.inclusiveLowerPoint = inclusiveLowerPoint; this.inclusiveUpperPoint = inclusiveUpperPoint; } @@ -160,16 +168,24 @@ public class GeoDistanceRangeFilter extends Filter { double[] lats = fieldData.latValues(doc); double[] lons = fieldData.lonValues(doc); for (int i = 0; i < lats.length; i++) { - double d = fixedSourceDistance.calculate(lats[i], lons[i]); - if (d >= inclusiveLowerPoint && d <= inclusiveUpperPoint) { - return true; + double lat = lats[i]; + double lon = lons[i]; + if (distanceBoundingCheck.isWithin(lat, lon)) { + double d = fixedSourceDistance.calculate(lat, lon); + if (d >= inclusiveLowerPoint && d <= inclusiveUpperPoint) { + return true; + } } } return false; } else { - double d = fixedSourceDistance.calculate(fieldData.latValue(doc), fieldData.lonValue(doc)); - if (d >= inclusiveLowerPoint && d <= inclusiveUpperPoint) { - return true; + double lat = fieldData.latValue(doc); + double lon = fieldData.lonValue(doc); + if (distanceBoundingCheck.isWithin(lat, lon)) { + double d = fixedSourceDistance.calculate(lat, lon); + if (d >= inclusiveLowerPoint && d <= inclusiveUpperPoint) { + return true; + } } return false; } diff --git a/modules/elasticsearch/src/main/java/org/elasticsearch/index/search/geo/GeoPolygonFilter.java b/modules/elasticsearch/src/main/java/org/elasticsearch/index/search/geo/GeoPolygonFilter.java index 08f4374f899..9be91c86826 100644 --- a/modules/elasticsearch/src/main/java/org/elasticsearch/index/search/geo/GeoPolygonFilter.java +++ b/modules/elasticsearch/src/main/java/org/elasticsearch/index/search/geo/GeoPolygonFilter.java @@ -115,17 +115,4 @@ public class GeoPolygonFilter extends Filter { return inPoly; } } - - public static class Point { - public double lat; - public double lon; - - public Point() { - } - - public Point(double lat, double lon) { - this.lat = lat; - this.lon = lon; - } - } } diff --git a/modules/elasticsearch/src/main/java/org/elasticsearch/index/search/geo/Point.java b/modules/elasticsearch/src/main/java/org/elasticsearch/index/search/geo/Point.java new file mode 100644 index 00000000000..ca32c68a45b --- /dev/null +++ b/modules/elasticsearch/src/main/java/org/elasticsearch/index/search/geo/Point.java @@ -0,0 +1,63 @@ +/* + * Licensed to Elastic Search and Shay Banon under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. Elastic Search licenses this + * file to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.index.search.geo; + +/** + */ +public class Point { + public double lat; + public double lon; + + public Point() { + } + + public Point(double lat, double lon) { + this.lat = lat; + this.lon = lon; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + + Point point = (Point) o; + + if (Double.compare(point.lat, lat) != 0) return false; + if (Double.compare(point.lon, lon) != 0) return false; + + return true; + } + + @Override + public int hashCode() { + int result; + long temp; + temp = lat != +0.0d ? Double.doubleToLongBits(lat) : 0L; + result = (int) (temp ^ (temp >>> 32)); + temp = lon != +0.0d ? Double.doubleToLongBits(lon) : 0L; + result = 31 * result + (int) (temp ^ (temp >>> 32)); + return result; + } + + public String toString() { + return "[" + lat + ", " + lon + "]"; + } +} diff --git a/modules/elasticsearch/src/test/java/org/elasticsearch/index/search/geo/GeoDistanceTests.java b/modules/elasticsearch/src/test/java/org/elasticsearch/index/search/geo/GeoDistanceTests.java new file mode 100644 index 00000000000..511940a8cf6 --- /dev/null +++ b/modules/elasticsearch/src/test/java/org/elasticsearch/index/search/geo/GeoDistanceTests.java @@ -0,0 +1,50 @@ +/* + * Licensed to Elastic Search and Shay Banon under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. Elastic Search licenses this + * file to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.index.search.geo; + +import org.elasticsearch.common.unit.DistanceUnit; +import org.testng.annotations.Test; + +import static org.hamcrest.MatcherAssert.*; +import static org.hamcrest.Matchers.*; + +/** + */ +@Test +public class GeoDistanceTests { + + @Test public void testDistanceCheck() { + // Note, is within is an approximation, so, even though 0.52 is outside 50mi, we still get "true" + GeoDistance.DistanceBoundingCheck check = GeoDistance.distanceBoundingCheck(0, 0, 50, DistanceUnit.MILES); + //System.out.println("Dist: " + GeoDistance.ARC.calculate(0, 0, 0.5, 0.5, DistanceUnit.MILES)); + assertThat(check.isWithin(0.5, 0.5), equalTo(true)); + //System.out.println("Dist: " + GeoDistance.ARC.calculate(0, 0, 0.52, 0.52, DistanceUnit.MILES)); + assertThat(check.isWithin(0.52, 0.52), equalTo(true)); + //System.out.println("Dist: " + GeoDistance.ARC.calculate(0, 0, 1, 1, DistanceUnit.MILES)); + assertThat(check.isWithin(1, 1), equalTo(false)); + + + check = GeoDistance.distanceBoundingCheck(0, 179, 200, DistanceUnit.MILES); + //System.out.println("Dist: " + GeoDistance.ARC.calculate(0, 179, 0, -179, DistanceUnit.MILES)); + assertThat(check.isWithin(0, -179), equalTo(true)); + //System.out.println("Dist: " + GeoDistance.ARC.calculate(0, 179, 0, -178, DistanceUnit.MILES)); + assertThat(check.isWithin(0, -178), equalTo(false)); + } +}