Browse Source

Adds support for geo-bounds filtering in geogrid aggregations (#50002)

It is fairly common to filter the geo point candidates in
geohash_grid and geotile_grid aggregations according to some
viewable bounding box. This change introduces the option of
specifying this filter directly in the tiling aggregation.

This is even more relevant to `geo_shape` where the bounds will restrict
the shape to be within the bounds

this optional `bounds` parameter is parsed in an equivalent fashion to 
the bounds specified in the geo_bounding_box query.
Tal Levy 5 years ago
parent
commit
6c86606d2a
28 changed files with 726 additions and 96 deletions
  1. 58 0
      docs/reference/aggregations/bucket/geohashgrid-aggregation.asciidoc
  2. 58 0
      docs/reference/aggregations/bucket/geotilegrid-aggregation.asciidoc
  3. 1 0
      docs/reference/query-dsl/geo-bounding-box-query.asciidoc
  4. 25 0
      server/src/main/java/org/elasticsearch/common/geo/GeoBoundingBox.java
  5. 1 0
      server/src/main/java/org/elasticsearch/common/geo/GeoUtils.java
  6. 28 3
      server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/GeoTileGridValuesSourceBuilder.java
  7. 49 0
      server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/BoundedCellValues.java
  8. 8 31
      server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/CellIdSource.java
  9. 68 0
      server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/CellValues.java
  10. 35 5
      server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoGridAggregationBuilder.java
  11. 2 2
      server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoGridAggregator.java
  12. 7 5
      server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoHashGridAggregationBuilder.java
  13. 5 3
      server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoHashGridAggregator.java
  14. 11 8
      server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoHashGridAggregatorFactory.java
  15. 6 6
      server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoTileGridAggregationBuilder.java
  16. 5 3
      server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoTileGridAggregator.java
  17. 11 8
      server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoTileGridAggregatorFactory.java
  18. 1 1
      server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoTileUtils.java
  19. 39 0
      server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/UnboundedCellValues.java
  20. 43 5
      server/src/test/java/org/elasticsearch/common/geo/GeoBoundingBoxTests.java
  21. 31 0
      server/src/test/java/org/elasticsearch/search/aggregations/bucket/GeoHashGridTests.java
  22. 76 0
      server/src/test/java/org/elasticsearch/search/aggregations/bucket/GeoTileGridTests.java
  23. 8 1
      server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregationBuilderTests.java
  24. 32 0
      server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/GeoTileGridValuesSourceBuilderTests.java
  25. 84 14
      server/src/test/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoGridAggregatorTestCase.java
  26. 17 0
      server/src/test/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoHashGridParserTests.java
  27. 0 1
      server/src/test/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoTileGridAggregatorTests.java
  28. 17 0
      server/src/test/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoTileGridParserTests.java

+ 58 - 0
docs/reference/aggregations/bucket/geohashgrid-aggregation.asciidoc

@@ -191,6 +191,62 @@ var bbox = geohash.decode_bbox('u17');
 --------------------------------------------------
 // NOTCONSOLE
 
+==== Requests with additional bounding box filtering
+
+The `geohash_grid` aggregation supports an optional `bounds` parameter
+that restricts the points considered to those that fall within the
+bounds provided. The `bounds` parameter accepts the bounding box in
+all the same <<query-dsl-geo-bounding-box-query-accepted-formats,accepted formats>> of the
+bounds specified in the Geo Bounding Box Query. This bounding box can be used with or
+without an additional `geo_bounding_box` query filtering the points prior to aggregating.
+It is an independent bounding box that can intersect with, be equal to, or be disjoint
+to any additional `geo_bounding_box` queries defined in the context of the aggregation.
+
+[source,console,id=geohashgrid-aggregation-with-bounds]
+--------------------------------------------------
+POST /museums/_search?size=0
+{
+    "aggregations" : {
+        "tiles-in-bounds" : {
+            "geohash_grid" : {
+                "field" : "location",
+                "precision" : 8,
+                "bounds": {
+                  "top_left" : "53.4375, 4.21875",
+                  "bottom_right" : "52.03125, 5.625"
+                }
+            }
+        }
+    }
+}
+--------------------------------------------------
+// TEST[continued]
+
+[source,console-result]
+--------------------------------------------------
+{
+    ...
+    "aggregations" : {
+        "tiles-in-bounds" : {
+           "buckets" : [
+               {
+                 "key" : "u173zy3j",
+                 "doc_count" : 1
+               },
+               {
+                 "key" : "u173zvfz",
+                 "doc_count" : 1
+               },
+               {
+                 "key" : "u173zt90",
+                 "doc_count" : 1
+               }
+           ]
+        }
+    }
+}
+--------------------------------------------------
+// TESTRESPONSE[s/\.\.\./"took": $body.took,"_shards": $body._shards,"hits":$body.hits,"timed_out":false,/]
 
 ==== Cell dimensions at the equator
 The table below shows the metric dimensions for cells covered by various string lengths of geohash.
@@ -230,6 +286,8 @@ precision::     Optional. The string length of the geohashes used to define
                 to precision levels higher than the supported 12 levels,
                 (e.g. for distances <5.6cm) the value is rejected.
 
+bounds:         Optional. The bounding box to filter the points in the bucket.
+
 size::          Optional. The maximum number of geohash buckets to return
                 (defaults to 10,000). When results are trimmed, buckets are
                 prioritised based on the volumes of documents they contain.

+ 58 - 0
docs/reference/aggregations/bucket/geotilegrid-aggregation.asciidoc

@@ -162,6 +162,62 @@ POST /museums/_search?size=0
 --------------------------------------------------
 // TESTRESPONSE[s/\.\.\./"took": $body.took,"_shards": $body._shards,"hits":$body.hits,"timed_out":false,/]
 
+==== Requests with additional bounding box filtering
+
+The `geotile_grid` aggregation supports an optional `bounds` parameter
+that restricts the points considered to those that fall within the
+bounds provided. The `bounds` parameter accepts the bounding box in
+all the same <<query-dsl-geo-bounding-box-query-accepted-formats,accepted formats>> of the
+bounds specified in the Geo Bounding Box Query. This bounding box can be used with or
+without an additional `geo_bounding_box` query filtering the points prior to aggregating.
+It is an independent bounding box that can intersect with, be equal to, or be disjoint
+to any additional `geo_bounding_box` queries defined in the context of the aggregation.
+
+[source,console,id=geotilegrid-aggregation-with-bounds]
+--------------------------------------------------
+POST /museums/_search?size=0
+{
+    "aggregations" : {
+        "tiles-in-bounds" : {
+            "geotile_grid" : {
+                "field" : "location",
+                "precision" : 22,
+                "bounds": {
+                  "top_left" : "52.4, 4.9",
+                  "bottom_right" : "52.3, 5.0"
+                }
+            }
+        }
+    }
+}
+--------------------------------------------------
+// TEST[continued]
+
+[source,console-result]
+--------------------------------------------------
+{
+    ...
+    "aggregations" : {
+        "tiles-in-bounds" : {
+           "buckets" : [
+               {
+                 "key" : "22/2154412/1378379",
+                 "doc_count" : 1
+               },
+               {
+                 "key" : "22/2154385/1378332",
+                 "doc_count" : 1
+               },
+               {
+                 "key" : "22/2154259/1378425",
+                 "doc_count" : 1
+               }
+           ]
+        }
+    }
+}
+--------------------------------------------------
+// TESTRESPONSE[s/\.\.\./"took": $body.took,"_shards": $body._shards,"hits":$body.hits,"timed_out":false,/]
 
 ==== Options
 
@@ -172,6 +228,8 @@ precision::     Optional. The integer zoom of the key used to define
                 cells/buckets in the results. Defaults to 7.
                 Values outside of [0,29] will be rejected.
 
+bounds:         Optional. The bounding box to filter the points in the bucket.
+
 size::          Optional. The maximum number of geohash buckets to return
                 (defaults to 10,000). When results are trimmed, buckets are
                 prioritised based on the volumes of documents they contain.

+ 1 - 0
docs/reference/query-dsl/geo-bounding-box-query.asciidoc

@@ -84,6 +84,7 @@ be executed in memory or indexed. See <<geo-bbox-type,Type>> below for further d
 Default is `memory`.
 |=======================================================================
 
+[[query-dsl-geo-bounding-box-query-accepted-formats]]
 [float]
 ==== Accepted Formats
 

+ 25 - 0
server/src/main/java/org/elasticsearch/common/geo/GeoBoundingBox.java

@@ -68,6 +68,11 @@ public class GeoBoundingBox implements ToXContentObject, Writeable {
         this.bottomRight = input.readGeoPoint();
     }
 
+    public boolean isUnbounded() {
+        return Double.isNaN(topLeft.lon()) || Double.isNaN(topLeft.lat())
+            || Double.isNaN(bottomRight.lon()) || Double.isNaN(bottomRight.lat());
+    }
+
     public GeoPoint topLeft() {
         return topLeft;
     }
@@ -120,6 +125,26 @@ public class GeoBoundingBox implements ToXContentObject, Writeable {
         return builder;
     }
 
+    /**
+     * If the bounding box crosses the date-line (left greater-than right) then the
+     * longitude of the point need only to be higher than the left or lower
+     * than the right. Otherwise, it must be both.
+     *
+     * @param lon the longitude of the point
+     * @param lat the latitude of the point
+     * @return whether the point (lon, lat) is in the specified bounding box
+     */
+    public boolean pointInBounds(double lon, double lat) {
+        if (lat >= bottom() && lat <= top()) {
+            if (left() <= right()) {
+                return lon >= left() && lon <= right();
+            } else {
+                return lon >= left() || lon <= right();
+            }
+        }
+        return false;
+    }
+
     @Override
     public void writeTo(StreamOutput out) throws IOException {
         out.writeGeoPoint(topLeft);

+ 1 - 0
server/src/main/java/org/elasticsearch/common/geo/GeoUtils.java

@@ -93,6 +93,7 @@ public class GeoUtils {
         return true;
     }
 
+
     /**
      * Calculate the width (in meters) of geohash cells at a specific level
      * @param level geohash level must be greater or equal to zero

+ 28 - 3
server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/GeoTileGridValuesSourceBuilder.java

@@ -19,7 +19,10 @@
 
 package org.elasticsearch.search.aggregations.bucket.composite;
 
+import org.elasticsearch.Version;
 import org.elasticsearch.common.ParseField;
+import org.elasticsearch.common.geo.GeoBoundingBox;
+import org.elasticsearch.common.geo.GeoPoint;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.xcontent.ObjectParser;
@@ -45,6 +48,8 @@ public class GeoTileGridValuesSourceBuilder extends CompositeValuesSourceBuilder
     static {
         PARSER = new ObjectParser<>(GeoTileGridValuesSourceBuilder.TYPE);
         PARSER.declareInt(GeoTileGridValuesSourceBuilder::precision, new ParseField("precision"));
+        PARSER.declareField(((p, builder, context) -> builder.geoBoundingBox(GeoBoundingBox.parseBoundingBox(p))),
+            GeoBoundingBox.BOUNDS_FIELD, ObjectParser.ValueType.OBJECT);
         CompositeValuesSourceParserHelper.declareValuesSourceFields(PARSER, ValueType.NUMERIC);
     }
 
@@ -53,6 +58,7 @@ public class GeoTileGridValuesSourceBuilder extends CompositeValuesSourceBuilder
     }
 
     private int precision = GeoTileGridAggregationBuilder.DEFAULT_PRECISION;
+    private GeoBoundingBox geoBoundingBox = new GeoBoundingBox(new GeoPoint(Double.NaN, Double.NaN), new GeoPoint(Double.NaN, Double.NaN));
 
     GeoTileGridValuesSourceBuilder(String name) {
         super(name);
@@ -61,6 +67,9 @@ public class GeoTileGridValuesSourceBuilder extends CompositeValuesSourceBuilder
     GeoTileGridValuesSourceBuilder(StreamInput in) throws IOException {
         super(in);
         this.precision = in.readInt();
+        if (in.getVersion().onOrAfter(Version.V_8_0_0)) {
+            this.geoBoundingBox = new GeoBoundingBox(in);
+        }
     }
 
     public GeoTileGridValuesSourceBuilder precision(int precision) {
@@ -68,6 +77,11 @@ public class GeoTileGridValuesSourceBuilder extends CompositeValuesSourceBuilder
         return this;
     }
 
+    public GeoTileGridValuesSourceBuilder geoBoundingBox(GeoBoundingBox geoBoundingBox) {
+        this.geoBoundingBox = geoBoundingBox;
+        return this;
+    }
+
     @Override
     public GeoTileGridValuesSourceBuilder format(String format) {
         throw new IllegalArgumentException("[format] is not supported for [" + TYPE + "]");
@@ -76,11 +90,17 @@ public class GeoTileGridValuesSourceBuilder extends CompositeValuesSourceBuilder
     @Override
     protected void innerWriteTo(StreamOutput out) throws IOException {
         out.writeInt(precision);
+        if (out.getVersion().onOrAfter(Version.V_8_0_0)) {
+            geoBoundingBox.writeTo(out);
+        }
     }
 
     @Override
     protected void doXContentBody(XContentBuilder builder, Params params) throws IOException {
         builder.field("precision", precision);
+        if (geoBoundingBox.isUnbounded() == false) {
+            geoBoundingBox.toXContent(builder, params);
+        }
     }
 
     @Override
@@ -88,9 +108,13 @@ public class GeoTileGridValuesSourceBuilder extends CompositeValuesSourceBuilder
         return TYPE;
     }
 
+    GeoBoundingBox geoBoundingBox() {
+        return geoBoundingBox;
+    }
+
     @Override
     public int hashCode() {
-        return Objects.hash(super.hashCode(), precision);
+        return Objects.hash(super.hashCode(), precision, geoBoundingBox);
     }
 
     @Override
@@ -99,7 +123,8 @@ public class GeoTileGridValuesSourceBuilder extends CompositeValuesSourceBuilder
         if (obj == null || getClass() != obj.getClass()) return false;
         if (super.equals(obj) == false) return false;
         GeoTileGridValuesSourceBuilder other = (GeoTileGridValuesSourceBuilder) obj;
-        return precision == other.precision;
+        return Objects.equals(precision,other.precision)
+            && Objects.equals(geoBoundingBox, other.geoBoundingBox);
     }
 
     @Override
@@ -112,7 +137,7 @@ public class GeoTileGridValuesSourceBuilder extends CompositeValuesSourceBuilder
             ValuesSource.GeoPoint geoPoint = (ValuesSource.GeoPoint) orig;
             // is specified in the builder.
             final MappedFieldType fieldType = config.fieldContext() != null ? config.fieldContext().fieldType() : null;
-            CellIdSource cellIdSource = new CellIdSource(geoPoint, precision, GeoTileUtils::longEncode);
+            CellIdSource cellIdSource = new CellIdSource(geoPoint, precision, geoBoundingBox, GeoTileUtils::longEncode);
             return new CompositeValuesSourceConfig(name, fieldType, cellIdSource, DocValueFormat.GEOTILE, order(),
                 missingBucket(), script() != null);
         } else {

+ 49 - 0
server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/BoundedCellValues.java

@@ -0,0 +1,49 @@
+/*
+ * Licensed to Elasticsearch under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch 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.search.aggregations.bucket.geogrid;
+
+import org.elasticsearch.common.geo.GeoBoundingBox;
+import org.elasticsearch.index.fielddata.MultiGeoPointValues;
+
+/**
+ * Class representing {@link CellValues} whose values are filtered
+ * according to whether they are within the specified {@link GeoBoundingBox}.
+ *
+ * The specified bounding box is assumed to be bounded.
+ */
+class BoundedCellValues extends CellValues {
+
+    private final GeoBoundingBox geoBoundingBox;
+
+    protected BoundedCellValues(MultiGeoPointValues geoValues, int precision, CellIdSource.GeoPointLongEncoder encoder,
+                                GeoBoundingBox geoBoundingBox) {
+        super(geoValues, precision, encoder);
+        this.geoBoundingBox = geoBoundingBox;
+    }
+
+
+    @Override
+    int advanceValue(org.elasticsearch.common.geo.GeoPoint target, int valuesIdx) {
+        if (geoBoundingBox.pointInBounds(target.getLon(), target.getLat())) {
+            values[valuesIdx] = encoder.encode(target.getLon(), target.getLat(), precision);
+            return valuesIdx + 1;
+        }
+        return valuesIdx;
+    }
+}

+ 8 - 31
server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/CellIdSource.java

@@ -20,14 +20,12 @@ package org.elasticsearch.search.aggregations.bucket.geogrid;
 
 import org.apache.lucene.index.LeafReaderContext;
 import org.apache.lucene.index.SortedNumericDocValues;
-import org.elasticsearch.index.fielddata.AbstractSortingNumericDocValues;
+import org.elasticsearch.common.geo.GeoBoundingBox;
 import org.elasticsearch.index.fielddata.MultiGeoPointValues;
 import org.elasticsearch.index.fielddata.SortedBinaryDocValues;
 import org.elasticsearch.index.fielddata.SortedNumericDoubleValues;
 import org.elasticsearch.search.aggregations.support.ValuesSource;
 
-import java.io.IOException;
-
 /**
  * Wrapper class to help convert {@link MultiGeoPointValues}
  * to numeric long values for bucketing.
@@ -36,11 +34,13 @@ public class CellIdSource extends ValuesSource.Numeric {
     private final ValuesSource.GeoPoint valuesSource;
     private final int precision;
     private final GeoPointLongEncoder encoder;
+    private final GeoBoundingBox geoBoundingBox;
 
-    public CellIdSource(GeoPoint valuesSource, int precision, GeoPointLongEncoder encoder) {
+    public CellIdSource(GeoPoint valuesSource,int precision, GeoBoundingBox geoBoundingBox, GeoPointLongEncoder encoder) {
         this.valuesSource = valuesSource;
         //different GeoPoints could map to the same or different hashing cells.
         this.precision = precision;
+        this.geoBoundingBox = geoBoundingBox;
         this.encoder = encoder;
     }
 
@@ -55,7 +55,10 @@ public class CellIdSource extends ValuesSource.Numeric {
 
     @Override
     public SortedNumericDocValues longValues(LeafReaderContext ctx) {
-        return new CellValues(valuesSource.geoPointValues(ctx), precision, encoder);
+        if (geoBoundingBox.isUnbounded()) {
+            return new UnboundedCellValues(valuesSource.geoPointValues(ctx), precision, encoder);
+        }
+        return new BoundedCellValues(valuesSource.geoPointValues(ctx), precision, encoder, geoBoundingBox);
     }
 
     @Override
@@ -77,30 +80,4 @@ public class CellIdSource extends ValuesSource.Numeric {
         long encode(double lon, double lat, int precision);
     }
 
-    private static class CellValues extends AbstractSortingNumericDocValues {
-        private MultiGeoPointValues geoValues;
-        private int precision;
-        private GeoPointLongEncoder encoder;
-
-        protected CellValues(MultiGeoPointValues geoValues, int precision, GeoPointLongEncoder encoder) {
-            this.geoValues = geoValues;
-            this.precision = precision;
-            this.encoder = encoder;
-        }
-
-        @Override
-        public boolean advanceExact(int docId) throws IOException {
-            if (geoValues.advanceExact(docId)) {
-                resize(geoValues.docValueCount());
-                for (int i = 0; i < docValueCount(); ++i) {
-                    org.elasticsearch.common.geo.GeoPoint target = geoValues.nextValue();
-                    values[i] = encoder.encode(target.getLon(), target.getLat(), precision);
-                }
-                sort();
-                return true;
-            } else {
-                return false;
-            }
-        }
-    }
 }

+ 68 - 0
server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/CellValues.java

@@ -0,0 +1,68 @@
+/*
+ * Licensed to Elasticsearch under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch 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.search.aggregations.bucket.geogrid;
+
+import org.elasticsearch.index.fielddata.AbstractSortingNumericDocValues;
+import org.elasticsearch.index.fielddata.MultiGeoPointValues;
+
+import java.io.IOException;
+
+/**
+ * Class representing the long-encoded grid-cells belonging to
+ * the geo-doc-values. Class must encode the values and then
+ * sort them in order to account for the cells correctly.
+ */
+abstract class CellValues extends AbstractSortingNumericDocValues {
+    private MultiGeoPointValues geoValues;
+    protected int precision;
+    protected CellIdSource.GeoPointLongEncoder encoder;
+
+    protected CellValues(MultiGeoPointValues geoValues, int precision, CellIdSource.GeoPointLongEncoder encoder) {
+        this.geoValues = geoValues;
+        this.precision = precision;
+        this.encoder = encoder;
+    }
+
+    @Override
+    public boolean advanceExact(int docId) throws IOException {
+        if (geoValues.advanceExact(docId)) {
+            int docValueCount = geoValues.docValueCount();
+            resize(docValueCount);
+            int j = 0;
+            for (int i = 0; i < docValueCount; i++) {
+                j = advanceValue(geoValues.nextValue(), j);
+            }
+            resize(j);
+            sort();
+            return true;
+        } else {
+            return false;
+        }
+    }
+
+    /**
+     * Sets the appropriate long-encoded value for <code>target</code>
+     * in <code>values</code>.
+     *
+     * @param target    the geo-value to encode
+     * @param valuesIdx the index into <code>values</code> to set
+     * @return          valuesIdx + 1 if value was set, valuesIdx otherwise.
+     */
+    abstract int advanceValue(org.elasticsearch.common.geo.GeoPoint target, int valuesIdx);
+}

+ 35 - 5
server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoGridAggregationBuilder.java

@@ -20,7 +20,10 @@
 package org.elasticsearch.search.aggregations.bucket.geogrid;
 
 import org.elasticsearch.ElasticsearchException;
+import org.elasticsearch.Version;
 import org.elasticsearch.common.ParseField;
+import org.elasticsearch.common.geo.GeoBoundingBox;
+import org.elasticsearch.common.geo.GeoPoint;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.xcontent.ObjectParser;
@@ -53,6 +56,8 @@ public abstract class GeoGridAggregationBuilder extends ValuesSourceAggregationB
     protected int precision;
     protected int requiredSize;
     protected int shardSize;
+    private GeoBoundingBox geoBoundingBox = new GeoBoundingBox(new GeoPoint(Double.NaN, Double.NaN), new GeoPoint(Double.NaN, Double.NaN));
+
 
     @FunctionalInterface
     protected interface PrecisionParser {
@@ -66,6 +71,10 @@ public abstract class GeoGridAggregationBuilder extends ValuesSourceAggregationB
             org.elasticsearch.common.xcontent.ObjectParser.ValueType.INT);
         parser.declareInt(GeoGridAggregationBuilder::size, FIELD_SIZE);
         parser.declareInt(GeoGridAggregationBuilder::shardSize, FIELD_SHARD_SIZE);
+        parser.declareField((p, builder, context) -> {
+                builder.setGeoBoundingBox(GeoBoundingBox.parseBoundingBox(p));
+            },
+            GeoBoundingBox.BOUNDS_FIELD, org.elasticsearch.common.xcontent.ObjectParser.ValueType.OBJECT);
         return parser;
     }
 
@@ -78,7 +87,7 @@ public abstract class GeoGridAggregationBuilder extends ValuesSourceAggregationB
         this.precision = clone.precision;
         this.requiredSize = clone.requiredSize;
         this.shardSize = clone.shardSize;
-
+        this.geoBoundingBox = clone.geoBoundingBox;
     }
 
     /**
@@ -89,6 +98,9 @@ public abstract class GeoGridAggregationBuilder extends ValuesSourceAggregationB
         precision = in.readVInt();
         requiredSize = in.readVInt();
         shardSize = in.readVInt();
+        if (in.getVersion().onOrAfter(Version.V_8_0_0)) {
+            geoBoundingBox = new GeoBoundingBox(in);
+        }
     }
 
     @Override
@@ -96,6 +108,9 @@ public abstract class GeoGridAggregationBuilder extends ValuesSourceAggregationB
         out.writeVInt(precision);
         out.writeVInt(requiredSize);
         out.writeVInt(shardSize);
+        if (out.getVersion().onOrAfter(Version.V_8_0_0)) {
+            geoBoundingBox.writeTo(out);
+        }
     }
 
     /**
@@ -110,7 +125,8 @@ public abstract class GeoGridAggregationBuilder extends ValuesSourceAggregationB
      */
     protected abstract ValuesSourceAggregatorFactory<ValuesSource.GeoPoint> createFactory(
         String name, ValuesSourceConfig<ValuesSource.GeoPoint> config, int precision, int requiredSize, int shardSize,
-        QueryShardContext queryShardContext, AggregatorFactory parent, Builder subFactoriesBuilder, Map<String, Object> metaData
+        GeoBoundingBox geoBoundingBox, QueryShardContext queryShardContext, AggregatorFactory parent,
+        Builder subFactoriesBuilder, Map<String, Object> metaData
     ) throws IOException;
 
     public int precision() {
@@ -143,6 +159,16 @@ public abstract class GeoGridAggregationBuilder extends ValuesSourceAggregationB
         return shardSize;
     }
 
+    public GeoGridAggregationBuilder setGeoBoundingBox(GeoBoundingBox geoBoundingBox) {
+        this.geoBoundingBox = geoBoundingBox;
+        // no validation done here, similar to geo_bounding_box query behavior.
+        return this;
+    }
+
+    public GeoBoundingBox geoBoundingBox() {
+        return geoBoundingBox;
+    }
+
     @Override
     protected ValuesSourceAggregatorFactory<ValuesSource.GeoPoint> innerBuild(QueryShardContext queryShardContext,
                                                                                 ValuesSourceConfig<ValuesSource.GeoPoint> config,
@@ -166,7 +192,7 @@ public abstract class GeoGridAggregationBuilder extends ValuesSourceAggregationB
         if (shardSize < requiredSize) {
             shardSize = requiredSize;
         }
-        return createFactory(name, config, precision, requiredSize, shardSize, queryShardContext, parent,
+        return createFactory(name, config, precision, requiredSize, shardSize, geoBoundingBox, queryShardContext, parent,
                 subFactoriesBuilder, metaData);
     }
 
@@ -177,6 +203,9 @@ public abstract class GeoGridAggregationBuilder extends ValuesSourceAggregationB
         if (shardSize > -1) {
             builder.field(FIELD_SHARD_SIZE.getPreferredName(), shardSize);
         }
+        if (geoBoundingBox.isUnbounded() == false) {
+            geoBoundingBox.toXContent(builder, params);
+        }
         return builder;
     }
 
@@ -188,11 +217,12 @@ public abstract class GeoGridAggregationBuilder extends ValuesSourceAggregationB
         GeoGridAggregationBuilder other = (GeoGridAggregationBuilder) obj;
         return precision == other.precision
             && requiredSize == other.requiredSize
-            && shardSize == other.shardSize;
+            && shardSize == other.shardSize
+            && Objects.equals(geoBoundingBox, other.geoBoundingBox);
     }
 
     @Override
     public int hashCode() {
-        return Objects.hash(super.hashCode(), precision, requiredSize, shardSize);
+        return Objects.hash(super.hashCode(), precision, requiredSize, shardSize, geoBoundingBox);
     }
 }

+ 2 - 2
server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoGridAggregator.java

@@ -48,8 +48,8 @@ public abstract class GeoGridAggregator<T extends InternalGeoGrid> extends Bucke
     protected final LongHash bucketOrds;
 
     GeoGridAggregator(String name, AggregatorFactories factories, CellIdSource valuesSource,
-                      int requiredSize, int shardSize, SearchContext aggregationContext, Aggregator parent,
-                      List<PipelineAggregator> pipelineAggregators, Map<String, Object> metaData) throws IOException {
+                      int requiredSize, int shardSize, SearchContext aggregationContext,
+                      Aggregator parent, List<PipelineAggregator> pipelineAggregators, Map<String, Object> metaData) throws IOException {
         super(name, factories, aggregationContext, parent, pipelineAggregators, metaData);
         this.valuesSource = valuesSource;
         this.requiredSize = requiredSize;

+ 7 - 5
server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoHashGridAggregationBuilder.java

@@ -19,6 +19,7 @@
 
 package org.elasticsearch.search.aggregations.bucket.geogrid;
 
+import org.elasticsearch.common.geo.GeoBoundingBox;
 import org.elasticsearch.common.geo.GeoUtils;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.xcontent.ObjectParser;
@@ -60,11 +61,12 @@ public class GeoHashGridAggregationBuilder extends GeoGridAggregationBuilder {
 
     @Override
     protected ValuesSourceAggregatorFactory<ValuesSource.GeoPoint> createFactory(
-        String name, ValuesSourceConfig<ValuesSource.GeoPoint> config, int precision, int requiredSize, int shardSize,
-        QueryShardContext queryShardContext, AggregatorFactory parent, AggregatorFactories.Builder subFactoriesBuilder,
-        Map<String, Object> metaData) throws IOException {
-        return new GeoHashGridAggregatorFactory(name, config, precision, requiredSize, shardSize, queryShardContext, parent,
-            subFactoriesBuilder, metaData);
+            String name, ValuesSourceConfig<ValuesSource.GeoPoint> config, int precision, int requiredSize, int shardSize,
+            GeoBoundingBox geoBoundingBox, QueryShardContext queryShardContext,
+            AggregatorFactory parent, AggregatorFactories.Builder subFactoriesBuilder,
+            Map<String, Object> metaData) throws IOException {
+        return new GeoHashGridAggregatorFactory(name, config, precision, requiredSize, shardSize, geoBoundingBox,
+            queryShardContext, parent, subFactoriesBuilder, metaData);
     }
 
     private GeoHashGridAggregationBuilder(GeoHashGridAggregationBuilder clone, AggregatorFactories.Builder factoriesBuilder,

+ 5 - 3
server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoHashGridAggregator.java

@@ -34,9 +34,11 @@ import java.util.Map;
 public class GeoHashGridAggregator extends GeoGridAggregator<InternalGeoHashGrid> {
 
     GeoHashGridAggregator(String name, AggregatorFactories factories, CellIdSource valuesSource,
-                          int requiredSize, int shardSize, SearchContext aggregationContext, Aggregator parent,
-                          List<PipelineAggregator> pipelineAggregators, Map<String, Object> metaData) throws IOException {
-        super(name, factories, valuesSource, requiredSize, shardSize, aggregationContext, parent, pipelineAggregators, metaData);
+                          int requiredSize, int shardSize, SearchContext aggregationContext,
+                          Aggregator parent, List<PipelineAggregator> pipelineAggregators,
+                          Map<String, Object> metaData) throws IOException {
+        super(name, factories, valuesSource, requiredSize, shardSize, aggregationContext, parent,
+            pipelineAggregators, metaData);
     }
 
     @Override

+ 11 - 8
server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoHashGridAggregatorFactory.java

@@ -19,6 +19,7 @@
 
 package org.elasticsearch.search.aggregations.bucket.geogrid;
 
+import org.elasticsearch.common.geo.GeoBoundingBox;
 import org.elasticsearch.geometry.utils.Geohash;
 import org.elasticsearch.index.query.QueryShardContext;
 import org.elasticsearch.search.aggregations.Aggregator;
@@ -28,7 +29,6 @@ import org.elasticsearch.search.aggregations.InternalAggregation;
 import org.elasticsearch.search.aggregations.NonCollectingAggregator;
 import org.elasticsearch.search.aggregations.pipeline.PipelineAggregator;
 import org.elasticsearch.search.aggregations.support.ValuesSource;
-import org.elasticsearch.search.aggregations.support.ValuesSource.GeoPoint;
 import org.elasticsearch.search.aggregations.support.ValuesSourceAggregatorFactory;
 import org.elasticsearch.search.aggregations.support.ValuesSourceConfig;
 import org.elasticsearch.search.internal.SearchContext;
@@ -43,14 +43,17 @@ public class GeoHashGridAggregatorFactory extends ValuesSourceAggregatorFactory<
     private final int precision;
     private final int requiredSize;
     private final int shardSize;
+    private final GeoBoundingBox geoBoundingBox;
 
-    GeoHashGridAggregatorFactory(String name, ValuesSourceConfig<GeoPoint> config, int precision, int requiredSize,
-                                 int shardSize, QueryShardContext queryShardContext, AggregatorFactory parent,
-                                 AggregatorFactories.Builder subFactoriesBuilder, Map<String, Object> metaData) throws IOException {
+    GeoHashGridAggregatorFactory(String name, ValuesSourceConfig<ValuesSource.GeoPoint> config, int precision, int requiredSize,
+                                 int shardSize, GeoBoundingBox geoBoundingBox, QueryShardContext queryShardContext,
+                                 AggregatorFactory parent, AggregatorFactories.Builder subFactoriesBuilder,
+                                 Map<String, Object> metaData) throws IOException {
         super(name, config, queryShardContext, parent, subFactoriesBuilder, metaData);
         this.precision = precision;
         this.requiredSize = requiredSize;
         this.shardSize = shardSize;
+        this.geoBoundingBox = geoBoundingBox;
     }
 
     @Override
@@ -69,7 +72,7 @@ public class GeoHashGridAggregatorFactory extends ValuesSourceAggregatorFactory<
     }
 
     @Override
-    protected Aggregator doCreateInternal(final GeoPoint valuesSource,
+    protected Aggregator doCreateInternal(final ValuesSource.GeoPoint valuesSource,
                                             SearchContext searchContext,
                                             Aggregator parent,
                                             boolean collectsFromSingleBucket,
@@ -78,8 +81,8 @@ public class GeoHashGridAggregatorFactory extends ValuesSourceAggregatorFactory<
         if (collectsFromSingleBucket == false) {
             return asMultiBucketAggregator(this, searchContext, parent);
         }
-        CellIdSource cellIdSource = new CellIdSource(valuesSource, precision, Geohash::longEncode);
-        return new GeoHashGridAggregator(name, factories, cellIdSource, requiredSize, shardSize, searchContext, parent,
-                pipelineAggregators, metaData);
+        CellIdSource cellIdSource = new CellIdSource(valuesSource, precision, geoBoundingBox, Geohash::longEncode);
+        return new GeoHashGridAggregator(name, factories, cellIdSource, requiredSize, shardSize,
+            searchContext, parent, pipelineAggregators, metaData);
     }
 }

+ 6 - 6
server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoTileGridAggregationBuilder.java

@@ -19,6 +19,7 @@
 
 package org.elasticsearch.search.aggregations.bucket.geogrid;
 
+import org.elasticsearch.common.geo.GeoBoundingBox;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.xcontent.ObjectParser;
 import org.elasticsearch.common.xcontent.XContentParser;
@@ -59,12 +60,11 @@ public class GeoTileGridAggregationBuilder extends GeoGridAggregationBuilder {
 
     @Override
     protected ValuesSourceAggregatorFactory<ValuesSource.GeoPoint> createFactory(
-        String name, ValuesSourceConfig<ValuesSource.GeoPoint> config, int precision, int requiredSize, int shardSize,
-        QueryShardContext queryShardContext, AggregatorFactory parent, AggregatorFactories.Builder subFactoriesBuilder,
-        Map<String, Object> metaData
-    ) throws IOException {
-        return new GeoTileGridAggregatorFactory(name, config, precision, requiredSize, shardSize, queryShardContext, parent,
-            subFactoriesBuilder, metaData);
+            String name, ValuesSourceConfig<ValuesSource.GeoPoint> config, int precision, int requiredSize, int shardSize,
+            GeoBoundingBox geoBoundingBox, QueryShardContext queryShardContext, AggregatorFactory parent,
+            AggregatorFactories.Builder subFactoriesBuilder, Map<String, Object> metaData ) throws IOException {
+        return new GeoTileGridAggregatorFactory(name, config, precision, requiredSize, shardSize, geoBoundingBox,
+            queryShardContext, parent, subFactoriesBuilder, metaData);
     }
 
     private GeoTileGridAggregationBuilder(GeoTileGridAggregationBuilder clone, AggregatorFactories.Builder factoriesBuilder,

+ 5 - 3
server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoTileGridAggregator.java

@@ -35,9 +35,11 @@ import java.util.Map;
 public class GeoTileGridAggregator extends GeoGridAggregator<InternalGeoTileGrid> {
 
     GeoTileGridAggregator(String name, AggregatorFactories factories, CellIdSource valuesSource,
-                          int requiredSize, int shardSize, SearchContext aggregationContext, Aggregator parent,
-                          List<PipelineAggregator> pipelineAggregators, Map<String, Object> metaData) throws IOException {
-        super(name, factories, valuesSource, requiredSize, shardSize, aggregationContext, parent, pipelineAggregators, metaData);
+                          int requiredSize, int shardSize, SearchContext aggregationContext,
+                          Aggregator parent, List<PipelineAggregator> pipelineAggregators,
+                          Map<String, Object> metaData) throws IOException {
+        super(name, factories, valuesSource, requiredSize, shardSize, aggregationContext, parent,
+            pipelineAggregators, metaData);
     }
 
     @Override

+ 11 - 8
server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoTileGridAggregatorFactory.java

@@ -19,6 +19,7 @@
 
 package org.elasticsearch.search.aggregations.bucket.geogrid;
 
+import org.elasticsearch.common.geo.GeoBoundingBox;
 import org.elasticsearch.index.query.QueryShardContext;
 import org.elasticsearch.search.aggregations.Aggregator;
 import org.elasticsearch.search.aggregations.AggregatorFactories;
@@ -27,7 +28,6 @@ import org.elasticsearch.search.aggregations.InternalAggregation;
 import org.elasticsearch.search.aggregations.NonCollectingAggregator;
 import org.elasticsearch.search.aggregations.pipeline.PipelineAggregator;
 import org.elasticsearch.search.aggregations.support.ValuesSource;
-import org.elasticsearch.search.aggregations.support.ValuesSource.GeoPoint;
 import org.elasticsearch.search.aggregations.support.ValuesSourceAggregatorFactory;
 import org.elasticsearch.search.aggregations.support.ValuesSourceConfig;
 import org.elasticsearch.search.internal.SearchContext;
@@ -42,14 +42,17 @@ public class GeoTileGridAggregatorFactory extends ValuesSourceAggregatorFactory<
     private final int precision;
     private final int requiredSize;
     private final int shardSize;
+    private final GeoBoundingBox geoBoundingBox;
 
-    GeoTileGridAggregatorFactory(String name, ValuesSourceConfig<GeoPoint> config, int precision, int requiredSize,
-                                 int shardSize, QueryShardContext queryShardContext, AggregatorFactory parent,
-                                 AggregatorFactories.Builder subFactoriesBuilder, Map<String, Object> metaData) throws IOException {
+    GeoTileGridAggregatorFactory(String name, ValuesSourceConfig<ValuesSource.GeoPoint> config, int precision, int requiredSize,
+                                 int shardSize, GeoBoundingBox geoBoundingBox, QueryShardContext queryShardContext,
+                                 AggregatorFactory parent, AggregatorFactories.Builder subFactoriesBuilder,
+                                 Map<String, Object> metaData) throws IOException {
         super(name, config, queryShardContext, parent, subFactoriesBuilder, metaData);
         this.precision = precision;
         this.requiredSize = requiredSize;
         this.shardSize = shardSize;
+        this.geoBoundingBox = geoBoundingBox;
     }
 
     @Override
@@ -68,7 +71,7 @@ public class GeoTileGridAggregatorFactory extends ValuesSourceAggregatorFactory<
     }
 
     @Override
-    protected Aggregator doCreateInternal(final GeoPoint valuesSource,
+    protected Aggregator doCreateInternal(final ValuesSource.GeoPoint valuesSource,
                                             SearchContext searchContext,
                                             Aggregator parent,
                                             boolean collectsFromSingleBucket,
@@ -77,8 +80,8 @@ public class GeoTileGridAggregatorFactory extends ValuesSourceAggregatorFactory<
         if (collectsFromSingleBucket == false) {
             return asMultiBucketAggregator(this, searchContext, parent);
         }
-        CellIdSource cellIdSource = new CellIdSource(valuesSource, precision, GeoTileUtils::longEncode);
-        return new GeoTileGridAggregator(name, factories, cellIdSource, requiredSize, shardSize, searchContext, parent,
-                pipelineAggregators, metaData);
+        CellIdSource cellIdSource = new CellIdSource(valuesSource, precision, geoBoundingBox, GeoTileUtils::longEncode);
+        return new GeoTileGridAggregator(name, factories, cellIdSource, requiredSize, shardSize,
+            searchContext, parent, pipelineAggregators, metaData);
     }
 }

+ 1 - 1
server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoTileUtils.java

@@ -51,7 +51,7 @@ public final class GeoTileUtils {
      * Another consideration is that index optimizes lat/lng storage, loosing some precision.
      * E.g. hash lng=140.74779717298918D lat=45.61884022447444D == "18/233561/93659", but shown as "18/233561/93658"
      */
-    static final int MAX_ZOOM = 29;
+    public static final int MAX_ZOOM = 29;
 
     /**
      * Bit position of the zoom value within hash - zoom is stored in the most significant 6 bits of a long number.

+ 39 - 0
server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/UnboundedCellValues.java

@@ -0,0 +1,39 @@
+/*
+ * Licensed to Elasticsearch under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch 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.search.aggregations.bucket.geogrid;
+
+import org.elasticsearch.common.geo.GeoBoundingBox;
+import org.elasticsearch.index.fielddata.MultiGeoPointValues;
+
+/**
+ * Class representing {@link CellValues} that are unbounded by any
+ * {@link GeoBoundingBox}.
+ */
+class UnboundedCellValues extends CellValues {
+
+    UnboundedCellValues(MultiGeoPointValues geoValues, int precision, CellIdSource.GeoPointLongEncoder encoder) {
+        super(geoValues, precision, encoder);
+    }
+
+    @Override
+    int advanceValue(org.elasticsearch.common.geo.GeoPoint target, int valuesIdx) {
+        values[valuesIdx] = encoder.encode(target.getLon(), target.getLat(), precision);
+        return valuesIdx + 1;
+    }
+}

+ 43 - 5
server/src/test/java/org/elasticsearch/common/geo/GeoBoundingBoxTests.java

@@ -19,11 +19,13 @@
 
 package org.elasticsearch.common.geo;
 
+import org.apache.lucene.geo.GeoEncodingUtils;
 import org.elasticsearch.ElasticsearchParseException;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentFactory;
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.geo.GeometryTestUtils;
+import org.elasticsearch.geometry.Rectangle;
 import org.elasticsearch.test.ESTestCase;
 
 import java.io.IOException;
@@ -133,6 +135,35 @@ public class GeoBoundingBoxTests extends ESTestCase {
         }
     }
 
+    public void testPointInBounds() {
+        for (int iter = 0; iter < 1000; iter++) {
+            GeoBoundingBox geoBoundingBox = randomBBox();
+            GeoBoundingBox bbox = new GeoBoundingBox(
+                new GeoPoint(quantizeLat(geoBoundingBox.top()), quantizeLon(geoBoundingBox.left())),
+                    new GeoPoint(quantizeLat(geoBoundingBox.bottom()), quantizeLon(geoBoundingBox.right())));
+            if (bbox.left() > bbox.right()) {
+                double lonWithin = randomBoolean() ?
+                    randomDoubleBetween(bbox.left(), 180.0, true)
+                    : randomDoubleBetween(-180.0, bbox.right(), true);
+                double latWithin = randomDoubleBetween(bbox.bottom(), bbox.top(), true);
+                double lonOutside = randomDoubleBetween(bbox.left(), bbox.right(), true);
+                double latOutside = randomBoolean() ? randomDoubleBetween(Math.max(bbox.top(), bbox.bottom()), 90, false)
+                    : randomDoubleBetween(-90, Math.min(bbox.bottom(), bbox.top()), false);
+
+                assertTrue(bbox.pointInBounds(lonWithin, latWithin));
+                    assertFalse(bbox.pointInBounds(lonOutside, latOutside));
+            } else {
+                double lonWithin = randomDoubleBetween(bbox.left(), bbox.right(), true);
+                double latWithin = randomDoubleBetween(bbox.bottom(), bbox.top(), true);
+                double lonOutside = GeoUtils.normalizeLon(randomDoubleBetween(bbox.right(), 180, false));
+                double latOutside = GeoUtils.normalizeLat(randomDoubleBetween(bbox.top(), 90, false));
+
+                assertTrue(bbox.pointInBounds(lonWithin, latWithin));
+                assertFalse(bbox.pointInBounds(lonOutside, latOutside));
+            }
+        }
+    }
+
     private void assertBBox(GeoBoundingBox expected, XContentBuilder builder) throws IOException {
         try (XContentParser parser = createParser(builder)) {
             parser.nextToken();
@@ -140,10 +171,17 @@ public class GeoBoundingBoxTests extends ESTestCase {
         }
     }
 
-    private GeoBoundingBox randomBBox() {
-        double topLat = GeometryTestUtils.randomLat();
-        double bottomLat = randomDoubleBetween(GeoUtils.MIN_LAT, topLat, true);
-        return new GeoBoundingBox(new GeoPoint(topLat, GeometryTestUtils.randomLon()),
-            new GeoPoint(bottomLat, GeometryTestUtils.randomLon()));
+    public static GeoBoundingBox randomBBox() {
+        Rectangle rectangle = GeometryTestUtils.randomRectangle();
+        return new GeoBoundingBox(new GeoPoint(rectangle.getMaxLat(), rectangle.getMinLon()),
+            new GeoPoint(rectangle.getMinLat(), rectangle.getMaxLon()));
+    }
+
+    private static double quantizeLat(double lat) {
+        return GeoEncodingUtils.decodeLatitude(GeoEncodingUtils.encodeLatitude(lat));
+    }
+
+    private static double quantizeLon(double lon) {
+        return GeoEncodingUtils.decodeLongitude(GeoEncodingUtils.encodeLongitude(lon));
     }
 }

+ 31 - 0
server/src/test/java/org/elasticsearch/search/aggregations/bucket/GeoHashGridTests.java

@@ -19,9 +19,22 @@
 
 package org.elasticsearch.search.aggregations.bucket;
 
+import org.elasticsearch.Version;
+import org.elasticsearch.common.geo.GeoBoundingBox;
+import org.elasticsearch.common.geo.GeoBoundingBoxTests;
+import org.elasticsearch.common.geo.GeoPoint;
+import org.elasticsearch.common.io.stream.BytesStreamOutput;
+import org.elasticsearch.common.io.stream.NamedWriteableAwareStreamInput;
+import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
+import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.search.aggregations.BaseAggregationTestCase;
 import org.elasticsearch.search.aggregations.bucket.geogrid.GeoGridAggregationBuilder;
 import org.elasticsearch.search.aggregations.bucket.geogrid.GeoHashGridAggregationBuilder;
+import org.elasticsearch.test.VersionUtils;
+
+import java.util.Collections;
+
+import static org.hamcrest.Matchers.equalTo;
 
 public class GeoHashGridTests extends BaseAggregationTestCase<GeoGridAggregationBuilder> {
 
@@ -39,7 +52,25 @@ public class GeoHashGridTests extends BaseAggregationTestCase<GeoGridAggregation
         if (randomBoolean()) {
             factory.shardSize(randomIntBetween(1, Integer.MAX_VALUE));
         }
+        if (randomBoolean()) {
+            factory.setGeoBoundingBox(GeoBoundingBoxTests.randomBBox());
+        }
         return factory;
     }
 
+    public void testSerializationPreBounds() throws Exception {
+        Version noBoundsSupportVersion = VersionUtils.randomPreviousCompatibleVersion(random(), Version.V_8_0_0);
+        GeoHashGridAggregationBuilder builder = createTestAggregatorBuilder();
+        try (BytesStreamOutput output = new BytesStreamOutput()) {
+            output.setVersion(Version.V_8_0_0);
+            builder.writeTo(output);
+            try (StreamInput in = new NamedWriteableAwareStreamInput(output.bytes().streamInput(),
+                new NamedWriteableRegistry(Collections.emptyList()))) {
+                in.setVersion(noBoundsSupportVersion);
+                GeoHashGridAggregationBuilder readBuilder = new GeoHashGridAggregationBuilder(in);
+                assertThat(readBuilder.geoBoundingBox(), equalTo(new GeoBoundingBox(
+                    new GeoPoint(Double.NaN, Double.NaN), new GeoPoint(Double.NaN, Double.NaN))));
+            }
+        }
+    }
 }

+ 76 - 0
server/src/test/java/org/elasticsearch/search/aggregations/bucket/GeoTileGridTests.java

@@ -0,0 +1,76 @@
+/*
+ * Licensed to Elasticsearch under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch 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.search.aggregations.bucket;
+
+import org.elasticsearch.Version;
+import org.elasticsearch.common.geo.GeoBoundingBox;
+import org.elasticsearch.common.geo.GeoBoundingBoxTests;
+import org.elasticsearch.common.geo.GeoPoint;
+import org.elasticsearch.common.io.stream.BytesStreamOutput;
+import org.elasticsearch.common.io.stream.NamedWriteableAwareStreamInput;
+import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
+import org.elasticsearch.common.io.stream.StreamInput;
+import org.elasticsearch.search.aggregations.BaseAggregationTestCase;
+import org.elasticsearch.search.aggregations.bucket.geogrid.GeoGridAggregationBuilder;
+import org.elasticsearch.search.aggregations.bucket.geogrid.GeoTileGridAggregationBuilder;
+import org.elasticsearch.search.aggregations.bucket.geogrid.GeoTileUtils;
+import org.elasticsearch.test.VersionUtils;
+
+import java.util.Collections;
+
+import static org.hamcrest.Matchers.equalTo;
+
+public class GeoTileGridTests extends BaseAggregationTestCase<GeoGridAggregationBuilder> {
+
+    @Override
+    protected GeoTileGridAggregationBuilder createTestAggregatorBuilder() {
+        String name = randomAlphaOfLengthBetween(3, 20);
+        GeoTileGridAggregationBuilder factory = new GeoTileGridAggregationBuilder(name);
+        if (randomBoolean()) {
+            factory.precision(randomIntBetween(0, GeoTileUtils.MAX_ZOOM));
+        }
+        if (randomBoolean()) {
+            factory.size(randomIntBetween(1, Integer.MAX_VALUE));
+        }
+        if (randomBoolean()) {
+            factory.shardSize(randomIntBetween(1, Integer.MAX_VALUE));
+        }
+        if (randomBoolean()) {
+            factory.setGeoBoundingBox(GeoBoundingBoxTests.randomBBox());
+        }
+        return factory;
+    }
+
+    public void testSerializationPreBounds() throws Exception {
+        Version noBoundsSupportVersion = VersionUtils.randomPreviousCompatibleVersion(random(), Version.V_8_0_0);
+        GeoTileGridAggregationBuilder builder = createTestAggregatorBuilder();
+        try (BytesStreamOutput output = new BytesStreamOutput()) {
+            output.setVersion(Version.V_8_0_0);
+            builder.writeTo(output);
+            try (StreamInput in = new NamedWriteableAwareStreamInput(output.bytes().streamInput(),
+                new NamedWriteableRegistry(Collections.emptyList()))) {
+                in.setVersion(noBoundsSupportVersion);
+                GeoTileGridAggregationBuilder readBuilder = new GeoTileGridAggregationBuilder(in);
+                assertThat(readBuilder.geoBoundingBox(), equalTo(new GeoBoundingBox(
+                    new GeoPoint(Double.NaN, Double.NaN), new GeoPoint(Double.NaN, Double.NaN))));
+            }
+        }
+    }
+}

+ 8 - 1
server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregationBuilderTests.java

@@ -19,8 +19,10 @@
 
 package org.elasticsearch.search.aggregations.bucket.composite;
 
+import org.elasticsearch.common.geo.GeoBoundingBoxTests;
 import org.elasticsearch.script.Script;
 import org.elasticsearch.search.aggregations.BaseAggregationTestCase;
+import org.elasticsearch.search.aggregations.bucket.geogrid.GeoTileUtils;
 import org.elasticsearch.search.aggregations.bucket.histogram.DateHistogramInterval;
 import org.elasticsearch.search.sort.SortOrder;
 
@@ -54,7 +56,10 @@ public class CompositeAggregationBuilderTests extends BaseAggregationTestCase<Co
     private GeoTileGridValuesSourceBuilder randomGeoTileGridValuesSourceBuilder() {
         GeoTileGridValuesSourceBuilder geoTile = new GeoTileGridValuesSourceBuilder(randomAlphaOfLengthBetween(5, 10));
         if (randomBoolean()) {
-            geoTile.precision(randomIntBetween(1, 12));
+            geoTile.precision(randomIntBetween(0, GeoTileUtils.MAX_ZOOM));
+        }
+        if (randomBoolean()) {
+            geoTile.geoBoundingBox(GeoBoundingBoxTests.randomBBox());
         }
         return geoTile;
     }
@@ -90,9 +95,11 @@ public class CompositeAggregationBuilderTests extends BaseAggregationTestCase<Co
     @Override
     protected CompositeAggregationBuilder createTestAggregatorBuilder() {
         int numSources = randomIntBetween(1, 10);
+        numSources = 1;
         List<CompositeValuesSourceBuilder<?>> sources = new ArrayList<>();
         for (int i = 0; i < numSources; i++) {
             int type = randomIntBetween(0, 3);
+            type = 3;
             switch (type) {
                 case 0:
                     sources.add(randomTermsSourceBuilder());

+ 32 - 0
server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/GeoTileGridValuesSourceBuilderTests.java

@@ -19,7 +19,21 @@
 
 package org.elasticsearch.search.aggregations.bucket.composite;
 
+import org.elasticsearch.Version;
+import org.elasticsearch.common.geo.GeoBoundingBox;
+import org.elasticsearch.common.geo.GeoBoundingBoxTests;
+import org.elasticsearch.common.geo.GeoPoint;
+import org.elasticsearch.common.io.stream.BytesStreamOutput;
+import org.elasticsearch.common.io.stream.NamedWriteableAwareStreamInput;
+import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
+import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.test.ESTestCase;
+import org.elasticsearch.test.VersionUtils;
+
+import java.io.IOException;
+import java.util.Collections;
+
+import static org.hamcrest.Matchers.equalTo;
 
 public class GeoTileGridValuesSourceBuilderTests extends ESTestCase {
 
@@ -28,4 +42,22 @@ public class GeoTileGridValuesSourceBuilderTests extends ESTestCase {
         expectThrows(IllegalArgumentException.class, () -> builder.format("format"));
     }
 
+    public void testBWCBounds() throws IOException {
+        Version noBoundsSupportVersion = VersionUtils.randomPreviousCompatibleVersion(random(), Version.V_8_0_0);
+        GeoTileGridValuesSourceBuilder builder = new GeoTileGridValuesSourceBuilder("name");
+        if (randomBoolean()) {
+            builder.geoBoundingBox(GeoBoundingBoxTests.randomBBox());
+        }
+        try (BytesStreamOutput output = new BytesStreamOutput()) {
+            output.setVersion(Version.V_8_0_0);
+            builder.writeTo(output);
+            try (StreamInput in = new NamedWriteableAwareStreamInput(output.bytes().streamInput(),
+                new NamedWriteableRegistry(Collections.emptyList()))) {
+                in.setVersion(noBoundsSupportVersion);
+                GeoTileGridValuesSourceBuilder readBuilder = new GeoTileGridValuesSourceBuilder(in);
+                assertThat(readBuilder.geoBoundingBox(), equalTo(new GeoBoundingBox(
+                    new GeoPoint(Double.NaN, Double.NaN), new GeoPoint(Double.NaN, Double.NaN))));
+            }
+        }
+    }
 }

+ 84 - 14
server/src/test/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoGridAggregatorTestCase.java

@@ -28,6 +28,9 @@ import org.apache.lucene.search.MatchAllDocsQuery;
 import org.apache.lucene.search.Query;
 import org.apache.lucene.store.Directory;
 import org.elasticsearch.common.CheckedConsumer;
+import org.elasticsearch.common.geo.GeoBoundingBox;
+import org.elasticsearch.common.geo.GeoBoundingBoxTests;
+import org.elasticsearch.common.geo.GeoUtils;
 import org.elasticsearch.index.mapper.GeoPointFieldMapper;
 import org.elasticsearch.index.mapper.MappedFieldType;
 import org.elasticsearch.search.aggregations.Aggregator;
@@ -44,6 +47,8 @@ import java.util.Map;
 import java.util.Set;
 import java.util.function.Consumer;
 
+import static org.hamcrest.Matchers.equalTo;
+
 public abstract class GeoGridAggregatorTestCase<T extends InternalGeoGridBucket> extends AggregatorTestCase {
 
     private static final String FIELD_NAME = "location";
@@ -64,18 +69,18 @@ public abstract class GeoGridAggregatorTestCase<T extends InternalGeoGridBucket>
     protected abstract GeoGridAggregationBuilder createBuilder(String name);
 
     public void testNoDocs() throws IOException {
-        testCase(new MatchAllDocsQuery(), FIELD_NAME, randomPrecision(), iw -> {
-            // Intentionally not writing any docs
-        }, geoGrid -> {
+        testCase(new MatchAllDocsQuery(), FIELD_NAME, randomPrecision(), null, geoGrid -> {
             assertEquals(0, geoGrid.getBuckets().size());
+        }, iw -> {
+            // Intentionally not writing any docs
         });
     }
 
     public void testFieldMissing() throws IOException {
-        testCase(new MatchAllDocsQuery(), "wrong_field", randomPrecision(), iw -> {
-            iw.addDocument(Collections.singleton(new LatLonDocValuesField(FIELD_NAME, 10D, 10D)));
-        }, geoGrid -> {
+        testCase(new MatchAllDocsQuery(), "wrong_field", randomPrecision(), null, geoGrid -> {
             assertEquals(0, geoGrid.getBuckets().size());
+        }, iw -> {
+            iw.addDocument(Collections.singleton(new LatLonDocValuesField(FIELD_NAME, 10D, 10D)));
         });
     }
 
@@ -83,7 +88,13 @@ public abstract class GeoGridAggregatorTestCase<T extends InternalGeoGridBucket>
         int precision = randomPrecision();
         int numPoints = randomIntBetween(8, 128);
         Map<String, Integer> expectedCountPerGeoHash = new HashMap<>();
-        testCase(new MatchAllDocsQuery(), FIELD_NAME, precision, iw -> {
+        testCase(new MatchAllDocsQuery(), FIELD_NAME, precision, null, geoHashGrid -> {
+            assertEquals(expectedCountPerGeoHash.size(), geoHashGrid.getBuckets().size());
+            for (GeoGrid.Bucket bucket : geoHashGrid.getBuckets()) {
+                assertEquals((long) expectedCountPerGeoHash.get(bucket.getKeyAsString()), bucket.getDocCount());
+            }
+            assertTrue(AggregationInspectionHelper.hasValue(geoHashGrid));
+        }, iw -> {
             List<LatLonDocValuesField> points = new ArrayList<>();
             Set<String> distinctHashesPerDoc = new HashSet<>();
             for (int pointId = 0; pointId < numPoints; pointId++) {
@@ -112,17 +123,71 @@ public abstract class GeoGridAggregatorTestCase<T extends InternalGeoGridBucket>
             if (points.size() != 0) {
                 iw.addDocument(points);
             }
-        }, geoHashGrid -> {
-            assertEquals(expectedCountPerGeoHash.size(), geoHashGrid.getBuckets().size());
-            for (GeoGrid.Bucket bucket : geoHashGrid.getBuckets()) {
-                assertEquals((long) expectedCountPerGeoHash.get(bucket.getKeyAsString()), bucket.getDocCount());
+        });
+    }
+
+    public void testBounds() throws IOException {
+        final int numDocs = randomIntBetween(64, 256);
+        final GeoGridAggregationBuilder builder = createBuilder("_name");
+
+        expectThrows(IllegalArgumentException.class, () -> builder.precision(-1));
+        expectThrows(IllegalArgumentException.class, () -> builder.precision(30));
+
+        GeoBoundingBox bbox = GeoBoundingBoxTests.randomBBox();
+
+        int in = 0, out = 0;
+        List<LatLonDocValuesField> docs = new ArrayList<>();
+        while (in + out < numDocs) {
+            if (bbox.left() > bbox.right()) {
+                if (randomBoolean()) {
+                    double lonWithin = randomBoolean() ?
+                        randomDoubleBetween(bbox.left(), 180.0, true)
+                        : randomDoubleBetween(-180.0, bbox.right(), true);
+                    double latWithin = randomDoubleBetween(bbox.bottom(), bbox.top(), true);
+                    in++;
+                    docs.add(new LatLonDocValuesField(FIELD_NAME, latWithin, lonWithin));
+                } else {
+                    double lonOutside = randomDoubleBetween(bbox.left(), bbox.right(), true);
+                    double latOutside = randomDoubleBetween(bbox.top(), -90, false);
+                    out++;
+                    docs.add(new LatLonDocValuesField(FIELD_NAME, latOutside, lonOutside));
+                }
+            } else {
+                if (randomBoolean()) {
+                    double lonWithin = randomDoubleBetween(bbox.left(), bbox.right(), true);
+                    double latWithin = randomDoubleBetween(bbox.bottom(), bbox.top(), true);
+                    in++;
+                    docs.add(new LatLonDocValuesField(FIELD_NAME, latWithin, lonWithin));
+                } else {
+                    double lonOutside = GeoUtils.normalizeLon(randomDoubleBetween(bbox.right(), 180.001, false));
+                    double latOutside = GeoUtils.normalizeLat(randomDoubleBetween(bbox.top(), 90.001, false));
+                    out++;
+                    docs.add(new LatLonDocValuesField(FIELD_NAME, latOutside, lonOutside));
+                }
+            }
+
+        }
+
+        final long numDocsInBucket = in;
+        final int precision = randomPrecision();
+
+        testCase(new MatchAllDocsQuery(), FIELD_NAME, precision, bbox, geoGrid -> {
+            assertTrue(AggregationInspectionHelper.hasValue(geoGrid));
+            long docCount = 0;
+            for (int i = 0; i < geoGrid.getBuckets().size(); i++) {
+                docCount += geoGrid.getBuckets().get(i).getDocCount();
+            }
+            assertThat(docCount, equalTo(numDocsInBucket));
+        }, iw -> {
+            for (LatLonDocValuesField docField : docs) {
+                iw.addDocument(Collections.singletonList(docField));
             }
-            assertTrue(AggregationInspectionHelper.hasValue(geoHashGrid));
         });
     }
 
-    private void testCase(Query query, String field, int precision, CheckedConsumer<RandomIndexWriter, IOException> buildIndex,
-                          Consumer<InternalGeoGrid<T>> verify) throws IOException {
+    private void testCase(Query query, String field, int precision, GeoBoundingBox geoBoundingBox,
+                          Consumer<InternalGeoGrid<T>> verify,
+                          CheckedConsumer<RandomIndexWriter, IOException> buildIndex) throws IOException {
         Directory directory = newDirectory();
         RandomIndexWriter indexWriter = new RandomIndexWriter(random(), directory);
         buildIndex.accept(indexWriter);
@@ -133,6 +198,11 @@ public abstract class GeoGridAggregatorTestCase<T extends InternalGeoGridBucket>
 
         GeoGridAggregationBuilder aggregationBuilder = createBuilder("_name").field(field);
         aggregationBuilder.precision(precision);
+        if (geoBoundingBox != null) {
+            aggregationBuilder.setGeoBoundingBox(geoBoundingBox);
+            assertThat(aggregationBuilder.geoBoundingBox(), equalTo(geoBoundingBox));
+        }
+
         MappedFieldType fieldType = new GeoPointFieldMapper.GeoPointFieldType();
         fieldType.setHasDocValues(true);
         fieldType.setName(FIELD_NAME);

+ 17 - 0
server/src/test/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoHashGridParserTests.java

@@ -22,6 +22,8 @@ import org.elasticsearch.common.unit.DistanceUnit;
 import org.elasticsearch.common.xcontent.XContentParseException;
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.common.xcontent.json.JsonXContent;
+import org.elasticsearch.geo.GeometryTestUtils;
+import org.elasticsearch.geometry.Rectangle;
 import org.elasticsearch.test.ESTestCase;
 
 import static org.hamcrest.Matchers.containsString;
@@ -113,4 +115,19 @@ public class GeoHashGridParserTests extends ESTestCase {
             assertEquals("Invalid geohash aggregation precision of 13. Must be between 1 and 12.", ex.getCause().getMessage());
         }
     }
+
+    public void testParseValidBounds() throws Exception {
+        Rectangle bbox = GeometryTestUtils.randomRectangle();
+        XContentParser stParser = createParser(JsonXContent.jsonXContent,
+            "{\"field\":\"my_loc\", \"precision\": 5, \"size\": 500, \"shard_size\": 550," + "\"bounds\": { "
+                + "\"top\": " + bbox.getMaxY() + ","
+                + "\"bottom\": " + bbox.getMinY() + ","
+                + "\"left\": " + bbox.getMinX() + ","
+                + "\"right\": " + bbox.getMaxX() + "}"
+                + "}");
+        XContentParser.Token token = stParser.nextToken();
+        assertSame(XContentParser.Token.START_OBJECT, token);
+        // can create a factory
+        assertNotNull(GeoHashGridAggregationBuilder.parse("geohash_grid", stParser));
+    }
 }

+ 0 - 1
server/src/test/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoTileGridAggregatorTests.java

@@ -46,5 +46,4 @@ public class GeoTileGridAggregatorTests extends GeoGridAggregatorTestCase<Intern
         builder.precision(precision);
         assertEquals(precision, builder.precision());
     }
-
 }

+ 17 - 0
server/src/test/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoTileGridParserTests.java

@@ -22,6 +22,8 @@ import org.elasticsearch.ExceptionsHelper;
 import org.elasticsearch.common.xcontent.XContentParseException;
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.common.xcontent.json.JsonXContent;
+import org.elasticsearch.geo.GeometryTestUtils;
+import org.elasticsearch.geometry.Rectangle;
 import org.elasticsearch.test.ESTestCase;
 
 import static org.hamcrest.Matchers.containsString;
@@ -70,4 +72,19 @@ public class GeoTileGridParserTests extends ESTestCase {
             assertEquals("Invalid geotile_grid precision of 30. Must be between 0 and 29.", ex.getCause().getMessage());
         }
     }
+
+    public void testParseValidBounds() throws Exception {
+        Rectangle bbox = GeometryTestUtils.randomRectangle();
+        XContentParser stParser = createParser(JsonXContent.jsonXContent,
+            "{\"field\":\"my_loc\", \"precision\": 5, \"size\": 500, \"shard_size\": 550," + "\"bounds\": { "
+                + "\"top\": " + bbox.getMaxY() + ","
+                + "\"bottom\": " + bbox.getMinY() + ","
+                + "\"left\": " + bbox.getMinX() + ","
+                + "\"right\": " + bbox.getMaxX() + "}"
+                + "}");
+        XContentParser.Token token = stParser.nextToken();
+        assertSame(XContentParser.Token.START_OBJECT, token);
+        // can create a factory
+        assertNotNull(GeoTileGridAggregationBuilder.parse("geotile_grid", stParser));
+    }
 }