From f953e1a67414000d967606d1c8b35f5bf06826b1 Mon Sep 17 00:00:00 2001 From: Tal Levy Date: Tue, 14 Apr 2020 20:14:02 -0700 Subject: [PATCH 1/6] add geo_bounds for shapes --- .../metrics/GeoBoundsAggregator.java | 18 +- .../metrics/GeoBoundsAggregatorSupplier.java | 2 +- .../metrics/InternalGeoBounds.java | 14 +- .../xpack/spatial/SpatialPlugin.java | 19 ++ .../metrics/GeoShapeBoundsAggregator.java | 83 +++++++ .../index/mapper/GeoShapeValuesSource.java | 2 +- .../mapper/GeoShapeValuesSourceType.java | 4 +- .../GeoShapeWithDocValuesFieldMapper.java | 6 + .../GeoShapeBoundsAggregatorTests.java | 231 ++++++++++++++++++ 9 files changed, 359 insertions(+), 20 deletions(-) create mode 100644 x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/aggregations/metrics/GeoShapeBoundsAggregator.java create mode 100644 x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/aggregations/metrics/GeoShapeBoundsAggregatorTests.java diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/GeoBoundsAggregator.java b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/GeoBoundsAggregator.java index 9a76873aaeaab..dfde53a719de7 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/GeoBoundsAggregator.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/GeoBoundsAggregator.java @@ -36,20 +36,20 @@ import java.io.IOException; import java.util.Map; -final class GeoBoundsAggregator extends MetricsAggregator { +public class GeoBoundsAggregator extends MetricsAggregator { - static final ParseField WRAP_LONGITUDE_FIELD = new ParseField("wrap_longitude"); + public static final ParseField WRAP_LONGITUDE_FIELD = new ParseField("wrap_longitude"); private final ValuesSource.GeoPoint valuesSource; private final boolean wrapLongitude; - DoubleArray tops; - DoubleArray bottoms; - DoubleArray posLefts; - DoubleArray posRights; - DoubleArray negLefts; - DoubleArray negRights; + protected DoubleArray tops; + protected DoubleArray bottoms; + protected DoubleArray posLefts; + protected DoubleArray posRights; + protected DoubleArray negLefts; + protected DoubleArray negRights; - GeoBoundsAggregator(String name, SearchContext aggregationContext, Aggregator parent, + public GeoBoundsAggregator(String name, SearchContext aggregationContext, Aggregator parent, ValuesSource.GeoPoint valuesSource, boolean wrapLongitude, Map metadata) throws IOException { super(name, aggregationContext, parent, metadata); this.valuesSource = valuesSource; diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/GeoBoundsAggregatorSupplier.java b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/GeoBoundsAggregatorSupplier.java index c6e8208967b7b..a580aab89b4b3 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/GeoBoundsAggregatorSupplier.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/GeoBoundsAggregatorSupplier.java @@ -30,6 +30,6 @@ @FunctionalInterface public interface GeoBoundsAggregatorSupplier extends AggregatorSupplier { - GeoBoundsAggregator build(String name, SearchContext aggregationContext, Aggregator parent, + MetricsAggregator build(String name, SearchContext aggregationContext, Aggregator parent, ValuesSource valuesSource, boolean wrapLongitude, Map metadata) throws IOException; } diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/InternalGeoBounds.java b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/InternalGeoBounds.java index 6e967b96ce90b..4bdac7f32afff 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/InternalGeoBounds.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/InternalGeoBounds.java @@ -32,13 +32,13 @@ import java.util.Objects; public class InternalGeoBounds extends InternalAggregation implements GeoBounds { - final double top; - final double bottom; - final double posLeft; - final double posRight; - final double negLeft; - final double negRight; - final boolean wrapLongitude; + public final double top; + public final double bottom; + public final double posLeft; + public final double posRight; + public final double negLeft; + public final double negRight; + public final boolean wrapLongitude; InternalGeoBounds(String name, double top, double bottom, double posLeft, double posRight, double negLeft, double negRight, boolean wrapLongitude, Map metadata) { diff --git a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/SpatialPlugin.java b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/SpatialPlugin.java index 07a3d6ebb0973..40d3c5dd06788 100644 --- a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/SpatialPlugin.java +++ b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/SpatialPlugin.java @@ -14,8 +14,14 @@ import org.elasticsearch.plugins.IngestPlugin; import org.elasticsearch.plugins.MapperPlugin; import org.elasticsearch.plugins.SearchPlugin; +import org.elasticsearch.search.aggregations.metrics.GeoBoundsAggregationBuilder; +import org.elasticsearch.search.aggregations.metrics.GeoBoundsAggregatorSupplier; +import org.elasticsearch.search.aggregations.support.ValuesSourceRegistry; import org.elasticsearch.xpack.core.action.XPackInfoFeatureAction; import org.elasticsearch.xpack.core.action.XPackUsageFeatureAction; +import org.elasticsearch.xpack.spatial.aggregations.metrics.GeoShapeBoundsAggregator; +import org.elasticsearch.xpack.spatial.index.mapper.GeoShapeValuesSource; +import org.elasticsearch.xpack.spatial.index.mapper.GeoShapeValuesSourceType; import org.elasticsearch.xpack.spatial.index.mapper.GeoShapeWithDocValuesFieldMapper; import org.elasticsearch.xpack.spatial.index.mapper.PointFieldMapper; import org.elasticsearch.xpack.spatial.index.mapper.ShapeFieldMapper; @@ -27,6 +33,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.function.Consumer; import static java.util.Collections.singletonList; @@ -53,8 +60,20 @@ public List> getQueries() { return singletonList(new QuerySpec<>(ShapeQueryBuilder.NAME, ShapeQueryBuilder::new, ShapeQueryBuilder::fromXContent)); } + @Override + public List> getBareAggregatorRegistrar() { + return List.of(SpatialPlugin::registerGeoShapeBoundsAggregator); + } + @Override public Map getProcessors(Processor.Parameters parameters) { return Map.of(CircleProcessor.TYPE, new CircleProcessor.Factory()); } + + public static void registerGeoShapeBoundsAggregator(ValuesSourceRegistry valuesSourceRegistry) { + valuesSourceRegistry.register(GeoBoundsAggregationBuilder.NAME, GeoShapeValuesSourceType.INSTANCE, + (GeoBoundsAggregatorSupplier) (name, aggregationContext, parent, valuesSource, wrapLongitude, metadata) + -> new GeoShapeBoundsAggregator(name, aggregationContext, parent, (GeoShapeValuesSource) valuesSource, + wrapLongitude, metadata)); + } } diff --git a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/aggregations/metrics/GeoShapeBoundsAggregator.java b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/aggregations/metrics/GeoShapeBoundsAggregator.java new file mode 100644 index 0000000000000..3da126b4524d4 --- /dev/null +++ b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/aggregations/metrics/GeoShapeBoundsAggregator.java @@ -0,0 +1,83 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +package org.elasticsearch.xpack.spatial.aggregations.metrics; + +import org.apache.lucene.index.LeafReaderContext; +import org.elasticsearch.common.util.BigArrays; +import org.elasticsearch.search.aggregations.Aggregator; +import org.elasticsearch.search.aggregations.LeafBucketCollector; +import org.elasticsearch.search.aggregations.LeafBucketCollectorBase; +import org.elasticsearch.search.aggregations.metrics.GeoBoundsAggregator; +import org.elasticsearch.search.aggregations.support.ValuesSource; +import org.elasticsearch.search.internal.SearchContext; +import org.elasticsearch.xpack.spatial.index.mapper.GeoShapeValuesSource; +import org.elasticsearch.xpack.spatial.index.mapper.MultiGeoShapeValues; + +import java.io.IOException; +import java.util.Map; + +public final class GeoShapeBoundsAggregator extends GeoBoundsAggregator { + private final GeoShapeValuesSource valuesSource; + + public GeoShapeBoundsAggregator(String name, SearchContext aggregationContext, Aggregator parent, + GeoShapeValuesSource valuesSource, boolean wrapLongitude, Map metadata) throws IOException { + // the valuesSource as GeoPoint passed to GeoBoundsAggregator is not used + super(name, aggregationContext, parent, ValuesSource.GeoPoint.EMPTY, wrapLongitude, metadata); + this.valuesSource = valuesSource; + } + + @Override + public LeafBucketCollector getLeafCollector(LeafReaderContext ctx, + LeafBucketCollector sub) { + if (valuesSource == null) { + return LeafBucketCollector.NO_OP_COLLECTOR; + } + final BigArrays bigArrays = context.bigArrays(); + final MultiGeoShapeValues values = valuesSource.geoShapeValues(ctx); + return new LeafBucketCollectorBase(sub, values) { + @Override + public void collect(int doc, long bucket) throws IOException { + if (bucket >= tops.size()) { + long from = tops.size(); + tops = bigArrays.grow(tops, bucket + 1); + tops.fill(from, tops.size(), Double.NEGATIVE_INFINITY); + bottoms = bigArrays.resize(bottoms, tops.size()); + bottoms.fill(from, bottoms.size(), Double.POSITIVE_INFINITY); + posLefts = bigArrays.resize(posLefts, tops.size()); + posLefts.fill(from, posLefts.size(), Double.POSITIVE_INFINITY); + posRights = bigArrays.resize(posRights, tops.size()); + posRights.fill(from, posRights.size(), Double.NEGATIVE_INFINITY); + negLefts = bigArrays.resize(negLefts, tops.size()); + negLefts.fill(from, negLefts.size(), Double.POSITIVE_INFINITY); + negRights = bigArrays.resize(negRights, tops.size()); + negRights.fill(from, negRights.size(), Double.NEGATIVE_INFINITY); + } + + if (values.advanceExact(doc)) { + final int valuesCount = values.docValueCount(); + + for (int i = 0; i < valuesCount; ++i) { + MultiGeoShapeValues.GeoShapeValue value = values.nextValue(); + MultiGeoShapeValues.BoundingBox bounds = value.boundingBox(); + double top = Math.max(tops.get(bucket), bounds.top); + double bottom = Math.min(bottoms.get(bucket), bounds.bottom); + double posLeft = Math.min(posLefts.get(bucket), bounds.posLeft); + double posRight = Math.max(posRights.get(bucket), bounds.posRight); + double negLeft = Math.min(negLefts.get(bucket), bounds.negLeft); + double negRight = Math.max(negRights.get(bucket), bounds.negRight); + tops.set(bucket, top); + bottoms.set(bucket, bottom); + posLefts.set(bucket, posLeft); + posRights.set(bucket, posRight); + negLefts.set(bucket, negLeft); + negRights.set(bucket, negRight); + } + } + } + }; + } +} diff --git a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/mapper/GeoShapeValuesSource.java b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/mapper/GeoShapeValuesSource.java index 96ba87023b44d..3604b8d908ebd 100644 --- a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/mapper/GeoShapeValuesSource.java +++ b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/mapper/GeoShapeValuesSource.java @@ -29,7 +29,7 @@ public SortedBinaryDocValues bytesValues(LeafReaderContext context) throws IOExc }; - abstract MultiGeoShapeValues geoShapeValues(LeafReaderContext context); + public abstract MultiGeoShapeValues geoShapeValues(LeafReaderContext context); @Override public DocValueBits docsWithValue(LeafReaderContext context) throws IOException { diff --git a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/mapper/GeoShapeValuesSourceType.java b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/mapper/GeoShapeValuesSourceType.java index ad94e12a6e73e..cd9412fe87f13 100644 --- a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/mapper/GeoShapeValuesSourceType.java +++ b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/mapper/GeoShapeValuesSourceType.java @@ -25,7 +25,7 @@ public class GeoShapeValuesSourceType implements Writeable, ValuesSourceType { - static GeoShapeValuesSourceType INSTANCE = new GeoShapeValuesSourceType(); + public static GeoShapeValuesSourceType INSTANCE = new GeoShapeValuesSourceType(); @Override public ValuesSource getEmpty() { @@ -58,7 +58,7 @@ public ValuesSource replaceMissing(ValuesSource valuesSource, Object rawMissing, final MultiGeoShapeValues.GeoShapeValue missing = MultiGeoShapeValues.GeoShapeValue.missing(rawMissing.toString()); return new GeoShapeValuesSource() { @Override - MultiGeoShapeValues geoShapeValues(LeafReaderContext context) { + public MultiGeoShapeValues geoShapeValues(LeafReaderContext context) { MultiGeoShapeValues values = geoShapeValuesSource.geoShapeValues(context); return new MultiGeoShapeValues() { diff --git a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/mapper/GeoShapeWithDocValuesFieldMapper.java b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/mapper/GeoShapeWithDocValuesFieldMapper.java index c7b3b9ee0bb3a..b1f17f037f5bb 100644 --- a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/mapper/GeoShapeWithDocValuesFieldMapper.java +++ b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/mapper/GeoShapeWithDocValuesFieldMapper.java @@ -35,6 +35,7 @@ import org.elasticsearch.index.mapper.TypeParsers; import org.elasticsearch.index.query.QueryShardContext; import org.elasticsearch.index.query.VectorGeoShapeQueryProcessor; +import org.elasticsearch.search.aggregations.support.ValuesSourceType; import java.io.IOException; import java.util.HashMap; @@ -170,6 +171,11 @@ public Query existsQuery(QueryShardContext context) { } } + @Override + public ValuesSourceType getValuesSourceType() { + return GeoShapeValuesSourceType.INSTANCE; + } + @Override public GeoShapeWithDocValuesFieldType clone() { return new GeoShapeWithDocValuesFieldType(this); diff --git a/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/aggregations/metrics/GeoShapeBoundsAggregatorTests.java b/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/aggregations/metrics/GeoShapeBoundsAggregatorTests.java new file mode 100644 index 0000000000000..28756a9d6f68a --- /dev/null +++ b/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/aggregations/metrics/GeoShapeBoundsAggregatorTests.java @@ -0,0 +1,231 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +package org.elasticsearch.xpack.spatial.aggregations.metrics; + +import org.apache.lucene.document.Document; +import org.apache.lucene.document.LatLonDocValuesField; +import org.apache.lucene.document.NumericDocValuesField; +import org.apache.lucene.geo.GeoEncodingUtils; +import org.apache.lucene.index.IndexReader; +import org.apache.lucene.index.RandomIndexWriter; +import org.apache.lucene.search.IndexSearcher; +import org.apache.lucene.search.MatchAllDocsQuery; +import org.apache.lucene.store.Directory; +import org.elasticsearch.geo.GeometryTestUtils; +import org.elasticsearch.geometry.Geometry; +import org.elasticsearch.geometry.MultiPoint; +import org.elasticsearch.geometry.Point; +import org.elasticsearch.index.mapper.MappedFieldType; +import org.elasticsearch.search.aggregations.AggregationBuilder; +import org.elasticsearch.search.aggregations.AggregatorTestCase; +import org.elasticsearch.search.aggregations.metrics.GeoBoundsAggregationBuilder; +import org.elasticsearch.search.aggregations.metrics.InternalGeoBounds; +import org.elasticsearch.search.aggregations.support.AggregationInspectionHelper; +import org.elasticsearch.search.aggregations.support.ValuesSourceType; +import org.elasticsearch.xpack.spatial.SpatialPlugin; +import org.elasticsearch.xpack.spatial.index.mapper.BinaryGeoShapeDocValuesField; +import org.elasticsearch.xpack.spatial.index.mapper.CentroidCalculator; +import org.elasticsearch.xpack.spatial.index.mapper.GeoShapeValuesSourceType; +import org.elasticsearch.xpack.spatial.index.mapper.GeoShapeWithDocValuesFieldMapper; +import org.elasticsearch.xpack.spatial.util.GeoTestUtils; +import org.junit.BeforeClass; + +import java.util.ArrayList; +import java.util.List; + +import static org.hamcrest.Matchers.closeTo; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.startsWith; + +public class GeoShapeBoundsAggregatorTests extends AggregatorTestCase { + static final double GEOHASH_TOLERANCE = 1E-5D; + + @BeforeClass() + public static void registerAggregator() { + SpatialPlugin.registerGeoShapeBoundsAggregator(valuesSourceRegistry); + } + + public void testEmpty() throws Exception { + try (Directory dir = newDirectory(); RandomIndexWriter w = new RandomIndexWriter(random(), dir)) { + GeoBoundsAggregationBuilder aggBuilder = new GeoBoundsAggregationBuilder("my_agg") + .field("field") + .wrapLongitude(false); + + MappedFieldType fieldType = new GeoShapeWithDocValuesFieldMapper.GeoShapeWithDocValuesFieldType(); + fieldType.setHasDocValues(true); + fieldType.setName("field"); + try (IndexReader reader = w.getReader()) { + IndexSearcher searcher = new IndexSearcher(reader); + InternalGeoBounds bounds = search(searcher, new MatchAllDocsQuery(), aggBuilder, fieldType); + assertTrue(Double.isInfinite(bounds.top)); + assertTrue(Double.isInfinite(bounds.bottom)); + assertTrue(Double.isInfinite(bounds.posLeft)); + assertTrue(Double.isInfinite(bounds.posRight)); + assertTrue(Double.isInfinite(bounds.negLeft)); + assertTrue(Double.isInfinite(bounds.negRight)); + assertFalse(AggregationInspectionHelper.hasValue(bounds)); + } + } + } + + public void testUnmappedFieldWithDocs() throws Exception { + try (Directory dir = newDirectory(); RandomIndexWriter w = new RandomIndexWriter(random(), dir)) { + if (randomBoolean()) { + Document doc = new Document(); + doc.add(new LatLonDocValuesField("field", 0.0, 0.0)); + w.addDocument(doc); + } + + GeoBoundsAggregationBuilder aggBuilder = new GeoBoundsAggregationBuilder("my_agg") + .field("non_existent") + .wrapLongitude(false); + + MappedFieldType fieldType = new GeoShapeWithDocValuesFieldMapper.GeoShapeWithDocValuesFieldType(); + fieldType.setHasDocValues(true); + fieldType.setName("field"); + try (IndexReader reader = w.getReader()) { + IndexSearcher searcher = new IndexSearcher(reader); + InternalGeoBounds bounds = search(searcher, new MatchAllDocsQuery(), aggBuilder, fieldType); + assertTrue(Double.isInfinite(bounds.top)); + assertTrue(Double.isInfinite(bounds.bottom)); + assertTrue(Double.isInfinite(bounds.posLeft)); + assertTrue(Double.isInfinite(bounds.posRight)); + assertTrue(Double.isInfinite(bounds.negLeft)); + assertTrue(Double.isInfinite(bounds.negRight)); + assertFalse(AggregationInspectionHelper.hasValue(bounds)); + } + } + } + + public void testMissing() throws Exception { + try (Directory dir = newDirectory(); RandomIndexWriter w = new RandomIndexWriter(random(), dir)) { + Document doc = new Document(); + doc.add(new NumericDocValuesField("not_field", 1000L)); + w.addDocument(doc); + + MappedFieldType fieldType = new GeoShapeWithDocValuesFieldMapper.GeoShapeWithDocValuesFieldType(); + fieldType.setHasDocValues(true); + fieldType.setName("field"); + + Point point = GeometryTestUtils.randomPoint(false); + double lon = GeoEncodingUtils.decodeLongitude(GeoEncodingUtils.encodeLongitude(point.getX())); + double lat = GeoEncodingUtils.decodeLatitude(GeoEncodingUtils.encodeLatitude(point.getY())); + Object missingVal = "POINT(" + lon + " " + lat + ")"; + + GeoBoundsAggregationBuilder aggBuilder = new GeoBoundsAggregationBuilder("my_agg") + .field("field") + .missing(missingVal) + .wrapLongitude(false); + + try (IndexReader reader = w.getReader()) { + IndexSearcher searcher = new IndexSearcher(reader); + InternalGeoBounds bounds = search(searcher, new MatchAllDocsQuery(), aggBuilder, fieldType); + assertThat(bounds.top, equalTo(lat)); + assertThat(bounds.bottom, equalTo(lat)); + assertThat(bounds.posLeft, equalTo(lon >= 0 ? lon : Double.POSITIVE_INFINITY)); + assertThat(bounds.posRight, equalTo(lon >= 0 ? lon : Double.NEGATIVE_INFINITY)); + assertThat(bounds.negLeft, equalTo(lon >= 0 ? Double.POSITIVE_INFINITY : lon)); + assertThat(bounds.negRight, equalTo(lon >= 0 ? Double.NEGATIVE_INFINITY : lon)); + } + } + } + + public void testInvalidMissing() throws Exception { + try (Directory dir = newDirectory(); RandomIndexWriter w = new RandomIndexWriter(random(), dir)) { + Document doc = new Document(); + doc.add(new NumericDocValuesField("not_field", 1000L)); + w.addDocument(doc); + + MappedFieldType fieldType = new GeoShapeWithDocValuesFieldMapper.GeoShapeWithDocValuesFieldType(); + fieldType.setHasDocValues(true); + fieldType.setName("field"); + + GeoBoundsAggregationBuilder aggBuilder = new GeoBoundsAggregationBuilder("my_agg") + .field("field") + .missing("invalid") + .wrapLongitude(false); + try (IndexReader reader = w.getReader()) { + IndexSearcher searcher = new IndexSearcher(reader); + IllegalArgumentException exception = expectThrows(IllegalArgumentException.class, + () -> search(searcher, new MatchAllDocsQuery(), aggBuilder, fieldType)); + assertThat(exception.getMessage(), startsWith("Unknown geometry type")); + } + } + } + + public void testRandomShapes() throws Exception { + double top = Double.NEGATIVE_INFINITY; + double bottom = Double.POSITIVE_INFINITY; + double posLeft = Double.POSITIVE_INFINITY; + double posRight = Double.NEGATIVE_INFINITY; + double negLeft = Double.POSITIVE_INFINITY; + double negRight = Double.NEGATIVE_INFINITY; + int numDocs = randomIntBetween(50, 100); + try (Directory dir = newDirectory(); + RandomIndexWriter w = new RandomIndexWriter(random(), dir)) { + for (int i = 0; i < numDocs; i++) { + Document doc = new Document(); + int numValues = randomIntBetween(1, 5); + List points = new ArrayList<>(); + for (int j = 0; j < numValues; j++) { + Point point = GeometryTestUtils.randomPoint(false); + points.add(point); + if (point.getLat() > top) { + top = point.getLat(); + } + if (point.getLat() < bottom) { + bottom = point.getLat(); + } + if (point.getLon() >= 0 && point.getLon() < posLeft) { + posLeft = point.getLon(); + } + if (point.getLon() >= 0 && point.getLon() > posRight) { + posRight = point.getLon(); + } + if (point.getLon() < 0 && point.getLon() < negLeft) { + negLeft = point.getLon(); + } + if (point.getLon() < 0 && point.getLon() > negRight) { + negRight = point.getLon(); + } + } + Geometry geometry = new MultiPoint(points); + doc.add(new BinaryGeoShapeDocValuesField("field", GeoTestUtils.toDecodedTriangles(geometry), + new CentroidCalculator(geometry))); + w.addDocument(doc); + } + GeoBoundsAggregationBuilder aggBuilder = new GeoBoundsAggregationBuilder("my_agg") + .field("field") + .wrapLongitude(false); + + MappedFieldType fieldType = new GeoShapeWithDocValuesFieldMapper.GeoShapeWithDocValuesFieldType(); + fieldType.setHasDocValues(true); + fieldType.setName("field"); + try (IndexReader reader = w.getReader()) { + IndexSearcher searcher = new IndexSearcher(reader); + InternalGeoBounds bounds = search(searcher, new MatchAllDocsQuery(), aggBuilder, fieldType); + assertThat(bounds.top, closeTo(top, GEOHASH_TOLERANCE)); + assertThat(bounds.bottom, closeTo(bottom, GEOHASH_TOLERANCE)); + assertThat(bounds.posLeft, closeTo(posLeft, GEOHASH_TOLERANCE)); + assertThat(bounds.posRight, closeTo(posRight, GEOHASH_TOLERANCE)); + assertThat(bounds.negRight, closeTo(negRight, GEOHASH_TOLERANCE)); + assertThat(bounds.negLeft, closeTo(negLeft, GEOHASH_TOLERANCE)); + assertTrue(AggregationInspectionHelper.hasValue(bounds)); + } + } + } + + @Override + protected AggregationBuilder createAggBuilderForTypeTest(MappedFieldType fieldType, String fieldName) { + return new GeoBoundsAggregationBuilder("foo").field(fieldName); + } + + @Override + protected List getSupportedValuesSourceTypes() { + return List.of(GeoShapeValuesSourceType.INSTANCE); + } +} From 1ef925cd78cf4cb4cba69b59527fe102d3282cc7 Mon Sep 17 00:00:00 2001 From: Tal Levy Date: Thu, 16 Apr 2020 17:49:24 -0700 Subject: [PATCH 2/6] add yaml tests --- .../metrics/GeoBoundsAggregatorSupplier.java | 2 +- .../rest-api-spec/test/10_geo_bounds.yml | 34 +++++++++ .../100_geo_shape_doc_values.yml | 27 +++++++ .../old_cluster/100_geo_shape_doc_values.yml | 31 ++++++++ .../100_geo_shape_doc_values.yml | 76 +++++++++++++++++++ 5 files changed, 169 insertions(+), 1 deletion(-) create mode 100644 x-pack/plugin/spatial/src/test/resources/rest-api-spec/test/10_geo_bounds.yml create mode 100644 x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/mixed_cluster/100_geo_shape_doc_values.yml create mode 100644 x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/old_cluster/100_geo_shape_doc_values.yml create mode 100644 x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/upgraded_cluster/100_geo_shape_doc_values.yml diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/GeoBoundsAggregatorSupplier.java b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/GeoBoundsAggregatorSupplier.java index a580aab89b4b3..c6e8208967b7b 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/GeoBoundsAggregatorSupplier.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/GeoBoundsAggregatorSupplier.java @@ -30,6 +30,6 @@ @FunctionalInterface public interface GeoBoundsAggregatorSupplier extends AggregatorSupplier { - MetricsAggregator build(String name, SearchContext aggregationContext, Aggregator parent, + GeoBoundsAggregator build(String name, SearchContext aggregationContext, Aggregator parent, ValuesSource valuesSource, boolean wrapLongitude, Map metadata) throws IOException; } diff --git a/x-pack/plugin/spatial/src/test/resources/rest-api-spec/test/10_geo_bounds.yml b/x-pack/plugin/spatial/src/test/resources/rest-api-spec/test/10_geo_bounds.yml new file mode 100644 index 0000000000000..6fc0dccdbfb2d --- /dev/null +++ b/x-pack/plugin/spatial/src/test/resources/rest-api-spec/test/10_geo_bounds.yml @@ -0,0 +1,34 @@ +--- +"Search existing index with geo_shape field": + - do: + indices.create: + index: locations + body: + mappings: + properties: + location: + type: geo_shape + + - do: + index: + index: locations + id: point_with_doc_values + body: { location: "POINT(34.25 -21.76)" } + + - do: + indices.refresh: {} + + - do: + search: + rest_total_hits_as_int: true + index: locations + size: 0 + body: + aggs: + my_agg: + geo_bounds: + field: location + wrap_longitude: true + - match: {hits.total: 1 } + - match: { aggregations.my_agg.bounds.top_left.lat: -21.760000032372773 } + - match: { aggregations.my_agg.bounds.top_left.lon: 34.24999997019768 } diff --git a/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/mixed_cluster/100_geo_shape_doc_values.yml b/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/mixed_cluster/100_geo_shape_doc_values.yml new file mode 100644 index 0000000000000..1f76e3ab1f1da --- /dev/null +++ b/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/mixed_cluster/100_geo_shape_doc_values.yml @@ -0,0 +1,27 @@ +--- +"Search existing index with geo_shape field": + + - do: + index: + index: old_locations + id: mixed_point_with_doc_value + body: { location: "POINT(11.25 43.24)" } + + - do: + indices.refresh: {} + + - do: + search: + rest_total_hits_as_int: true + index: old_locations + size: 10 + body: + query: + match_all: {} + - match: {hits.total: 2 } + - length: {hits.hits: 2 } + + - do: + indices.force_merge: + index: old_locations + diff --git a/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/old_cluster/100_geo_shape_doc_values.yml b/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/old_cluster/100_geo_shape_doc_values.yml new file mode 100644 index 0000000000000..a075b1339b349 --- /dev/null +++ b/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/old_cluster/100_geo_shape_doc_values.yml @@ -0,0 +1,31 @@ +--- +"Search existing index with geo_shape field": + - do: + indices.create: + index: old_locations + body: + mappings: + properties: + location: + type: geo_shape + + - do: + index: + index: old_locations + id: point + body: { location: "POINT(34.25 -21.76)" } + + - do: + indices.refresh: {} + + - do: + search: + rest_total_hits_as_int: true + index: old_locations + size: 1 + body: + query: + match_all: {} + - match: {hits.total: 1 } + - length: {hits.hits: 1 } + - match: {hits.hits.0._id: "point" } diff --git a/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/upgraded_cluster/100_geo_shape_doc_values.yml b/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/upgraded_cluster/100_geo_shape_doc_values.yml new file mode 100644 index 0000000000000..de07593a6c53e --- /dev/null +++ b/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/upgraded_cluster/100_geo_shape_doc_values.yml @@ -0,0 +1,76 @@ +--- +"Search existing index with geo_shape field": + - do: + indices.create: + index: new_locations + body: + mappings: + properties: + location: + type: geo_shape + + - do: + index: + index: new_locations + id: upgraded_point_with_doc_values + body: { location: "POINT(34.25 -21.76)" } + + - do: + indices.refresh: {} + + - do: + search: + rest_total_hits_as_int: true + index: "*locations" + size: 10 + body: + query: + match_all: {} + - match: {hits.total: 3 } + - length: {hits.hits: 3 } + + - do: + catch: bad_request + search: + rest_total_hits_as_int: true + index: old_locations + size: 0 + body: + aggs: + my_agg: + geo_bounds: + field: location + wrap_longitude: true + + - do: + search: + rest_total_hits_as_int: true + index: "*locations" + size: 0 + body: + aggs: + my_agg: + geo_bounds: + field: location + wrap_longitude: true + - match: {hits.total: 1 } + - match: { aggregations.my_agg.bounds.top_left.lat: -21.760000032372773 } + - match: { aggregations.my_agg.bounds.top_left.lon: 34.24999997019768 } + - match: { _shards.failures.0.index: old_locations } + - match: { _shards.failures.0.reason.type: illegal_argument_exception } + - match: { _shards.failures.0.reason.reason: "Can't load fielddata on [location] because fielddata is unsupported on fields of type [geo_shape]. Use doc values instead." } + + - do: + search: + rest_total_hits_as_int: true + index: new_locations + size: 0 + body: + aggs: + my_agg: + geo_bounds: + field: location + wrap_longitude: true + - match: {hits.total: 1 } + - match: { aggregations.my_agg.bounds.top_left.lat: -21.760000032372773 } + - match: { aggregations.my_agg.bounds.top_left.lon: 34.24999997019768 } From 9e723014e3ba15e503fdc8db89cb11b9feb256e2 Mon Sep 17 00:00:00 2001 From: Tal Levy Date: Thu, 16 Apr 2020 18:13:32 -0700 Subject: [PATCH 3/6] fix --- .../src/test/resources/rest-api-spec/test/10_geo_bounds.yml | 2 +- .../test/mixed_cluster/100_geo_shape_doc_values.yml | 5 +++-- .../test/old_cluster/100_geo_shape_doc_values.yml | 2 +- .../test/upgraded_cluster/100_geo_shape_doc_values.yml | 2 +- 4 files changed, 6 insertions(+), 5 deletions(-) diff --git a/x-pack/plugin/spatial/src/test/resources/rest-api-spec/test/10_geo_bounds.yml b/x-pack/plugin/spatial/src/test/resources/rest-api-spec/test/10_geo_bounds.yml index 6fc0dccdbfb2d..d47f047bd6af8 100644 --- a/x-pack/plugin/spatial/src/test/resources/rest-api-spec/test/10_geo_bounds.yml +++ b/x-pack/plugin/spatial/src/test/resources/rest-api-spec/test/10_geo_bounds.yml @@ -1,5 +1,5 @@ --- -"Search existing index with geo_shape field": +"Test geo_bounds aggregation on geo_shape field": - do: indices.create: index: locations diff --git a/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/mixed_cluster/100_geo_shape_doc_values.yml b/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/mixed_cluster/100_geo_shape_doc_values.yml index 1f76e3ab1f1da..c3e4483f3d039 100644 --- a/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/mixed_cluster/100_geo_shape_doc_values.yml +++ b/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/mixed_cluster/100_geo_shape_doc_values.yml @@ -1,5 +1,5 @@ --- -"Search existing index with geo_shape field": +"Test mixed geo_shape indexing": - do: index: @@ -22,6 +22,7 @@ - length: {hits.hits: 2 } - do: - indices.force_merge: + indices.forcemerge: index: old_locations + max_num_segments: 1 diff --git a/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/old_cluster/100_geo_shape_doc_values.yml b/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/old_cluster/100_geo_shape_doc_values.yml index a075b1339b349..555b05256eb6c 100644 --- a/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/old_cluster/100_geo_shape_doc_values.yml +++ b/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/old_cluster/100_geo_shape_doc_values.yml @@ -1,5 +1,5 @@ --- -"Search existing index with geo_shape field": +"Test 7.x geo_shape fields": - do: indices.create: index: old_locations diff --git a/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/upgraded_cluster/100_geo_shape_doc_values.yml b/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/upgraded_cluster/100_geo_shape_doc_values.yml index de07593a6c53e..013cca1a79c4c 100644 --- a/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/upgraded_cluster/100_geo_shape_doc_values.yml +++ b/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/upgraded_cluster/100_geo_shape_doc_values.yml @@ -1,5 +1,5 @@ --- -"Search existing index with geo_shape field": +"Test upgraded 8.x cluster with 8.x and 7.x geo_shape fields": - do: indices.create: index: new_locations From 20ec976b71ead310369f1cf3dcb56c18d8bacb79 Mon Sep 17 00:00:00 2001 From: Tal Levy Date: Sun, 19 Apr 2020 22:25:00 -0700 Subject: [PATCH 4/6] inherit from MetricsAggregator and fix tests --- .../metrics/GeoBoundsAggregatorSupplier.java | 2 +- .../metrics/InternalGeoBounds.java | 4 +- .../metrics/GeoShapeBoundsAggregator.java | 61 +++++++++++++++++-- x-pack/qa/rolling-upgrade/build.gradle | 5 ++ .../100_geo_shape_doc_values.yml | 22 ++++++- .../old_cluster/100_geo_shape_doc_values.yml | 12 ++++ .../100_geo_shape_doc_values.yml | 7 +++ 7 files changed, 103 insertions(+), 10 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/GeoBoundsAggregatorSupplier.java b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/GeoBoundsAggregatorSupplier.java index c6e8208967b7b..a580aab89b4b3 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/GeoBoundsAggregatorSupplier.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/GeoBoundsAggregatorSupplier.java @@ -30,6 +30,6 @@ @FunctionalInterface public interface GeoBoundsAggregatorSupplier extends AggregatorSupplier { - GeoBoundsAggregator build(String name, SearchContext aggregationContext, Aggregator parent, + MetricsAggregator build(String name, SearchContext aggregationContext, Aggregator parent, ValuesSource valuesSource, boolean wrapLongitude, Map metadata) throws IOException; } diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/InternalGeoBounds.java b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/InternalGeoBounds.java index 4bdac7f32afff..c1828b6ab9739 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/InternalGeoBounds.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/InternalGeoBounds.java @@ -40,8 +40,8 @@ public class InternalGeoBounds extends InternalAggregation implements GeoBounds public final double negRight; public final boolean wrapLongitude; - InternalGeoBounds(String name, double top, double bottom, double posLeft, double posRight, - double negLeft, double negRight, boolean wrapLongitude, Map metadata) { + public InternalGeoBounds(String name, double top, double bottom, double posLeft, double posRight, + double negLeft, double negRight, boolean wrapLongitude, Map metadata) { super(name, metadata); this.top = top; this.bottom = bottom; diff --git a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/aggregations/metrics/GeoShapeBoundsAggregator.java b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/aggregations/metrics/GeoShapeBoundsAggregator.java index 3da126b4524d4..0911cbfc2d26c 100644 --- a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/aggregations/metrics/GeoShapeBoundsAggregator.java +++ b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/aggregations/metrics/GeoShapeBoundsAggregator.java @@ -7,12 +7,15 @@ package org.elasticsearch.xpack.spatial.aggregations.metrics; import org.apache.lucene.index.LeafReaderContext; +import org.elasticsearch.common.lease.Releasables; import org.elasticsearch.common.util.BigArrays; +import org.elasticsearch.common.util.DoubleArray; import org.elasticsearch.search.aggregations.Aggregator; +import org.elasticsearch.search.aggregations.InternalAggregation; import org.elasticsearch.search.aggregations.LeafBucketCollector; import org.elasticsearch.search.aggregations.LeafBucketCollectorBase; -import org.elasticsearch.search.aggregations.metrics.GeoBoundsAggregator; -import org.elasticsearch.search.aggregations.support.ValuesSource; +import org.elasticsearch.search.aggregations.metrics.InternalGeoBounds; +import org.elasticsearch.search.aggregations.metrics.MetricsAggregator; import org.elasticsearch.search.internal.SearchContext; import org.elasticsearch.xpack.spatial.index.mapper.GeoShapeValuesSource; import org.elasticsearch.xpack.spatial.index.mapper.MultiGeoShapeValues; @@ -20,14 +23,36 @@ import java.io.IOException; import java.util.Map; -public final class GeoShapeBoundsAggregator extends GeoBoundsAggregator { +public final class GeoShapeBoundsAggregator extends MetricsAggregator { private final GeoShapeValuesSource valuesSource; + private final boolean wrapLongitude; + protected DoubleArray tops; + protected DoubleArray bottoms; + protected DoubleArray posLefts; + protected DoubleArray posRights; + protected DoubleArray negLefts; + protected DoubleArray negRights; public GeoShapeBoundsAggregator(String name, SearchContext aggregationContext, Aggregator parent, GeoShapeValuesSource valuesSource, boolean wrapLongitude, Map metadata) throws IOException { - // the valuesSource as GeoPoint passed to GeoBoundsAggregator is not used - super(name, aggregationContext, parent, ValuesSource.GeoPoint.EMPTY, wrapLongitude, metadata); + super(name, aggregationContext, parent, metadata); this.valuesSource = valuesSource; + this.wrapLongitude = wrapLongitude; + if (valuesSource != null) { + final BigArrays bigArrays = context.bigArrays(); + tops = bigArrays.newDoubleArray(1, false); + tops.fill(0, tops.size(), Double.NEGATIVE_INFINITY); + bottoms = bigArrays.newDoubleArray(1, false); + bottoms.fill(0, bottoms.size(), Double.POSITIVE_INFINITY); + posLefts = bigArrays.newDoubleArray(1, false); + posLefts.fill(0, posLefts.size(), Double.POSITIVE_INFINITY); + posRights = bigArrays.newDoubleArray(1, false); + posRights.fill(0, posRights.size(), Double.NEGATIVE_INFINITY); + negLefts = bigArrays.newDoubleArray(1, false); + negLefts.fill(0, negLefts.size(), Double.POSITIVE_INFINITY); + negRights = bigArrays.newDoubleArray(1, false); + negRights.fill(0, negRights.size(), Double.NEGATIVE_INFINITY); + } } @Override @@ -80,4 +105,30 @@ public void collect(int doc, long bucket) throws IOException { } }; } + + + @Override + public InternalAggregation buildAggregation(long owningBucketOrdinal) { + if (valuesSource == null) { + return buildEmptyAggregation(); + } + double top = tops.get(owningBucketOrdinal); + double bottom = bottoms.get(owningBucketOrdinal); + double posLeft = posLefts.get(owningBucketOrdinal); + double posRight = posRights.get(owningBucketOrdinal); + double negLeft = negLefts.get(owningBucketOrdinal); + double negRight = negRights.get(owningBucketOrdinal); + return new InternalGeoBounds(name, top, bottom, posLeft, posRight, negLeft, negRight, wrapLongitude, metadata()); + } + + @Override + public InternalAggregation buildEmptyAggregation() { + return new InternalGeoBounds(name, Double.NEGATIVE_INFINITY, Double.POSITIVE_INFINITY, Double.POSITIVE_INFINITY, + Double.NEGATIVE_INFINITY, Double.POSITIVE_INFINITY, Double.NEGATIVE_INFINITY, wrapLongitude, metadata()); + } + + @Override + public void doClose() { + Releasables.close(tops, bottoms, posLefts, posRights, negLefts, negRights); + } } diff --git a/x-pack/qa/rolling-upgrade/build.gradle b/x-pack/qa/rolling-upgrade/build.gradle index c05e0c9a1275c..8321b11717f45 100644 --- a/x-pack/qa/rolling-upgrade/build.gradle +++ b/x-pack/qa/rolling-upgrade/build.gradle @@ -142,6 +142,11 @@ for (Version bwcVersion : bwcVersions.wireCompatible) { nonInputProperties.systemProperty('tests.rest.cluster', "${-> testClusters."${baseName}".allHttpSocketURI.join(",")}") nonInputProperties.systemProperty('tests.clustername', "${-> testClusters."${baseName}".getName()}") systemProperty 'tests.rest.suite', 'upgraded_cluster' + if (bwcVersion.onOrAfter("8.0.0")) { + systemProperty 'tests.rest.blacklist', [ + 'upgraded_cluster/100_geo_shape_doc_values/Test upgraded 8.x cluster with 8.x and 7.x geo_shape fields' + ] + } systemProperty 'tests.upgrade_from_version', oldVersion } diff --git a/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/mixed_cluster/100_geo_shape_doc_values.yml b/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/mixed_cluster/100_geo_shape_doc_values.yml index c3e4483f3d039..f38938252d9c0 100644 --- a/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/mixed_cluster/100_geo_shape_doc_values.yml +++ b/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/mixed_cluster/100_geo_shape_doc_values.yml @@ -1,5 +1,15 @@ --- "Test mixed geo_shape indexing": + - skip: + version: "8.0.0 -" + reason: This is meant to run on 7.x indices + + - do: + indices.get_field_mapping: + index: old_locations + fields: location + include_defaults: true + - is_false: old_locations.mappings.location.mapping.location.doc_values - do: index: @@ -22,7 +32,15 @@ - length: {hits.hits: 2 } - do: - indices.forcemerge: + catch: bad_request + search: + rest_total_hits_as_int: true index: old_locations - max_num_segments: 1 + size: 0 + body: + aggs: + my_agg: + geo_bounds: + field: location + wrap_longitude: true diff --git a/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/old_cluster/100_geo_shape_doc_values.yml b/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/old_cluster/100_geo_shape_doc_values.yml index 555b05256eb6c..56551acaf3f9b 100644 --- a/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/old_cluster/100_geo_shape_doc_values.yml +++ b/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/old_cluster/100_geo_shape_doc_values.yml @@ -1,5 +1,9 @@ --- "Test 7.x geo_shape fields": + - skip: + version: "8.0.0 -" + reason: This is meant to run on 7.x indices + - do: indices.create: index: old_locations @@ -9,6 +13,14 @@ location: type: geo_shape + + - do: + indices.get_field_mapping: + index: old_locations + fields: location + include_defaults: true + - match: { old_locations.mappings.location.mapping.location.doc_values: null } + - do: index: index: old_locations diff --git a/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/upgraded_cluster/100_geo_shape_doc_values.yml b/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/upgraded_cluster/100_geo_shape_doc_values.yml index 013cca1a79c4c..3661138072655 100644 --- a/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/upgraded_cluster/100_geo_shape_doc_values.yml +++ b/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/upgraded_cluster/100_geo_shape_doc_values.yml @@ -9,6 +9,13 @@ location: type: geo_shape + - do: + indices.get_field_mapping: + index: new_locations + fields: location + include_defaults: true + - is_true: new_locations.mappings.location.mapping.location.doc_values + - do: index: index: new_locations From 2548f967d151ddcc1acb73baf290ae496fd8d113 Mon Sep 17 00:00:00 2001 From: Tal Levy Date: Mon, 20 Apr 2020 15:08:58 -0700 Subject: [PATCH 5/6] revert the upgraded cluster skipping tests --- x-pack/qa/rolling-upgrade/build.gradle | 5 --- .../100_geo_shape_doc_values.yml | 2 +- .../100_geo_shape_doc_values.yml | 42 ------------------- 3 files changed, 1 insertion(+), 48 deletions(-) diff --git a/x-pack/qa/rolling-upgrade/build.gradle b/x-pack/qa/rolling-upgrade/build.gradle index 8321b11717f45..c05e0c9a1275c 100644 --- a/x-pack/qa/rolling-upgrade/build.gradle +++ b/x-pack/qa/rolling-upgrade/build.gradle @@ -142,11 +142,6 @@ for (Version bwcVersion : bwcVersions.wireCompatible) { nonInputProperties.systemProperty('tests.rest.cluster', "${-> testClusters."${baseName}".allHttpSocketURI.join(",")}") nonInputProperties.systemProperty('tests.clustername', "${-> testClusters."${baseName}".getName()}") systemProperty 'tests.rest.suite', 'upgraded_cluster' - if (bwcVersion.onOrAfter("8.0.0")) { - systemProperty 'tests.rest.blacklist', [ - 'upgraded_cluster/100_geo_shape_doc_values/Test upgraded 8.x cluster with 8.x and 7.x geo_shape fields' - ] - } systemProperty 'tests.upgrade_from_version', oldVersion } diff --git a/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/mixed_cluster/100_geo_shape_doc_values.yml b/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/mixed_cluster/100_geo_shape_doc_values.yml index f38938252d9c0..008f3fad39019 100644 --- a/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/mixed_cluster/100_geo_shape_doc_values.yml +++ b/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/mixed_cluster/100_geo_shape_doc_values.yml @@ -14,7 +14,7 @@ - do: index: index: old_locations - id: mixed_point_with_doc_value + id: mixed_point_without_doc_value body: { location: "POINT(11.25 43.24)" } - do: diff --git a/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/upgraded_cluster/100_geo_shape_doc_values.yml b/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/upgraded_cluster/100_geo_shape_doc_values.yml index 3661138072655..e145e00ccf769 100644 --- a/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/upgraded_cluster/100_geo_shape_doc_values.yml +++ b/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/upgraded_cluster/100_geo_shape_doc_values.yml @@ -25,48 +25,6 @@ - do: indices.refresh: {} - - do: - search: - rest_total_hits_as_int: true - index: "*locations" - size: 10 - body: - query: - match_all: {} - - match: {hits.total: 3 } - - length: {hits.hits: 3 } - - - do: - catch: bad_request - search: - rest_total_hits_as_int: true - index: old_locations - size: 0 - body: - aggs: - my_agg: - geo_bounds: - field: location - wrap_longitude: true - - - do: - search: - rest_total_hits_as_int: true - index: "*locations" - size: 0 - body: - aggs: - my_agg: - geo_bounds: - field: location - wrap_longitude: true - - match: {hits.total: 1 } - - match: { aggregations.my_agg.bounds.top_left.lat: -21.760000032372773 } - - match: { aggregations.my_agg.bounds.top_left.lon: 34.24999997019768 } - - match: { _shards.failures.0.index: old_locations } - - match: { _shards.failures.0.reason.type: illegal_argument_exception } - - match: { _shards.failures.0.reason.reason: "Can't load fielddata on [location] because fielddata is unsupported on fields of type [geo_shape]. Use doc values instead." } - - do: search: rest_total_hits_as_int: true From e4b30d44529596cb4212d45c957d1209ac5248f0 Mon Sep 17 00:00:00 2001 From: Tal Levy Date: Wed, 22 Apr 2020 06:49:21 -0700 Subject: [PATCH 6/6] remove bwc tests to be merged in 7.x. fix scoping --- .../metrics/GeoBoundsAggregator.java | 18 ++++---- .../metrics/GeoShapeBoundsAggregator.java | 12 ++--- .../100_geo_shape_doc_values.yml | 46 ------------------- .../old_cluster/100_geo_shape_doc_values.yml | 43 ----------------- .../100_geo_shape_doc_values.yml | 41 ----------------- 5 files changed, 15 insertions(+), 145 deletions(-) delete mode 100644 x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/mixed_cluster/100_geo_shape_doc_values.yml delete mode 100644 x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/old_cluster/100_geo_shape_doc_values.yml delete mode 100644 x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/upgraded_cluster/100_geo_shape_doc_values.yml diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/GeoBoundsAggregator.java b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/GeoBoundsAggregator.java index dfde53a719de7..9a76873aaeaab 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/GeoBoundsAggregator.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/GeoBoundsAggregator.java @@ -36,20 +36,20 @@ import java.io.IOException; import java.util.Map; -public class GeoBoundsAggregator extends MetricsAggregator { +final class GeoBoundsAggregator extends MetricsAggregator { - public static final ParseField WRAP_LONGITUDE_FIELD = new ParseField("wrap_longitude"); + static final ParseField WRAP_LONGITUDE_FIELD = new ParseField("wrap_longitude"); private final ValuesSource.GeoPoint valuesSource; private final boolean wrapLongitude; - protected DoubleArray tops; - protected DoubleArray bottoms; - protected DoubleArray posLefts; - protected DoubleArray posRights; - protected DoubleArray negLefts; - protected DoubleArray negRights; + DoubleArray tops; + DoubleArray bottoms; + DoubleArray posLefts; + DoubleArray posRights; + DoubleArray negLefts; + DoubleArray negRights; - public GeoBoundsAggregator(String name, SearchContext aggregationContext, Aggregator parent, + GeoBoundsAggregator(String name, SearchContext aggregationContext, Aggregator parent, ValuesSource.GeoPoint valuesSource, boolean wrapLongitude, Map metadata) throws IOException { super(name, aggregationContext, parent, metadata); this.valuesSource = valuesSource; diff --git a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/aggregations/metrics/GeoShapeBoundsAggregator.java b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/aggregations/metrics/GeoShapeBoundsAggregator.java index 0911cbfc2d26c..06270f2edfbe1 100644 --- a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/aggregations/metrics/GeoShapeBoundsAggregator.java +++ b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/aggregations/metrics/GeoShapeBoundsAggregator.java @@ -26,12 +26,12 @@ public final class GeoShapeBoundsAggregator extends MetricsAggregator { private final GeoShapeValuesSource valuesSource; private final boolean wrapLongitude; - protected DoubleArray tops; - protected DoubleArray bottoms; - protected DoubleArray posLefts; - protected DoubleArray posRights; - protected DoubleArray negLefts; - protected DoubleArray negRights; + private DoubleArray tops; + private DoubleArray bottoms; + private DoubleArray posLefts; + private DoubleArray posRights; + private DoubleArray negLefts; + private DoubleArray negRights; public GeoShapeBoundsAggregator(String name, SearchContext aggregationContext, Aggregator parent, GeoShapeValuesSource valuesSource, boolean wrapLongitude, Map metadata) throws IOException { diff --git a/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/mixed_cluster/100_geo_shape_doc_values.yml b/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/mixed_cluster/100_geo_shape_doc_values.yml deleted file mode 100644 index 008f3fad39019..0000000000000 --- a/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/mixed_cluster/100_geo_shape_doc_values.yml +++ /dev/null @@ -1,46 +0,0 @@ ---- -"Test mixed geo_shape indexing": - - skip: - version: "8.0.0 -" - reason: This is meant to run on 7.x indices - - - do: - indices.get_field_mapping: - index: old_locations - fields: location - include_defaults: true - - is_false: old_locations.mappings.location.mapping.location.doc_values - - - do: - index: - index: old_locations - id: mixed_point_without_doc_value - body: { location: "POINT(11.25 43.24)" } - - - do: - indices.refresh: {} - - - do: - search: - rest_total_hits_as_int: true - index: old_locations - size: 10 - body: - query: - match_all: {} - - match: {hits.total: 2 } - - length: {hits.hits: 2 } - - - do: - catch: bad_request - search: - rest_total_hits_as_int: true - index: old_locations - size: 0 - body: - aggs: - my_agg: - geo_bounds: - field: location - wrap_longitude: true - diff --git a/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/old_cluster/100_geo_shape_doc_values.yml b/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/old_cluster/100_geo_shape_doc_values.yml deleted file mode 100644 index 56551acaf3f9b..0000000000000 --- a/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/old_cluster/100_geo_shape_doc_values.yml +++ /dev/null @@ -1,43 +0,0 @@ ---- -"Test 7.x geo_shape fields": - - skip: - version: "8.0.0 -" - reason: This is meant to run on 7.x indices - - - do: - indices.create: - index: old_locations - body: - mappings: - properties: - location: - type: geo_shape - - - - do: - indices.get_field_mapping: - index: old_locations - fields: location - include_defaults: true - - match: { old_locations.mappings.location.mapping.location.doc_values: null } - - - do: - index: - index: old_locations - id: point - body: { location: "POINT(34.25 -21.76)" } - - - do: - indices.refresh: {} - - - do: - search: - rest_total_hits_as_int: true - index: old_locations - size: 1 - body: - query: - match_all: {} - - match: {hits.total: 1 } - - length: {hits.hits: 1 } - - match: {hits.hits.0._id: "point" } diff --git a/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/upgraded_cluster/100_geo_shape_doc_values.yml b/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/upgraded_cluster/100_geo_shape_doc_values.yml deleted file mode 100644 index e145e00ccf769..0000000000000 --- a/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/upgraded_cluster/100_geo_shape_doc_values.yml +++ /dev/null @@ -1,41 +0,0 @@ ---- -"Test upgraded 8.x cluster with 8.x and 7.x geo_shape fields": - - do: - indices.create: - index: new_locations - body: - mappings: - properties: - location: - type: geo_shape - - - do: - indices.get_field_mapping: - index: new_locations - fields: location - include_defaults: true - - is_true: new_locations.mappings.location.mapping.location.doc_values - - - do: - index: - index: new_locations - id: upgraded_point_with_doc_values - body: { location: "POINT(34.25 -21.76)" } - - - do: - indices.refresh: {} - - - do: - search: - rest_total_hits_as_int: true - index: new_locations - size: 0 - body: - aggs: - my_agg: - geo_bounds: - field: location - wrap_longitude: true - - match: {hits.total: 1 } - - match: { aggregations.my_agg.bounds.top_left.lat: -21.760000032372773 } - - match: { aggregations.my_agg.bounds.top_left.lon: 34.24999997019768 }