From 96887d7c775106cb97d2461fc4edba7ce52487b7 Mon Sep 17 00:00:00 2001 From: Benjamin Trent <4357155+benwtrent@users.noreply.github.com> Date: Tue, 20 Aug 2019 17:45:22 -0500 Subject: [PATCH 01/12] Support geohash_grid aggregation in composite agg sources --- .../test/search.aggregation/230_composite.yml | 94 ++++++++++++++- .../bucket/composite/CellIdSource.java | 110 ++++++++++++++++++ .../bucket/composite/CompositeAggregator.java | 14 +++ .../CompositeValuesSourceParserHelper.java | 8 ++ .../GeoHashGridValuesSourceBuilder.java | 109 +++++++++++++++++ .../bucket/composite/GeohashValuesSource.java | 52 +++++++++ .../CompositeAggregationBuilderTests.java | 16 ++- .../composite/CompositeAggregatorTests.java | 65 ++++++++++- 8 files changed, 462 insertions(+), 6 deletions(-) create mode 100644 server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/CellIdSource.java create mode 100644 server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/GeoHashGridValuesSourceBuilder.java create mode 100644 server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/GeohashValuesSource.java diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/search.aggregation/230_composite.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/search.aggregation/230_composite.yml index fc0710fdb5375..1c3726978859f 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/search.aggregation/230_composite.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/search.aggregation/230_composite.yml @@ -12,6 +12,8 @@ setup: type: keyword long: type: long + geo_point: + type: geo_point nested: type: nested properties: @@ -38,25 +40,25 @@ setup: index: index: test id: 1 - body: { "keyword": "foo", "long": [10, 20], "nested": [{"nested_long": 10}, {"nested_long": 20}] } + body: { "keyword": "foo", "long": [10, 20], "geo_point": "37.2343,-115.8067", "nested": [{"nested_long": 10}, {"nested_long": 20}] } - do: index: index: test id: 2 - body: { "keyword": ["foo", "bar"] } + body: { "keyword": ["foo", "bar"], "geo_point": "41.12,-71.34" } - do: index: index: test id: 3 - body: { "keyword": "bar", "long": [100, 0], "nested": [{"nested_long": 10}, {"nested_long": 0}] } + body: { "keyword": "bar", "long": [100, 0], "geo_point": "90.0,0.0", "nested": [{"nested_long": 10}, {"nested_long": 0}] } - do: index: index: test id: 4 - body: { "keyword": "bar", "long": [1000, 0], "nested": [{"nested_long": 1000}, {"nested_long": 20}] } + body: { "keyword": "bar", "long": [1000, 0], "geo_point": "41.12,-71.34", "nested": [{"nested_long": 1000}, {"nested_long": 20}] } - do: index: @@ -615,3 +617,87 @@ setup: } ] +--- +"Simple Composite aggregation with Geohash grid": + - skip: + version: " - 7.99.99" + reason: geohash_grid is not supported until 8.0.0 + - do: + search: + rest_total_hits_as_int: true + index: test + body: + aggregations: + test: + composite: + sources: [ + "geo": { + "geohash_grid": { + "field": "geo_point", + "precision": 12 + } + }, + { + "kw": { + "terms": { + "field": "keyword" + } + } + } + ] + + - match: {hits.total: 6} + - length: { aggregations.test.buckets: 4 } + - match: { aggregations.test.buckets.0.key.geo: "upbpbpbpbpbp" } + - match: { aggregations.test.buckets.0.key.kw: "bar" } + - match: { aggregations.test.buckets.0.doc_count: 1 } + - match: { aggregations.test.buckets.1.key.geo: "9qteuf21s29g" } + - match: { aggregations.test.buckets.1.key.kw: "foo" } + - match: { aggregations.test.buckets.1.doc_count: 1 } + - match: { aggregations.test.buckets.2.key.geo: "drm3btev3e86" } + - match: { aggregations.test.buckets.2.key.kw: "bar" } + - match: { aggregations.test.buckets.2.doc_count: 2 } + - match: { aggregations.test.buckets.3.key.geo: "drm3btev3e86" } + - match: { aggregations.test.buckets.3.key.kw: "foo" } + - match: { aggregations.test.buckets.3.doc_count: 1 } +--- +"Simple Composite aggregation with geohash grid add aggregate after": + - skip: + version: " - 7.99.99" + reason: geohash_grid is not supported until 8.0.0 + - do: + search: + rest_total_hits_as_int: true + index: test + body: + aggregations: + test: + composite: + sources: [ + "geo": { + "geohash_grid": { + "field": "geo_point", + "precision": 12 + } + }, + { + "kw": { + "terms": { + "field": "keyword" + } + } + } + ] + after: { "geo": "upbpbpbpbpbp", "kw": "bar" } + + - match: {hits.total: 6} + - length: { aggregations.test.buckets: 3 } + - match: { aggregations.test.buckets.0.key.geo: "9qteuf21s29g" } + - match: { aggregations.test.buckets.0.key.kw: "foo" } + - match: { aggregations.test.buckets.0.doc_count: 1 } + - match: { aggregations.test.buckets.1.key.geo: "drm3btev3e86" } + - match: { aggregations.test.buckets.1.key.kw: "bar" } + - match: { aggregations.test.buckets.1.doc_count: 2 } + - match: { aggregations.test.buckets.2.key.geo: "drm3btev3e86" } + - match: { aggregations.test.buckets.2.key.kw: "foo" } + - match: { aggregations.test.buckets.2.doc_count: 1 } diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/CellIdSource.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/CellIdSource.java new file mode 100644 index 0000000000000..fb05225764d4c --- /dev/null +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/CellIdSource.java @@ -0,0 +1,110 @@ +/* + * 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.composite; + +import org.apache.lucene.index.LeafReaderContext; +import org.apache.lucene.index.SortedNumericDocValues; +import org.elasticsearch.index.fielddata.AbstractSortingNumericDocValues; +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. + */ +class CellIdSource extends ValuesSource.Numeric { + private final GeoPoint valuesSource; + private final int precision; + private final GeoPointLongEncoder encoder; + + CellIdSource(GeoPoint valuesSource, int precision, GeoPointLongEncoder encoder) { + this.valuesSource = valuesSource; + //different GeoPoints could map to the same or different hashing cells. + this.precision = precision; + this.encoder = encoder; + } + + int precision() { + return precision; + } + + GeoPointLongEncoder encoder() { + return encoder; + } + + @Override + public boolean isFloatingPoint() { + return false; + } + + @Override + public SortedNumericDocValues longValues(LeafReaderContext ctx) { + return new CellValues(valuesSource.geoPointValues(ctx), precision, encoder); + } + + @Override + public SortedNumericDoubleValues doubleValues(LeafReaderContext ctx) { + throw new UnsupportedOperationException(); + } + + @Override + public SortedBinaryDocValues bytesValues(LeafReaderContext ctx) { + throw new UnsupportedOperationException(); + } + + /** + * The encoder to use to convert a geopoint's (lon, lat, precision) into + * a long-encoded bucket key for aggregating. + */ + @FunctionalInterface + public interface GeoPointLongEncoder { + long encode(double lon, double lat, int precision); + } + + private static class CellValues extends AbstractSortingNumericDocValues { + private MultiGeoPointValues geoValues; + private int precision; + private GeoPointLongEncoder encoder; + + private 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; + } + } + } +} diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregator.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregator.java index cd7fd6abe8ca9..9c906bc51d8e6 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregator.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregator.java @@ -299,6 +299,20 @@ private SingleDimensionValuesSource createValuesSource(BigArrays bigArrays, I reverseMul ); + } else if (config.valuesSource() instanceof CellIdSource) { + final CellIdSource cis = (CellIdSource) config.valuesSource(); + return new GeohashValuesSource( + bigArrays, + config.fieldType(), + cis::longValues, + LongUnaryOperator.identity(), + config.format(), + config.missingBucket(), + size, + reverseMul, + cis.precision(), + cis.encoder() + ); } else if (config.valuesSource() instanceof ValuesSource.Numeric) { final ValuesSource.Numeric vs = (ValuesSource.Numeric) config.valuesSource(); if (vs.isFloatingPoint()) { diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeValuesSourceParserHelper.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeValuesSourceParserHelper.java index d773a09d645d3..e4b5744a760af 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeValuesSourceParserHelper.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeValuesSourceParserHelper.java @@ -19,6 +19,7 @@ package org.elasticsearch.search.aggregations.bucket.composite; +import org.elasticsearch.Version; import org.elasticsearch.common.ParseField; import org.elasticsearch.common.ParsingException; import org.elasticsearch.common.io.stream.StreamInput; @@ -67,6 +68,8 @@ public static void writeTo(CompositeValuesSourceBuilder builder, StreamOutput code = 1; } else if (builder.getClass() == HistogramValuesSourceBuilder.class) { code = 2; + } else if (builder.getClass() == GeoHashGridValuesSourceBuilder.class && out.getVersion().onOrAfter(Version.V_8_0_0)) { + code = 3; } else { throw new IOException("invalid builder type: " + builder.getClass().getSimpleName()); } @@ -83,6 +86,8 @@ public static CompositeValuesSourceBuilder readFrom(StreamInput in) throws IO return new DateHistogramValuesSourceBuilder(in); case 2: return new HistogramValuesSourceBuilder(in); + case 3: + return new GeoHashGridValuesSourceBuilder(in); default: throw new IOException("Invalid code " + code); } @@ -112,6 +117,9 @@ public static CompositeValuesSourceBuilder fromXContent(XContentParser parser case HistogramValuesSourceBuilder.TYPE: builder = HistogramValuesSourceBuilder.parse(name, parser); break; + case GeoHashGridValuesSourceBuilder.TYPE: + builder = GeoHashGridValuesSourceBuilder.parse(name, parser); + break; default: throw new ParsingException(parser.getTokenLocation(), "invalid source type: " + type); } diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/GeoHashGridValuesSourceBuilder.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/GeoHashGridValuesSourceBuilder.java new file mode 100644 index 0000000000000..d608a090f6275 --- /dev/null +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/GeoHashGridValuesSourceBuilder.java @@ -0,0 +1,109 @@ +package org.elasticsearch.search.aggregations.bucket.composite; + +import org.elasticsearch.common.ParseField; +import org.elasticsearch.common.geo.GeoUtils; +import org.elasticsearch.common.io.stream.StreamInput; +import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.common.xcontent.ObjectParser; +import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.geometry.utils.Geohash; +import org.elasticsearch.index.mapper.MappedFieldType; +import org.elasticsearch.search.DocValueFormat; +import org.elasticsearch.search.aggregations.bucket.geogrid.GeoHashGridAggregationBuilder; +import org.elasticsearch.search.aggregations.support.ValueType; +import org.elasticsearch.search.aggregations.support.ValuesSource; +import org.elasticsearch.search.aggregations.support.ValuesSourceConfig; +import org.elasticsearch.search.internal.SearchContext; + +import java.io.IOException; +import java.util.Objects; + +public class GeoHashGridValuesSourceBuilder extends CompositeValuesSourceBuilder { + static final String TYPE = "geohash_grid"; + + private static final ObjectParser PARSER; + static { + PARSER = new ObjectParser<>(GeoHashGridValuesSourceBuilder.TYPE); + PARSER.declareInt(GeoHashGridValuesSourceBuilder::precision, new ParseField("precision")); + PARSER.declareInt(GeoHashGridValuesSourceBuilder::shardSize, new ParseField("shard_size")); + CompositeValuesSourceParserHelper.declareValuesSourceFields(PARSER, ValueType.NUMERIC); + } + + static GeoHashGridValuesSourceBuilder parse(String name, XContentParser parser) throws IOException { + return PARSER.parse(parser, new GeoHashGridValuesSourceBuilder(name), null); + } + + private int precision = GeoHashGridAggregationBuilder.DEFAULT_PRECISION; + private int shardSize = -1; + + GeoHashGridValuesSourceBuilder(String name) { + super(name); + } + + GeoHashGridValuesSourceBuilder(StreamInput in) throws IOException { + super(in); + this.precision = in.readInt(); + this.shardSize = in.readInt(); + } + + public GeoHashGridValuesSourceBuilder precision(int precision) { + this.precision = GeoUtils.checkPrecisionRange(precision); + return this; + } + + public GeoHashGridValuesSourceBuilder shardSize(int shardSize) { + this.shardSize = shardSize; + return this; + } + + + @Override + protected void innerWriteTo(StreamOutput out) throws IOException { + out.writeInt(precision); + out.writeInt(shardSize); + } + + @Override + protected void doXContentBody(XContentBuilder builder, Params params) throws IOException { + builder.field("precision", precision); + builder.field("shard_size", shardSize); + } + + @Override + String type() { + return TYPE; + } + + @Override + public int hashCode() { + return Objects.hash(super.hashCode(), precision); + } + + @Override + public boolean equals(Object obj) { + if (this == obj) return true; + if (obj == null || getClass() != obj.getClass()) return false; + if (super.equals(obj) == false) return false; + GeoHashGridValuesSourceBuilder other = (GeoHashGridValuesSourceBuilder) obj; + return Objects.equals(precision, other.precision); + } + + + @Override + protected CompositeValuesSourceConfig innerBuild(SearchContext context, ValuesSourceConfig config) throws IOException { + ValuesSource orig = config.toValuesSource(context.getQueryShardContext()); + if (orig == null) { + orig = ValuesSource.GeoPoint.EMPTY; + } + if (orig instanceof ValuesSource.GeoPoint) { + 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, Geohash::longEncode); + return new CompositeValuesSourceConfig(name, fieldType, cellIdSource, DocValueFormat.GEOHASH, order(), missingBucket()); + } else { + throw new IllegalArgumentException("invalid source, expected numeric, got " + orig.getClass().getSimpleName()); + } + } +} diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/GeohashValuesSource.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/GeohashValuesSource.java new file mode 100644 index 0000000000000..fe3ac0ae47c75 --- /dev/null +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/GeohashValuesSource.java @@ -0,0 +1,52 @@ +package org.elasticsearch.search.aggregations.bucket.composite; + +import org.apache.lucene.index.LeafReaderContext; +import org.apache.lucene.index.SortedNumericDocValues; +import org.elasticsearch.common.CheckedFunction; +import org.elasticsearch.common.util.BigArrays; +import org.elasticsearch.geometry.Point; +import org.elasticsearch.geometry.utils.Geohash; +import org.elasticsearch.index.mapper.MappedFieldType; +import org.elasticsearch.search.DocValueFormat; + +import java.io.IOException; +import java.util.function.LongUnaryOperator; + +/** + * A {@link SingleDimensionValuesSource} for geohash values. + * + * Since geohash values can be represented as long values, this class is almost the same as {@link LongValuesSource} + * The main differences is {@link GeohashValuesSource#setAfter(Comparable)} as it needs to accept geohash string values. + */ +class GeohashValuesSource extends LongValuesSource { + private final int precision; + private final CellIdSource.GeoPointLongEncoder encoder; + GeohashValuesSource(BigArrays bigArrays, + MappedFieldType fieldType, + CheckedFunction docValuesFunc, + LongUnaryOperator rounding, + DocValueFormat format, + boolean missingBucket, + int size, + int reverseMul, + int precision, + CellIdSource.GeoPointLongEncoder encoder) { + super(bigArrays, fieldType, docValuesFunc, rounding, format, missingBucket, size, reverseMul); + this.precision = precision; + this.encoder = encoder; + } + + @Override + void setAfter(Comparable value) { + if (missingBucket && value == null) { + afterValue = null; + } else if (value instanceof Number) { + afterValue = ((Number) value).longValue(); + } else { + // if it is a string it should be a geohash formatted value. + // We need to preserve the precision between the decoding the geohash and encoding it into a long + Point point = Geohash.toPoint(value.toString()); + afterValue = encoder.encode(point.getLon(), point.getLat(), precision); + } + } +} diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregationBuilderTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregationBuilderTests.java index 08b8cb13a3377..4390f233c0ead 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregationBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregationBuilderTests.java @@ -51,6 +51,17 @@ private DateHistogramValuesSourceBuilder randomDateHistogramSourceBuilder() { return histo; } + private GeoHashGridValuesSourceBuilder randomGeoHashGridValuesSourceBuilder() { + GeoHashGridValuesSourceBuilder geoHash = new GeoHashGridValuesSourceBuilder(randomAlphaOfLengthBetween(5, 10)); + if (randomBoolean()) { + geoHash.precision(randomIntBetween(1, 12)); + } + if (randomBoolean()) { + geoHash.shardSize(randomIntBetween(0, 10_000)); + } + return geoHash; + } + private TermsValuesSourceBuilder randomTermsSourceBuilder() { TermsValuesSourceBuilder terms = new TermsValuesSourceBuilder(randomAlphaOfLengthBetween(5, 10)); if (randomBoolean()) { @@ -84,7 +95,7 @@ protected CompositeAggregationBuilder createTestAggregatorBuilder() { int numSources = randomIntBetween(1, 10); List> sources = new ArrayList<>(); for (int i = 0; i < numSources; i++) { - int type = randomIntBetween(0, 2); + int type = randomIntBetween(0, 3); switch (type) { case 0: sources.add(randomTermsSourceBuilder()); @@ -95,6 +106,9 @@ protected CompositeAggregationBuilder createTestAggregatorBuilder() { case 2: sources.add(randomHistogramSourceBuilder()); break; + case 3: + sources.add(randomGeoHashGridValuesSourceBuilder()); + break; default: throw new AssertionError("wrong branch"); } diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregatorTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregatorTests.java index 706638db1db02..22d9dc429897e 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregatorTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregatorTests.java @@ -24,6 +24,7 @@ import org.apache.lucene.document.Field; import org.apache.lucene.document.InetAddressPoint; import org.apache.lucene.document.IntPoint; +import org.apache.lucene.document.LatLonPoint; import org.apache.lucene.document.LongPoint; import org.apache.lucene.document.SortedNumericDocValuesField; import org.apache.lucene.document.SortedSetDocValuesField; @@ -39,9 +40,12 @@ import org.apache.lucene.util.BytesRef; import org.apache.lucene.util.NumericUtils; import org.elasticsearch.ElasticsearchParseException; +import org.elasticsearch.common.geo.GeoPoint; import org.elasticsearch.common.time.DateFormatters; +import org.elasticsearch.geometry.utils.Geohash; import org.elasticsearch.index.mapper.ContentPath; import org.elasticsearch.index.mapper.DateFieldMapper; +import org.elasticsearch.index.mapper.GeoPointFieldMapper; import org.elasticsearch.index.mapper.IpFieldMapper; import org.elasticsearch.index.mapper.KeywordFieldMapper; import org.elasticsearch.index.mapper.MappedFieldType; @@ -49,6 +53,7 @@ import org.elasticsearch.index.mapper.NumberFieldMapper; import org.elasticsearch.search.aggregations.Aggregator; import org.elasticsearch.search.aggregations.AggregatorTestCase; +import org.elasticsearch.search.aggregations.bucket.geogrid.GeoHashGridAggregationBuilder; import org.elasticsearch.search.aggregations.bucket.histogram.DateHistogramInterval; import org.elasticsearch.search.aggregations.bucket.terms.StringTerms; import org.elasticsearch.search.aggregations.bucket.terms.TermsAggregationBuilder; @@ -89,7 +94,7 @@ public class CompositeAggregatorTests extends AggregatorTestCase { @Before public void setUp() throws Exception { super.setUp(); - FIELD_TYPES = new MappedFieldType[7]; + FIELD_TYPES = new MappedFieldType[8]; FIELD_TYPES[0] = new KeywordFieldMapper.KeywordFieldType(); FIELD_TYPES[0].setName("keyword"); FIELD_TYPES[0].setHasDocValues(true); @@ -119,6 +124,10 @@ public void setUp() throws Exception { FIELD_TYPES[6] = new IpFieldMapper.IpFieldType(); FIELD_TYPES[6].setName("ip"); FIELD_TYPES[6].setHasDocValues(true); + + FIELD_TYPES[7] = new GeoPointFieldMapper.GeoPointFieldType(); + FIELD_TYPES[7].setName("geo_point"); + FIELD_TYPES[7].setHasDocValues(true); } @Override @@ -1788,6 +1797,55 @@ public void testWithIP() throws Exception { ); } + public void testWithGeoPoint() throws Exception { + final List>> dataset = new ArrayList<>(); + dataset.addAll( + Arrays.asList( + createDocument("geo_point", new GeoPoint(48.934059, 41.610741)), + createDocument("geo_point", new GeoPoint(-23.065941, 113.610741)), + createDocument("geo_point", new GeoPoint(90.0, 0.0)), + createDocument("geo_point", new GeoPoint(37.2343, -115.8067)), + createDocument("geo_point", new GeoPoint(90.0, 0.0)) + ) + ); + testSearchCase(Arrays.asList(new MatchAllDocsQuery(), new DocValuesFieldExistsQuery("geo_point")), dataset, + () -> { + GeoHashGridValuesSourceBuilder geoHash = new GeoHashGridValuesSourceBuilder("geo_point") + .field("geo_point"); + return new CompositeAggregationBuilder("name", Collections.singletonList(geoHash)); + }, (result) -> { + assertEquals(4, result.getBuckets().size()); + assertEquals("{geo_point=sb48j}", result.afterKey().toString()); + assertEquals("{geo_point=s218n}", result.getBuckets().get(0).getKeyAsString()); + assertEquals(1L, result.getBuckets().get(0).getDocCount()); + assertEquals("{geo_point=s8n2j}", result.getBuckets().get(1).getKeyAsString()); + assertEquals(1L, result.getBuckets().get(1).getDocCount()); + assertEquals("{geo_point=sb424}", result.getBuckets().get(2).getKeyAsString()); + assertEquals(1L, result.getBuckets().get(2).getDocCount()); + assertEquals("{geo_point=sb48j}", result.getBuckets().get(3).getKeyAsString()); + assertEquals(2L, result.getBuckets().get(3).getDocCount()); + } + ); + + testSearchCase(Arrays.asList(new MatchAllDocsQuery(), new DocValuesFieldExistsQuery("geo_point")), dataset, + () -> { + GeoHashGridValuesSourceBuilder geoHash = new GeoHashGridValuesSourceBuilder("geo_point") + .field("geo_point"); + return new CompositeAggregationBuilder("name", Collections.singletonList(geoHash)) + .aggregateAfter(Collections.singletonMap("geo_point", "s218n")); + }, (result) -> { + assertEquals(3, result.getBuckets().size()); + assertEquals("{geo_point=sb48j}", result.afterKey().toString()); + assertEquals("{geo_point=s8n2j}", result.getBuckets().get(0).getKeyAsString()); + assertEquals(1L, result.getBuckets().get(0).getDocCount()); + assertEquals("{geo_point=sb424}", result.getBuckets().get(1).getKeyAsString()); + assertEquals(1L, result.getBuckets().get(1).getDocCount()); + assertEquals("{geo_point=sb48j}", result.getBuckets().get(2).getKeyAsString()); + assertEquals(2L, result.getBuckets().get(2).getDocCount()); + } + ); + } + private void testSearchCase(List queries, List>> dataset, Supplier create, @@ -1845,6 +1903,11 @@ private void addToDocument(Document doc, Map> keys) { } else if (value instanceof InetAddress) { doc.add(new SortedSetDocValuesField(name, new BytesRef(InetAddressPoint.encode((InetAddress) value)))); doc.add(new InetAddressPoint(name, (InetAddress) value)); + } else if (value instanceof GeoPoint) { + GeoPoint point = (GeoPoint)value; + doc.add(new SortedNumericDocValuesField(name, + Geohash.longEncode(point.lon(), point.lat(), GeoHashGridAggregationBuilder.DEFAULT_PRECISION))); + doc.add(new LatLonPoint(name, point.lat(), point.lon())); } else { throw new AssertionError("invalid object: " + value.getClass().getSimpleName()); } From bb572f76ea1a6e1e89eec8a3eb2912523e8f956a Mon Sep 17 00:00:00 2001 From: Benjamin Trent <4357155+benwtrent@users.noreply.github.com> Date: Wed, 21 Aug 2019 12:30:40 -0500 Subject: [PATCH 02/12] adding license headers --- .../GeoHashGridValuesSourceBuilder.java | 25 ++++++++++++++++--- .../bucket/composite/GeohashValuesSource.java | 19 ++++++++++++++ 2 files changed, 40 insertions(+), 4 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/GeoHashGridValuesSourceBuilder.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/GeoHashGridValuesSourceBuilder.java index d608a090f6275..999cafca6bb18 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/GeoHashGridValuesSourceBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/GeoHashGridValuesSourceBuilder.java @@ -1,3 +1,22 @@ +/* + * 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.composite; import org.elasticsearch.common.ParseField; @@ -57,7 +76,6 @@ public GeoHashGridValuesSourceBuilder shardSize(int shardSize) { return this; } - @Override protected void innerWriteTo(StreamOutput out) throws IOException { out.writeInt(precision); @@ -77,7 +95,7 @@ String type() { @Override public int hashCode() { - return Objects.hash(super.hashCode(), precision); + return Objects.hash(super.hashCode(), precision, shardSize); } @Override @@ -86,10 +104,9 @@ public boolean equals(Object obj) { if (obj == null || getClass() != obj.getClass()) return false; if (super.equals(obj) == false) return false; GeoHashGridValuesSourceBuilder other = (GeoHashGridValuesSourceBuilder) obj; - return Objects.equals(precision, other.precision); + return precision == other.precision && shardSize == other.shardSize; } - @Override protected CompositeValuesSourceConfig innerBuild(SearchContext context, ValuesSourceConfig config) throws IOException { ValuesSource orig = config.toValuesSource(context.getQueryShardContext()); diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/GeohashValuesSource.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/GeohashValuesSource.java index fe3ac0ae47c75..7b5cd2ad6a714 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/GeohashValuesSource.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/GeohashValuesSource.java @@ -1,3 +1,22 @@ +/* + * 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.composite; import org.apache.lucene.index.LeafReaderContext; From 6cadfa4284ef860ff80de1e7b998d2183f4cfa0d Mon Sep 17 00:00:00 2001 From: Benjamin Trent <4357155+benwtrent@users.noreply.github.com> Date: Wed, 28 Aug 2019 10:21:33 -0500 Subject: [PATCH 03/12] making cellIDsource a public class per PR comments --- .../bucket/composite/CellIdSource.java | 110 ------------------ .../bucket/composite/CompositeAggregator.java | 1 + .../GeoHashGridValuesSourceBuilder.java | 1 + .../bucket/composite/GeohashValuesSource.java | 1 + .../bucket/geogrid/CellIdSource.java | 8 +- 5 files changed, 9 insertions(+), 112 deletions(-) delete mode 100644 server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/CellIdSource.java diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/CellIdSource.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/CellIdSource.java deleted file mode 100644 index fb05225764d4c..0000000000000 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/CellIdSource.java +++ /dev/null @@ -1,110 +0,0 @@ -/* - * 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.composite; - -import org.apache.lucene.index.LeafReaderContext; -import org.apache.lucene.index.SortedNumericDocValues; -import org.elasticsearch.index.fielddata.AbstractSortingNumericDocValues; -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. - */ -class CellIdSource extends ValuesSource.Numeric { - private final GeoPoint valuesSource; - private final int precision; - private final GeoPointLongEncoder encoder; - - CellIdSource(GeoPoint valuesSource, int precision, GeoPointLongEncoder encoder) { - this.valuesSource = valuesSource; - //different GeoPoints could map to the same or different hashing cells. - this.precision = precision; - this.encoder = encoder; - } - - int precision() { - return precision; - } - - GeoPointLongEncoder encoder() { - return encoder; - } - - @Override - public boolean isFloatingPoint() { - return false; - } - - @Override - public SortedNumericDocValues longValues(LeafReaderContext ctx) { - return new CellValues(valuesSource.geoPointValues(ctx), precision, encoder); - } - - @Override - public SortedNumericDoubleValues doubleValues(LeafReaderContext ctx) { - throw new UnsupportedOperationException(); - } - - @Override - public SortedBinaryDocValues bytesValues(LeafReaderContext ctx) { - throw new UnsupportedOperationException(); - } - - /** - * The encoder to use to convert a geopoint's (lon, lat, precision) into - * a long-encoded bucket key for aggregating. - */ - @FunctionalInterface - public interface GeoPointLongEncoder { - long encode(double lon, double lat, int precision); - } - - private static class CellValues extends AbstractSortingNumericDocValues { - private MultiGeoPointValues geoValues; - private int precision; - private GeoPointLongEncoder encoder; - - private 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; - } - } - } -} diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregator.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregator.java index 9c906bc51d8e6..0eea2e7eef271 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregator.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregator.java @@ -42,6 +42,7 @@ import org.elasticsearch.search.aggregations.MultiBucketCollector; import org.elasticsearch.search.aggregations.MultiBucketConsumerService; import org.elasticsearch.search.aggregations.bucket.BucketsAggregator; +import org.elasticsearch.search.aggregations.bucket.geogrid.CellIdSource; import org.elasticsearch.search.aggregations.pipeline.PipelineAggregator; import org.elasticsearch.search.aggregations.support.ValuesSource; import org.elasticsearch.search.internal.SearchContext; diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/GeoHashGridValuesSourceBuilder.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/GeoHashGridValuesSourceBuilder.java index 999cafca6bb18..1ec0e5645598e 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/GeoHashGridValuesSourceBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/GeoHashGridValuesSourceBuilder.java @@ -29,6 +29,7 @@ import org.elasticsearch.geometry.utils.Geohash; import org.elasticsearch.index.mapper.MappedFieldType; import org.elasticsearch.search.DocValueFormat; +import org.elasticsearch.search.aggregations.bucket.geogrid.CellIdSource; import org.elasticsearch.search.aggregations.bucket.geogrid.GeoHashGridAggregationBuilder; import org.elasticsearch.search.aggregations.support.ValueType; import org.elasticsearch.search.aggregations.support.ValuesSource; diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/GeohashValuesSource.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/GeohashValuesSource.java index 7b5cd2ad6a714..ef00aa2747748 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/GeohashValuesSource.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/GeohashValuesSource.java @@ -27,6 +27,7 @@ import org.elasticsearch.geometry.utils.Geohash; import org.elasticsearch.index.mapper.MappedFieldType; import org.elasticsearch.search.DocValueFormat; +import org.elasticsearch.search.aggregations.bucket.geogrid.CellIdSource; import java.io.IOException; import java.util.function.LongUnaryOperator; diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/CellIdSource.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/CellIdSource.java index 0cc7734ad7685..01ee27f019e86 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/CellIdSource.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/CellIdSource.java @@ -32,12 +32,12 @@ * Wrapper class to help convert {@link MultiGeoPointValues} * to numeric long values for bucketing. */ -class CellIdSource extends ValuesSource.Numeric { +public class CellIdSource extends ValuesSource.Numeric { private final ValuesSource.GeoPoint valuesSource; private final int precision; private final GeoPointLongEncoder encoder; - CellIdSource(GeoPoint valuesSource, int precision, GeoPointLongEncoder encoder) { + public CellIdSource(GeoPoint valuesSource, int precision, GeoPointLongEncoder encoder) { this.valuesSource = valuesSource; //different GeoPoints could map to the same or different hashing cells. this.precision = precision; @@ -48,6 +48,10 @@ public int precision() { return precision; } + public GeoPointLongEncoder encoder() { + return encoder; + } + @Override public boolean isFloatingPoint() { return false; From 9b21532daf2dc73158e8c6cbe844d7ece6efe24f Mon Sep 17 00:00:00 2001 From: Benjamin Trent <4357155+benwtrent@users.noreply.github.com> Date: Fri, 30 Aug 2019 08:38:42 -0500 Subject: [PATCH 04/12] Throwing error on bwc serialization failure, removing unused shardSize --- .../CompositeValuesSourceParserHelper.java | 6 +++++- .../composite/GeoHashGridValuesSourceBuilder.java | 14 ++------------ 2 files changed, 7 insertions(+), 13 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeValuesSourceParserHelper.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeValuesSourceParserHelper.java index e4b5744a760af..c43b0c29c5a2a 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeValuesSourceParserHelper.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeValuesSourceParserHelper.java @@ -68,7 +68,11 @@ public static void writeTo(CompositeValuesSourceBuilder builder, StreamOutput code = 1; } else if (builder.getClass() == HistogramValuesSourceBuilder.class) { code = 2; - } else if (builder.getClass() == GeoHashGridValuesSourceBuilder.class && out.getVersion().onOrAfter(Version.V_8_0_0)) { + } else if (builder.getClass() == GeoHashGridValuesSourceBuilder.class) { + if (out.getVersion().before(Version.V_8_0_0)) { + throw new IOException("Attempting to serialize [" + builder.getClass().getSimpleName() + + "] to a node with unsupported version [" + out.getVersion() + "]"); + } code = 3; } else { throw new IOException("invalid builder type: " + builder.getClass().getSimpleName()); diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/GeoHashGridValuesSourceBuilder.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/GeoHashGridValuesSourceBuilder.java index 1ec0e5645598e..c970762069e9d 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/GeoHashGridValuesSourceBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/GeoHashGridValuesSourceBuilder.java @@ -46,7 +46,6 @@ public class GeoHashGridValuesSourceBuilder extends CompositeValuesSourceBuilder static { PARSER = new ObjectParser<>(GeoHashGridValuesSourceBuilder.TYPE); PARSER.declareInt(GeoHashGridValuesSourceBuilder::precision, new ParseField("precision")); - PARSER.declareInt(GeoHashGridValuesSourceBuilder::shardSize, new ParseField("shard_size")); CompositeValuesSourceParserHelper.declareValuesSourceFields(PARSER, ValueType.NUMERIC); } @@ -55,7 +54,6 @@ static GeoHashGridValuesSourceBuilder parse(String name, XContentParser parser) } private int precision = GeoHashGridAggregationBuilder.DEFAULT_PRECISION; - private int shardSize = -1; GeoHashGridValuesSourceBuilder(String name) { super(name); @@ -64,7 +62,6 @@ static GeoHashGridValuesSourceBuilder parse(String name, XContentParser parser) GeoHashGridValuesSourceBuilder(StreamInput in) throws IOException { super(in); this.precision = in.readInt(); - this.shardSize = in.readInt(); } public GeoHashGridValuesSourceBuilder precision(int precision) { @@ -72,21 +69,14 @@ public GeoHashGridValuesSourceBuilder precision(int precision) { return this; } - public GeoHashGridValuesSourceBuilder shardSize(int shardSize) { - this.shardSize = shardSize; - return this; - } - @Override protected void innerWriteTo(StreamOutput out) throws IOException { out.writeInt(precision); - out.writeInt(shardSize); } @Override protected void doXContentBody(XContentBuilder builder, Params params) throws IOException { builder.field("precision", precision); - builder.field("shard_size", shardSize); } @Override @@ -96,7 +86,7 @@ String type() { @Override public int hashCode() { - return Objects.hash(super.hashCode(), precision, shardSize); + return Objects.hash(super.hashCode(), precision); } @Override @@ -105,7 +95,7 @@ public boolean equals(Object obj) { if (obj == null || getClass() != obj.getClass()) return false; if (super.equals(obj) == false) return false; GeoHashGridValuesSourceBuilder other = (GeoHashGridValuesSourceBuilder) obj; - return precision == other.precision && shardSize == other.shardSize; + return precision == other.precision; } @Override From ecdcce66a8e7d1db809e634a56d850e69fa41d77 Mon Sep 17 00:00:00 2001 From: Benjamin Trent <4357155+benwtrent@users.noreply.github.com> Date: Fri, 30 Aug 2019 08:57:06 -0500 Subject: [PATCH 05/12] removing bad method call --- .../bucket/composite/CompositeAggregationBuilderTests.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregationBuilderTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregationBuilderTests.java index 4390f233c0ead..9a0ba90c8be72 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregationBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregationBuilderTests.java @@ -56,9 +56,6 @@ private GeoHashGridValuesSourceBuilder randomGeoHashGridValuesSourceBuilder() { if (randomBoolean()) { geoHash.precision(randomIntBetween(1, 12)); } - if (randomBoolean()) { - geoHash.shardSize(randomIntBetween(0, 10_000)); - } return geoHash; } From 2e4461d6cf53935b2769cd8092713e26fffb5201 Mon Sep 17 00:00:00 2001 From: Benjamin Trent <4357155+benwtrent@users.noreply.github.com> Date: Fri, 30 Aug 2019 10:46:05 -0500 Subject: [PATCH 06/12] correcting error message --- .../bucket/composite/GeoHashGridValuesSourceBuilder.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/GeoHashGridValuesSourceBuilder.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/GeoHashGridValuesSourceBuilder.java index c970762069e9d..69a95116617b7 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/GeoHashGridValuesSourceBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/GeoHashGridValuesSourceBuilder.java @@ -111,7 +111,7 @@ protected CompositeValuesSourceConfig innerBuild(SearchContext context, ValuesSo CellIdSource cellIdSource = new CellIdSource(geoPoint, precision, Geohash::longEncode); return new CompositeValuesSourceConfig(name, fieldType, cellIdSource, DocValueFormat.GEOHASH, order(), missingBucket()); } else { - throw new IllegalArgumentException("invalid source, expected numeric, got " + orig.getClass().getSimpleName()); + throw new IllegalArgumentException("invalid source, expected geo_point, got " + orig.getClass().getSimpleName()); } } } From 022e66d9226920a587139489de69c89c92f38402 Mon Sep 17 00:00:00 2001 From: Benjamin Trent <4357155+benwtrent@users.noreply.github.com> Date: Fri, 30 Aug 2019 12:55:40 -0500 Subject: [PATCH 07/12] making .format(String) throw IAE --- .../GeoHashGridValuesSourceBuilder.java | 5 +++ .../GeoHashGridValuesSourceBuilderTests.java | 31 +++++++++++++++++++ 2 files changed, 36 insertions(+) create mode 100644 server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/GeoHashGridValuesSourceBuilderTests.java diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/GeoHashGridValuesSourceBuilder.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/GeoHashGridValuesSourceBuilder.java index 69a95116617b7..3d2e4983a0d5b 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/GeoHashGridValuesSourceBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/GeoHashGridValuesSourceBuilder.java @@ -69,6 +69,11 @@ public GeoHashGridValuesSourceBuilder precision(int precision) { return this; } + @Override + public GeoHashGridValuesSourceBuilder format(String format) { + throw new IllegalArgumentException("[format] is not supported for [" + TYPE + "]"); + } + @Override protected void innerWriteTo(StreamOutput out) throws IOException { out.writeInt(precision); diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/GeoHashGridValuesSourceBuilderTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/GeoHashGridValuesSourceBuilderTests.java new file mode 100644 index 0000000000000..db02578855b6b --- /dev/null +++ b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/GeoHashGridValuesSourceBuilderTests.java @@ -0,0 +1,31 @@ +/* + * 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.composite; + +import org.elasticsearch.test.ESTestCase; + +public class GeoHashGridValuesSourceBuilderTests extends ESTestCase { + + public void testSetFormat() { + CompositeValuesSourceBuilder builder = new GeoHashGridValuesSourceBuilder("name"); + expectThrows(IllegalArgumentException.class, () -> builder.format("format")); + } + +} From ff518c982cc2eea1268d3fddbd764bb8111f963f Mon Sep 17 00:00:00 2001 From: Benjamin Trent <4357155+benwtrent@users.noreply.github.com> Date: Tue, 3 Sep 2019 19:13:21 -0500 Subject: [PATCH 08/12] move to use geotile instead of hash --- .../test/search.aggregation/230_composite.yml | 50 +++++++-------- .../bucket/composite/CompositeAggregator.java | 7 +-- .../CompositeValuesSourceParserHelper.java | 8 +-- ...va => GeoTileGridValuesSourceBuilder.java} | 60 ++++++++++++------ ...esSource.java => GeoTileValuesSource.java} | 27 +++----- .../GeoTileGridAggregationBuilder.java | 2 +- .../bucket/geogrid/GeoTileUtils.java | 62 +++++++++++++------ .../CompositeAggregationBuilderTests.java | 10 +-- .../composite/CompositeAggregatorTests.java | 38 +++++------- ... GeoTileGridValuesSourceBuilderTests.java} | 4 +- .../bucket/geogrid/GeoTileUtilsTests.java | 25 ++++++++ 11 files changed, 172 insertions(+), 121 deletions(-) rename server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/{GeoHashGridValuesSourceBuilder.java => GeoTileGridValuesSourceBuilder.java} (69%) rename server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/{GeohashValuesSource.java => GeoTileValuesSource.java} (63%) rename server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/{GeoHashGridValuesSourceBuilderTests.java => GeoTileGridValuesSourceBuilderTests.java} (89%) diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/search.aggregation/230_composite.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/search.aggregation/230_composite.yml index 1c3726978859f..78277cd2b6701 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/search.aggregation/230_composite.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/search.aggregation/230_composite.yml @@ -618,10 +618,10 @@ setup: ] --- -"Simple Composite aggregation with Geohash grid": +"Simple Composite aggregation with GeoTile grid": - skip: version: " - 7.99.99" - reason: geohash_grid is not supported until 8.0.0 + reason: geotile_grid is not supported until 8.0.0 - do: search: rest_total_hits_as_int: true @@ -632,7 +632,7 @@ setup: composite: sources: [ "geo": { - "geohash_grid": { + "geotile_grid": { "field": "geo_point", "precision": 12 } @@ -648,23 +648,23 @@ setup: - match: {hits.total: 6} - length: { aggregations.test.buckets: 4 } - - match: { aggregations.test.buckets.0.key.geo: "upbpbpbpbpbp" } - - match: { aggregations.test.buckets.0.key.kw: "bar" } + - match: { aggregations.test.buckets.0.key.geo: "12/730/1590" } + - match: { aggregations.test.buckets.0.key.kw: "foo" } - match: { aggregations.test.buckets.0.doc_count: 1 } - - match: { aggregations.test.buckets.1.key.geo: "9qteuf21s29g" } - - match: { aggregations.test.buckets.1.key.kw: "foo" } - - match: { aggregations.test.buckets.1.doc_count: 1 } - - match: { aggregations.test.buckets.2.key.geo: "drm3btev3e86" } - - match: { aggregations.test.buckets.2.key.kw: "bar" } - - match: { aggregations.test.buckets.2.doc_count: 2 } - - match: { aggregations.test.buckets.3.key.geo: "drm3btev3e86" } - - match: { aggregations.test.buckets.3.key.kw: "foo" } + - match: { aggregations.test.buckets.1.key.geo: "12/1236/1533" } + - match: { aggregations.test.buckets.1.key.kw: "bar" } + - match: { aggregations.test.buckets.1.doc_count: 2 } + - match: { aggregations.test.buckets.2.key.geo: "12/1236/1533" } + - match: { aggregations.test.buckets.2.key.kw: "foo" } + - match: { aggregations.test.buckets.2.doc_count: 1 } + - match: { aggregations.test.buckets.3.key.geo: "12/2048/0" } + - match: { aggregations.test.buckets.3.key.kw: "bar" } - match: { aggregations.test.buckets.3.doc_count: 1 } --- -"Simple Composite aggregation with geohash grid add aggregate after": +"Simple Composite aggregation with geotile grid add aggregate after": - skip: version: " - 7.99.99" - reason: geohash_grid is not supported until 8.0.0 + reason: geotile_grid is not supported until 8.0.0 - do: search: rest_total_hits_as_int: true @@ -675,7 +675,7 @@ setup: composite: sources: [ "geo": { - "geohash_grid": { + "geotile_grid": { "field": "geo_point", "precision": 12 } @@ -688,16 +688,16 @@ setup: } } ] - after: { "geo": "upbpbpbpbpbp", "kw": "bar" } + after: { "geo": "12/730/1590", "kw": "foo" } - match: {hits.total: 6} - length: { aggregations.test.buckets: 3 } - - match: { aggregations.test.buckets.0.key.geo: "9qteuf21s29g" } - - match: { aggregations.test.buckets.0.key.kw: "foo" } - - match: { aggregations.test.buckets.0.doc_count: 1 } - - match: { aggregations.test.buckets.1.key.geo: "drm3btev3e86" } - - match: { aggregations.test.buckets.1.key.kw: "bar" } - - match: { aggregations.test.buckets.1.doc_count: 2 } - - match: { aggregations.test.buckets.2.key.geo: "drm3btev3e86" } - - match: { aggregations.test.buckets.2.key.kw: "foo" } + - match: { aggregations.test.buckets.0.key.geo: "12/1236/1533" } + - match: { aggregations.test.buckets.0.key.kw: "bar" } + - match: { aggregations.test.buckets.0.doc_count: 2 } + - match: { aggregations.test.buckets.1.key.geo: "12/1236/1533" } + - match: { aggregations.test.buckets.1.key.kw: "foo" } + - match: { aggregations.test.buckets.1.doc_count: 1 } + - match: { aggregations.test.buckets.2.key.geo: "12/2048/0" } + - match: { aggregations.test.buckets.2.key.kw: "bar" } - match: { aggregations.test.buckets.2.doc_count: 1 } diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregator.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregator.java index 0eea2e7eef271..4effb22f30cb2 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregator.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregator.java @@ -302,7 +302,7 @@ private SingleDimensionValuesSource createValuesSource(BigArrays bigArrays, I } else if (config.valuesSource() instanceof CellIdSource) { final CellIdSource cis = (CellIdSource) config.valuesSource(); - return new GeohashValuesSource( + return new GeoTileValuesSource( bigArrays, config.fieldType(), cis::longValues, @@ -310,10 +310,7 @@ private SingleDimensionValuesSource createValuesSource(BigArrays bigArrays, I config.format(), config.missingBucket(), size, - reverseMul, - cis.precision(), - cis.encoder() - ); + reverseMul); } else if (config.valuesSource() instanceof ValuesSource.Numeric) { final ValuesSource.Numeric vs = (ValuesSource.Numeric) config.valuesSource(); if (vs.isFloatingPoint()) { diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeValuesSourceParserHelper.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeValuesSourceParserHelper.java index c43b0c29c5a2a..4ca96ea577c14 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeValuesSourceParserHelper.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeValuesSourceParserHelper.java @@ -68,7 +68,7 @@ public static void writeTo(CompositeValuesSourceBuilder builder, StreamOutput code = 1; } else if (builder.getClass() == HistogramValuesSourceBuilder.class) { code = 2; - } else if (builder.getClass() == GeoHashGridValuesSourceBuilder.class) { + } else if (builder.getClass() == GeoTileGridValuesSourceBuilder.class) { if (out.getVersion().before(Version.V_8_0_0)) { throw new IOException("Attempting to serialize [" + builder.getClass().getSimpleName() + "] to a node with unsupported version [" + out.getVersion() + "]"); @@ -91,7 +91,7 @@ public static CompositeValuesSourceBuilder readFrom(StreamInput in) throws IO case 2: return new HistogramValuesSourceBuilder(in); case 3: - return new GeoHashGridValuesSourceBuilder(in); + return new GeoTileGridValuesSourceBuilder(in); default: throw new IOException("Invalid code " + code); } @@ -121,8 +121,8 @@ public static CompositeValuesSourceBuilder fromXContent(XContentParser parser case HistogramValuesSourceBuilder.TYPE: builder = HistogramValuesSourceBuilder.parse(name, parser); break; - case GeoHashGridValuesSourceBuilder.TYPE: - builder = GeoHashGridValuesSourceBuilder.parse(name, parser); + case GeoTileGridValuesSourceBuilder.TYPE: + builder = GeoTileGridValuesSourceBuilder.parse(name, parser); break; default: throw new ParsingException(parser.getTokenLocation(), "invalid source type: " + type); diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/GeoHashGridValuesSourceBuilder.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/GeoTileGridValuesSourceBuilder.java similarity index 69% rename from server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/GeoHashGridValuesSourceBuilder.java rename to server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/GeoTileGridValuesSourceBuilder.java index 3d2e4983a0d5b..6fed5afc96665 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/GeoHashGridValuesSourceBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/GeoTileGridValuesSourceBuilder.java @@ -20,17 +20,16 @@ package org.elasticsearch.search.aggregations.bucket.composite; import org.elasticsearch.common.ParseField; -import org.elasticsearch.common.geo.GeoUtils; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.xcontent.ObjectParser; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; -import org.elasticsearch.geometry.utils.Geohash; import org.elasticsearch.index.mapper.MappedFieldType; import org.elasticsearch.search.DocValueFormat; import org.elasticsearch.search.aggregations.bucket.geogrid.CellIdSource; -import org.elasticsearch.search.aggregations.bucket.geogrid.GeoHashGridAggregationBuilder; +import org.elasticsearch.search.aggregations.bucket.geogrid.GeoTileGridAggregationBuilder; +import org.elasticsearch.search.aggregations.bucket.geogrid.GeoTileUtils; import org.elasticsearch.search.aggregations.support.ValueType; import org.elasticsearch.search.aggregations.support.ValuesSource; import org.elasticsearch.search.aggregations.support.ValuesSourceConfig; @@ -39,38 +38,38 @@ import java.io.IOException; import java.util.Objects; -public class GeoHashGridValuesSourceBuilder extends CompositeValuesSourceBuilder { - static final String TYPE = "geohash_grid"; +public class GeoTileGridValuesSourceBuilder extends CompositeValuesSourceBuilder { + static final String TYPE = "geotile_grid"; - private static final ObjectParser PARSER; + private static final ObjectParser PARSER; static { - PARSER = new ObjectParser<>(GeoHashGridValuesSourceBuilder.TYPE); - PARSER.declareInt(GeoHashGridValuesSourceBuilder::precision, new ParseField("precision")); + PARSER = new ObjectParser<>(GeoTileGridValuesSourceBuilder.TYPE); + PARSER.declareInt(GeoTileGridValuesSourceBuilder::precision, new ParseField("precision")); CompositeValuesSourceParserHelper.declareValuesSourceFields(PARSER, ValueType.NUMERIC); } - static GeoHashGridValuesSourceBuilder parse(String name, XContentParser parser) throws IOException { - return PARSER.parse(parser, new GeoHashGridValuesSourceBuilder(name), null); + static GeoTileGridValuesSourceBuilder parse(String name, XContentParser parser) throws IOException { + return PARSER.parse(parser, new GeoTileGridValuesSourceBuilder(name), null); } - private int precision = GeoHashGridAggregationBuilder.DEFAULT_PRECISION; + private int precision = GeoTileGridAggregationBuilder.DEFAULT_PRECISION; - GeoHashGridValuesSourceBuilder(String name) { + GeoTileGridValuesSourceBuilder(String name) { super(name); } - GeoHashGridValuesSourceBuilder(StreamInput in) throws IOException { + GeoTileGridValuesSourceBuilder(StreamInput in) throws IOException { super(in); this.precision = in.readInt(); } - public GeoHashGridValuesSourceBuilder precision(int precision) { - this.precision = GeoUtils.checkPrecisionRange(precision); + public GeoTileGridValuesSourceBuilder precision(int precision) { + this.precision = GeoTileUtils.checkPrecisionRange(precision); return this; } @Override - public GeoHashGridValuesSourceBuilder format(String format) { + public GeoTileGridValuesSourceBuilder format(String format) { throw new IllegalArgumentException("[format] is not supported for [" + TYPE + "]"); } @@ -99,7 +98,7 @@ public boolean equals(Object obj) { if (this == obj) return true; if (obj == null || getClass() != obj.getClass()) return false; if (super.equals(obj) == false) return false; - GeoHashGridValuesSourceBuilder other = (GeoHashGridValuesSourceBuilder) obj; + GeoTileGridValuesSourceBuilder other = (GeoTileGridValuesSourceBuilder) obj; return precision == other.precision; } @@ -113,10 +112,33 @@ protected CompositeValuesSourceConfig innerBuild(SearchContext context, ValuesSo 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, Geohash::longEncode); - return new CompositeValuesSourceConfig(name, fieldType, cellIdSource, DocValueFormat.GEOHASH, order(), missingBucket()); + CellIdSource cellIdSource = new CellIdSource(geoPoint, precision, GeoTileUtils::longEncode); + return new CompositeValuesSourceConfig(name, fieldType, cellIdSource, GEOTILE, order(), missingBucket()); } else { throw new IllegalArgumentException("invalid source, expected geo_point, got " + orig.getClass().getSimpleName()); } } + + DocValueFormat GEOTILE = new DocValueFormat() { + + @Override + public String getWriteableName() { + return "geo_tile"; + } + + @Override + public void writeTo(StreamOutput out) { + } + + @Override + public String format(long value) { + return GeoTileUtils.stringEncode(value); + } + + @Override + public String format(double value) { + return format((long) value); + } + }; + } diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/GeohashValuesSource.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/GeoTileValuesSource.java similarity index 63% rename from server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/GeohashValuesSource.java rename to server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/GeoTileValuesSource.java index ef00aa2747748..b7517f0577409 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/GeohashValuesSource.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/GeoTileValuesSource.java @@ -23,37 +23,29 @@ import org.apache.lucene.index.SortedNumericDocValues; import org.elasticsearch.common.CheckedFunction; import org.elasticsearch.common.util.BigArrays; -import org.elasticsearch.geometry.Point; -import org.elasticsearch.geometry.utils.Geohash; import org.elasticsearch.index.mapper.MappedFieldType; import org.elasticsearch.search.DocValueFormat; -import org.elasticsearch.search.aggregations.bucket.geogrid.CellIdSource; +import org.elasticsearch.search.aggregations.bucket.geogrid.GeoTileUtils; import java.io.IOException; import java.util.function.LongUnaryOperator; /** - * A {@link SingleDimensionValuesSource} for geohash values. + * A {@link SingleDimensionValuesSource} for geotile values. * - * Since geohash values can be represented as long values, this class is almost the same as {@link LongValuesSource} - * The main differences is {@link GeohashValuesSource#setAfter(Comparable)} as it needs to accept geohash string values. + * Since geotile values can be represented as long values, this class is almost the same as {@link LongValuesSource} + * The main differences is {@link GeoTileValuesSource#setAfter(Comparable)} as it needs to accept geotile string values i.e. "zoom/x/y". */ -class GeohashValuesSource extends LongValuesSource { - private final int precision; - private final CellIdSource.GeoPointLongEncoder encoder; - GeohashValuesSource(BigArrays bigArrays, +class GeoTileValuesSource extends LongValuesSource { + GeoTileValuesSource(BigArrays bigArrays, MappedFieldType fieldType, CheckedFunction docValuesFunc, LongUnaryOperator rounding, DocValueFormat format, boolean missingBucket, int size, - int reverseMul, - int precision, - CellIdSource.GeoPointLongEncoder encoder) { + int reverseMul) { super(bigArrays, fieldType, docValuesFunc, rounding, format, missingBucket, size, reverseMul); - this.precision = precision; - this.encoder = encoder; } @Override @@ -63,10 +55,7 @@ void setAfter(Comparable value) { } else if (value instanceof Number) { afterValue = ((Number) value).longValue(); } else { - // if it is a string it should be a geohash formatted value. - // We need to preserve the precision between the decoding the geohash and encoding it into a long - Point point = Geohash.toPoint(value.toString()); - afterValue = encoder.encode(point.getLon(), point.getLat(), precision); + afterValue = GeoTileUtils.longEncode(value.toString()); } } } diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoTileGridAggregationBuilder.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoTileGridAggregationBuilder.java index 374b7ddf70735..966bb81a9f856 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoTileGridAggregationBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoTileGridAggregationBuilder.java @@ -35,7 +35,7 @@ public class GeoTileGridAggregationBuilder extends GeoGridAggregationBuilder { public static final String NAME = "geotile_grid"; - private static final int DEFAULT_PRECISION = 7; + public static final int DEFAULT_PRECISION = 7; private static final int DEFAULT_MAX_NUM_CELLS = 10000; private static final ObjectParser PARSER = createParser(NAME, GeoTileUtils::parsePrecision); diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoTileUtils.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoTileUtils.java index d85cf6b1a56ce..53c980daf5568 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoTileUtils.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoTileUtils.java @@ -38,7 +38,7 @@ * bits 29..57 -- X tile index (0..2^zoom) * bits 0..28 -- Y tile index (0..2^zoom) */ -final class GeoTileUtils { +public final class GeoTileUtils { private GeoTileUtils() {} @@ -80,7 +80,7 @@ static int parsePrecision(XContentParser parser) throws IOException, Elasticsear /** * Assert the precision value is within the allowed range, and return it if ok, or throw. */ - static int checkPrecisionRange(int precision) { + public static int checkPrecisionRange(int precision) { if (precision < 0 || precision > MAX_ZOOM) { throw new IllegalArgumentException("Invalid geotile_grid precision of " + precision + ". Must be between 0 and " + MAX_ZOOM + "."); @@ -93,7 +93,7 @@ static int checkPrecisionRange(int precision) { * The resulting hash contains interleaved tile X and Y coordinates. * The precision itself is also encoded as a few high bits. */ - static long longEncode(double longitude, double latitude, int precision) { + public static long longEncode(double longitude, double latitude, int precision) { // Mathematics for this code was adapted from https://wiki.openstreetmap.org/wiki/Slippy_map_tilenames#Java // Number of tiles for the current zoom level along the X and Y axis @@ -119,10 +119,18 @@ static long longEncode(double longitude, double latitude, int precision) { yTile = tiles - 1; } - // Zoom value is placed in front of all the bits used for the geotile - // e.g. when max zoom is 29, the largest index would use 58 bits (57th..0th), - // leaving 5 bits unused for zoom. See MAX_ZOOM comment above. - return ((long) precision << ZOOM_SHIFT) | (xTile << MAX_ZOOM) | yTile; + return longEncode((long) precision, xTile, yTile); + } + + /** + * Encode a geotile hash as a string. + * + * @param hashAsString String in format "zoom/x/y" + * @return long encoded value of the given hash + */ + public static long longEncode(String hashAsString) { + int[] parsed = parseHash(hashAsString); + return longEncode((long)parsed[0], (long)parsed[1], (long)parsed[2]); } /** @@ -135,10 +143,34 @@ private static int[] parseHash(long hash) { return new int[]{zoom, xTile, yTile}; } + private static long longEncode(long precision, long xTile, long yTile) { + // Zoom value is placed in front of all the bits used for the geotile + // e.g. when max zoom is 29, the largest index would use 58 bits (57th..0th), + // leaving 5 bits unused for zoom. See MAX_ZOOM comment above. + return (precision << ZOOM_SHIFT) | (xTile << MAX_ZOOM) | yTile; + } + + /** + * Parse geotile String hash format in "zoom/x/y" into an array of integers + */ + private static int[] parseHash(String hashAsString) { + final String[] parts = hashAsString.split("/", 4); + if (parts.length != 3) { + throw new IllegalArgumentException("Invalid geotile_grid hash string of " + + hashAsString + ". Must be three integers in a form \"zoom/x/y\"."); + } + try { + return new int[]{Integer.parseInt(parts[0]), Integer.parseInt(parts[1]), Integer.parseInt(parts[2])}; + } catch (NumberFormatException e) { + throw new IllegalArgumentException("Invalid geotile_grid hash string of " + + hashAsString + ". Must be three integers in a form \"zoom/x/y\".", e); + } + } + /** * Encode to a geotile string from the geotile based long format */ - static String stringEncode(long hash) { + public static String stringEncode(long hash) { int[] res = parseHash(hash); validateZXY(res[0], res[1], res[2]); return "" + res[0] + "/" + res[1] + "/" + res[2]; @@ -156,18 +188,8 @@ static GeoPoint hashToGeoPoint(long hash) { * Decode a string bucket key in "zoom/x/y" format to a GeoPoint (center of the tile) */ static GeoPoint keyToGeoPoint(String hashAsString) { - final String[] parts = hashAsString.split("/", 4); - if (parts.length != 3) { - throw new IllegalArgumentException("Invalid geotile_grid hash string of " + - hashAsString + ". Must be three integers in a form \"zoom/x/y\"."); - } - - try { - return zxyToGeoPoint(Integer.parseInt(parts[0]), Integer.parseInt(parts[1]), Integer.parseInt(parts[2])); - } catch (NumberFormatException e) { - throw new IllegalArgumentException("Invalid geotile_grid hash string of " + - hashAsString + ". Must be three integers in a form \"zoom/x/y\".", e); - } + int[] hashAsInts = parseHash(hashAsString); + return zxyToGeoPoint(hashAsInts[0], hashAsInts[1], hashAsInts[2]); } /** diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregationBuilderTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregationBuilderTests.java index 9a0ba90c8be72..c78a55b253297 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregationBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregationBuilderTests.java @@ -51,12 +51,12 @@ private DateHistogramValuesSourceBuilder randomDateHistogramSourceBuilder() { return histo; } - private GeoHashGridValuesSourceBuilder randomGeoHashGridValuesSourceBuilder() { - GeoHashGridValuesSourceBuilder geoHash = new GeoHashGridValuesSourceBuilder(randomAlphaOfLengthBetween(5, 10)); + private GeoTileGridValuesSourceBuilder randomGeoTileGridValuesSourceBuilder() { + GeoTileGridValuesSourceBuilder geoTile = new GeoTileGridValuesSourceBuilder(randomAlphaOfLengthBetween(5, 10)); if (randomBoolean()) { - geoHash.precision(randomIntBetween(1, 12)); + geoTile.precision(randomIntBetween(1, 12)); } - return geoHash; + return geoTile; } private TermsValuesSourceBuilder randomTermsSourceBuilder() { @@ -104,7 +104,7 @@ protected CompositeAggregationBuilder createTestAggregatorBuilder() { sources.add(randomHistogramSourceBuilder()); break; case 3: - sources.add(randomGeoHashGridValuesSourceBuilder()); + sources.add(randomGeoTileGridValuesSourceBuilder()); break; default: throw new AssertionError("wrong branch"); diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregatorTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregatorTests.java index 22d9dc429897e..88fd00818f397 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregatorTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregatorTests.java @@ -53,7 +53,7 @@ import org.elasticsearch.index.mapper.NumberFieldMapper; import org.elasticsearch.search.aggregations.Aggregator; import org.elasticsearch.search.aggregations.AggregatorTestCase; -import org.elasticsearch.search.aggregations.bucket.geogrid.GeoHashGridAggregationBuilder; +import org.elasticsearch.search.aggregations.bucket.geogrid.GeoTileGridAggregationBuilder; import org.elasticsearch.search.aggregations.bucket.histogram.DateHistogramInterval; import org.elasticsearch.search.aggregations.bucket.terms.StringTerms; import org.elasticsearch.search.aggregations.bucket.terms.TermsAggregationBuilder; @@ -1810,38 +1810,34 @@ public void testWithGeoPoint() throws Exception { ); testSearchCase(Arrays.asList(new MatchAllDocsQuery(), new DocValuesFieldExistsQuery("geo_point")), dataset, () -> { - GeoHashGridValuesSourceBuilder geoHash = new GeoHashGridValuesSourceBuilder("geo_point") + GeoTileGridValuesSourceBuilder geoTile = new GeoTileGridValuesSourceBuilder("geo_point") .field("geo_point"); - return new CompositeAggregationBuilder("name", Collections.singletonList(geoHash)); + return new CompositeAggregationBuilder("name", Collections.singletonList(geoTile)); }, (result) -> { - assertEquals(4, result.getBuckets().size()); - assertEquals("{geo_point=sb48j}", result.afterKey().toString()); - assertEquals("{geo_point=s218n}", result.getBuckets().get(0).getKeyAsString()); - assertEquals(1L, result.getBuckets().get(0).getDocCount()); - assertEquals("{geo_point=s8n2j}", result.getBuckets().get(1).getKeyAsString()); + assertEquals(3, result.getBuckets().size()); + assertEquals("{geo_point=7/108/63}", result.afterKey().toString()); + assertEquals("{geo_point=7/21/63}", result.getBuckets().get(0).getKeyAsString()); + assertEquals(3L, result.getBuckets().get(0).getDocCount()); + assertEquals("{geo_point=7/44/63}", result.getBuckets().get(1).getKeyAsString()); assertEquals(1L, result.getBuckets().get(1).getDocCount()); - assertEquals("{geo_point=sb424}", result.getBuckets().get(2).getKeyAsString()); + assertEquals("{geo_point=7/108/63}", result.getBuckets().get(2).getKeyAsString()); assertEquals(1L, result.getBuckets().get(2).getDocCount()); - assertEquals("{geo_point=sb48j}", result.getBuckets().get(3).getKeyAsString()); - assertEquals(2L, result.getBuckets().get(3).getDocCount()); } ); testSearchCase(Arrays.asList(new MatchAllDocsQuery(), new DocValuesFieldExistsQuery("geo_point")), dataset, () -> { - GeoHashGridValuesSourceBuilder geoHash = new GeoHashGridValuesSourceBuilder("geo_point") + GeoTileGridValuesSourceBuilder geoTile = new GeoTileGridValuesSourceBuilder("geo_point") .field("geo_point"); - return new CompositeAggregationBuilder("name", Collections.singletonList(geoHash)) - .aggregateAfter(Collections.singletonMap("geo_point", "s218n")); + return new CompositeAggregationBuilder("name", Collections.singletonList(geoTile)) + .aggregateAfter(Collections.singletonMap("geo_point", "7/21/63")); }, (result) -> { - assertEquals(3, result.getBuckets().size()); - assertEquals("{geo_point=sb48j}", result.afterKey().toString()); - assertEquals("{geo_point=s8n2j}", result.getBuckets().get(0).getKeyAsString()); + assertEquals(2, result.getBuckets().size()); + assertEquals("{geo_point=7/108/63}", result.afterKey().toString()); + assertEquals("{geo_point=7/44/63}", result.getBuckets().get(0).getKeyAsString()); assertEquals(1L, result.getBuckets().get(0).getDocCount()); - assertEquals("{geo_point=sb424}", result.getBuckets().get(1).getKeyAsString()); + assertEquals("{geo_point=7/108/63}", result.getBuckets().get(1).getKeyAsString()); assertEquals(1L, result.getBuckets().get(1).getDocCount()); - assertEquals("{geo_point=sb48j}", result.getBuckets().get(2).getKeyAsString()); - assertEquals(2L, result.getBuckets().get(2).getDocCount()); } ); } @@ -1906,7 +1902,7 @@ private void addToDocument(Document doc, Map> keys) { } else if (value instanceof GeoPoint) { GeoPoint point = (GeoPoint)value; doc.add(new SortedNumericDocValuesField(name, - Geohash.longEncode(point.lon(), point.lat(), GeoHashGridAggregationBuilder.DEFAULT_PRECISION))); + Geohash.longEncode(point.lon(), point.lat(), GeoTileGridAggregationBuilder.DEFAULT_PRECISION))); doc.add(new LatLonPoint(name, point.lat(), point.lon())); } else { throw new AssertionError("invalid object: " + value.getClass().getSimpleName()); diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/GeoHashGridValuesSourceBuilderTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/GeoTileGridValuesSourceBuilderTests.java similarity index 89% rename from server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/GeoHashGridValuesSourceBuilderTests.java rename to server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/GeoTileGridValuesSourceBuilderTests.java index db02578855b6b..6f9e0f697da25 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/GeoHashGridValuesSourceBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/GeoTileGridValuesSourceBuilderTests.java @@ -21,10 +21,10 @@ import org.elasticsearch.test.ESTestCase; -public class GeoHashGridValuesSourceBuilderTests extends ESTestCase { +public class GeoTileGridValuesSourceBuilderTests extends ESTestCase { public void testSetFormat() { - CompositeValuesSourceBuilder builder = new GeoHashGridValuesSourceBuilder("name"); + CompositeValuesSourceBuilder builder = new GeoTileGridValuesSourceBuilder("name"); expectThrows(IllegalArgumentException.class, () -> builder.format("format")); } diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoTileUtilsTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoTileUtilsTests.java index e2881fd9b9145..fc5cf6cb910bd 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoTileUtilsTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoTileUtilsTests.java @@ -76,6 +76,31 @@ public void testLongEncode() { expectThrows(IllegalArgumentException.class, () -> longEncode(-1, 0, MAX_ZOOM + 1)); } + public void testLongEncodeFromString() { + assertEquals(0x0000000000000000L, longEncode(stringEncode(longEncode(0, 0, 0)))); + assertEquals(0x3C00095540001CA5L, longEncode(stringEncode(longEncode(30, 70, 15)))); + assertEquals(0x77FFFF4580000000L, longEncode(stringEncode(longEncode(179.999, 89.999, 29)))); + assertEquals(0x740000BA7FFFFFFFL, longEncode(stringEncode(longEncode(-179.999, -89.999, 29)))); + assertEquals(0x0800000040000001L, longEncode(stringEncode(longEncode(1, 1, 2)))); + assertEquals(0x0C00000060000000L, longEncode(stringEncode(longEncode(-20, 100, 3)))); + assertEquals(0x71127D27C8ACA67AL, longEncode(stringEncode(longEncode(13, -15, 28)))); + assertEquals(0x4C0077776003A9ACL, longEncode(stringEncode(longEncode(-12, 15, 19)))); + assertEquals(0x140000024000000EL, longEncode(stringEncode(longEncode(-328.231870,16.064082, 5)))); + assertEquals(0x6436F96B60000000L, longEncode(stringEncode(longEncode(-590.769588,89.549167, 25)))); + assertEquals(0x6411BD6BA0A98359L, longEncode(stringEncode(longEncode(999.787079,51.830093, 25)))); + assertEquals(0x751BD6BBCA983596L, longEncode(stringEncode(longEncode(999.787079,51.830093, 29)))); + assertEquals(0x77CF880A20000000L, longEncode(stringEncode(longEncode(-557.039740,-632.103969, 29)))); + assertEquals(0x7624FA4FA0000000L, longEncode(stringEncode(longEncode(13,88, 29)))); + assertEquals(0x7624FA4FBFFFFFFFL, longEncode(stringEncode(longEncode(13,-88, 29)))); + assertEquals(0x0400000020000000L, longEncode(stringEncode(longEncode(13,89, 1)))); + assertEquals(0x0400000020000001L, longEncode(stringEncode(longEncode(13,-89, 1)))); + assertEquals(0x0400000020000000L, longEncode(stringEncode(longEncode(13,95, 1)))); + assertEquals(0x0400000020000001L, longEncode(stringEncode(longEncode(13,-95, 1)))); + + expectThrows(IllegalArgumentException.class, () -> longEncode("12/asdf/1")); + expectThrows(IllegalArgumentException.class, () -> longEncode("foo")); + } + private void assertGeoPointEquals(GeoPoint gp, final double longitude, final double latitude) { assertThat(gp.lon(), closeTo(longitude, GEOTILE_TOLERANCE)); assertThat(gp.lat(), closeTo(latitude, GEOTILE_TOLERANCE)); From 54c9e12e49d6dfe5514a3c9630b5f6563282ef5a Mon Sep 17 00:00:00 2001 From: Benjamin Trent <4357155+benwtrent@users.noreply.github.com> Date: Tue, 3 Sep 2019 19:18:50 -0500 Subject: [PATCH 09/12] minor fixups --- .../search/aggregations/bucket/geogrid/CellIdSource.java | 4 ---- .../search/aggregations/bucket/geogrid/GeoTileUtils.java | 4 ++-- .../bucket/composite/CompositeAggregatorTests.java | 4 ++-- 3 files changed, 4 insertions(+), 8 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/CellIdSource.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/CellIdSource.java index 01ee27f019e86..4ebb689c7c44f 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/CellIdSource.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/CellIdSource.java @@ -48,10 +48,6 @@ public int precision() { return precision; } - public GeoPointLongEncoder encoder() { - return encoder; - } - @Override public boolean isFloatingPoint() { return false; diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoTileUtils.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoTileUtils.java index 53c980daf5568..c417be016288d 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoTileUtils.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoTileUtils.java @@ -123,10 +123,10 @@ public static long longEncode(double longitude, double latitude, int precision) } /** - * Encode a geotile hash as a string. + * Encode a geotile hash style string to a long. * * @param hashAsString String in format "zoom/x/y" - * @return long encoded value of the given hash + * @return long encoded value of the given string hash */ public static long longEncode(String hashAsString) { int[] parsed = parseHash(hashAsString); diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregatorTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregatorTests.java index 88fd00818f397..ff0edcd7d1df8 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregatorTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregatorTests.java @@ -42,7 +42,6 @@ import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.common.geo.GeoPoint; import org.elasticsearch.common.time.DateFormatters; -import org.elasticsearch.geometry.utils.Geohash; import org.elasticsearch.index.mapper.ContentPath; import org.elasticsearch.index.mapper.DateFieldMapper; import org.elasticsearch.index.mapper.GeoPointFieldMapper; @@ -54,6 +53,7 @@ import org.elasticsearch.search.aggregations.Aggregator; import org.elasticsearch.search.aggregations.AggregatorTestCase; import org.elasticsearch.search.aggregations.bucket.geogrid.GeoTileGridAggregationBuilder; +import org.elasticsearch.search.aggregations.bucket.geogrid.GeoTileUtils; import org.elasticsearch.search.aggregations.bucket.histogram.DateHistogramInterval; import org.elasticsearch.search.aggregations.bucket.terms.StringTerms; import org.elasticsearch.search.aggregations.bucket.terms.TermsAggregationBuilder; @@ -1902,7 +1902,7 @@ private void addToDocument(Document doc, Map> keys) { } else if (value instanceof GeoPoint) { GeoPoint point = (GeoPoint)value; doc.add(new SortedNumericDocValuesField(name, - Geohash.longEncode(point.lon(), point.lat(), GeoTileGridAggregationBuilder.DEFAULT_PRECISION))); + GeoTileUtils.longEncode(point.lon(), point.lat(), GeoTileGridAggregationBuilder.DEFAULT_PRECISION))); doc.add(new LatLonPoint(name, point.lat(), point.lon())); } else { throw new AssertionError("invalid object: " + value.getClass().getSimpleName()); From c30cc32d52fd3332c0756d76e3e8e502dbe033fa Mon Sep 17 00:00:00 2001 From: Benjamin Trent <4357155+benwtrent@users.noreply.github.com> Date: Tue, 3 Sep 2019 19:51:21 -0500 Subject: [PATCH 10/12] fixing test --- .../composite/CompositeAggregatorTests.java | 26 ++++++++----------- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregatorTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregatorTests.java index ff0edcd7d1df8..1520dfde8a116 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregatorTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregatorTests.java @@ -1814,14 +1814,12 @@ public void testWithGeoPoint() throws Exception { .field("geo_point"); return new CompositeAggregationBuilder("name", Collections.singletonList(geoTile)); }, (result) -> { - assertEquals(3, result.getBuckets().size()); - assertEquals("{geo_point=7/108/63}", result.afterKey().toString()); - assertEquals("{geo_point=7/21/63}", result.getBuckets().get(0).getKeyAsString()); - assertEquals(3L, result.getBuckets().get(0).getDocCount()); - assertEquals("{geo_point=7/44/63}", result.getBuckets().get(1).getKeyAsString()); - assertEquals(1L, result.getBuckets().get(1).getDocCount()); - assertEquals("{geo_point=7/108/63}", result.getBuckets().get(2).getKeyAsString()); - assertEquals(1L, result.getBuckets().get(2).getDocCount()); + assertEquals(2, result.getBuckets().size()); + assertEquals("{geo_point=7/64/56}", result.afterKey().toString()); + assertEquals("{geo_point=7/32/56}", result.getBuckets().get(0).getKeyAsString()); + assertEquals(2L, result.getBuckets().get(0).getDocCount()); + assertEquals("{geo_point=7/64/56}", result.getBuckets().get(1).getKeyAsString()); + assertEquals(3L, result.getBuckets().get(1).getDocCount()); } ); @@ -1830,14 +1828,12 @@ public void testWithGeoPoint() throws Exception { GeoTileGridValuesSourceBuilder geoTile = new GeoTileGridValuesSourceBuilder("geo_point") .field("geo_point"); return new CompositeAggregationBuilder("name", Collections.singletonList(geoTile)) - .aggregateAfter(Collections.singletonMap("geo_point", "7/21/63")); + .aggregateAfter(Collections.singletonMap("geo_point", "7/32/56")); }, (result) -> { - assertEquals(2, result.getBuckets().size()); - assertEquals("{geo_point=7/108/63}", result.afterKey().toString()); - assertEquals("{geo_point=7/44/63}", result.getBuckets().get(0).getKeyAsString()); - assertEquals(1L, result.getBuckets().get(0).getDocCount()); - assertEquals("{geo_point=7/108/63}", result.getBuckets().get(1).getKeyAsString()); - assertEquals(1L, result.getBuckets().get(1).getDocCount()); + assertEquals(1, result.getBuckets().size()); + assertEquals("{geo_point=7/64/56}", result.afterKey().toString()); + assertEquals("{geo_point=7/64/56}", result.getBuckets().get(0).getKeyAsString()); + assertEquals(3L, result.getBuckets().get(0).getDocCount()); } ); } From 2a09dc1e5a500343a351fc8d3913b9bbc51e6373 Mon Sep 17 00:00:00 2001 From: Benjamin Trent <4357155+benwtrent@users.noreply.github.com> Date: Wed, 4 Sep 2019 06:58:37 -0500 Subject: [PATCH 11/12] moving DocValueFormat for geotile --- .../GeoTileGridValuesSourceBuilder.java | 44 +++++++++---------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/GeoTileGridValuesSourceBuilder.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/GeoTileGridValuesSourceBuilder.java index 6fed5afc96665..96246c49be24c 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/GeoTileGridValuesSourceBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/GeoTileGridValuesSourceBuilder.java @@ -41,6 +41,28 @@ public class GeoTileGridValuesSourceBuilder extends CompositeValuesSourceBuilder { static final String TYPE = "geotile_grid"; + private static final DocValueFormat GEOTILE = new DocValueFormat() { + + @Override + public String getWriteableName() { + return "geo_tile"; + } + + @Override + public void writeTo(StreamOutput out) { + } + + @Override + public String format(long value) { + return GeoTileUtils.stringEncode(value); + } + + @Override + public String format(double value) { + return format((long) value); + } + }; + private static final ObjectParser PARSER; static { PARSER = new ObjectParser<>(GeoTileGridValuesSourceBuilder.TYPE); @@ -119,26 +141,4 @@ protected CompositeValuesSourceConfig innerBuild(SearchContext context, ValuesSo } } - DocValueFormat GEOTILE = new DocValueFormat() { - - @Override - public String getWriteableName() { - return "geo_tile"; - } - - @Override - public void writeTo(StreamOutput out) { - } - - @Override - public String format(long value) { - return GeoTileUtils.stringEncode(value); - } - - @Override - public String format(double value) { - return format((long) value); - } - }; - } From 5ecd671a9e0da487214a018154cfcbc30ec9f5d7 Mon Sep 17 00:00:00 2001 From: Benjamin Trent <4357155+benwtrent@users.noreply.github.com> Date: Wed, 4 Sep 2019 08:34:27 -0500 Subject: [PATCH 12/12] moving geotile so that it can be serialized --- .../elasticsearch/search/DocValueFormat.java | 23 ++++++++++++++++++ .../elasticsearch/search/SearchModule.java | 1 + .../GeoTileGridValuesSourceBuilder.java | 24 +------------------ .../search/DocValueFormatTests.java | 18 ++++++++++++++ .../search/SearchSortValuesTests.java | 7 +++++- 5 files changed, 49 insertions(+), 24 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/search/DocValueFormat.java b/server/src/main/java/org/elasticsearch/search/DocValueFormat.java index ad7699d9f9c22..684b403a0f0f6 100644 --- a/server/src/main/java/org/elasticsearch/search/DocValueFormat.java +++ b/server/src/main/java/org/elasticsearch/search/DocValueFormat.java @@ -30,6 +30,7 @@ import org.elasticsearch.common.time.DateMathParser; import org.elasticsearch.geometry.utils.Geohash; import org.elasticsearch.index.mapper.DateFieldMapper; +import org.elasticsearch.search.aggregations.bucket.geogrid.GeoTileUtils; import java.io.IOException; import java.net.InetAddress; @@ -246,6 +247,28 @@ public String format(double value) { } }; + DocValueFormat GEOTILE = new DocValueFormat() { + + @Override + public String getWriteableName() { + return "geo_tile"; + } + + @Override + public void writeTo(StreamOutput out) { + } + + @Override + public String format(long value) { + return GeoTileUtils.stringEncode(value); + } + + @Override + public String format(double value) { + return format((long) value); + } + }; + DocValueFormat BOOLEAN = new DocValueFormat() { @Override diff --git a/server/src/main/java/org/elasticsearch/search/SearchModule.java b/server/src/main/java/org/elasticsearch/search/SearchModule.java index 01a4aa66810c8..025f150330995 100644 --- a/server/src/main/java/org/elasticsearch/search/SearchModule.java +++ b/server/src/main/java/org/elasticsearch/search/SearchModule.java @@ -667,6 +667,7 @@ private void registerValueFormats() { registerValueFormat(DocValueFormat.DateTime.NAME, DocValueFormat.DateTime::new); registerValueFormat(DocValueFormat.Decimal.NAME, DocValueFormat.Decimal::new); registerValueFormat(DocValueFormat.GEOHASH.getWriteableName(), in -> DocValueFormat.GEOHASH); + registerValueFormat(DocValueFormat.GEOTILE.getWriteableName(), in -> DocValueFormat.GEOTILE); registerValueFormat(DocValueFormat.IP.getWriteableName(), in -> DocValueFormat.IP); registerValueFormat(DocValueFormat.RAW.getWriteableName(), in -> DocValueFormat.RAW); registerValueFormat(DocValueFormat.BINARY.getWriteableName(), in -> DocValueFormat.BINARY); diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/GeoTileGridValuesSourceBuilder.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/GeoTileGridValuesSourceBuilder.java index 96246c49be24c..36debb1e40fb5 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/GeoTileGridValuesSourceBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/GeoTileGridValuesSourceBuilder.java @@ -41,28 +41,6 @@ public class GeoTileGridValuesSourceBuilder extends CompositeValuesSourceBuilder { static final String TYPE = "geotile_grid"; - private static final DocValueFormat GEOTILE = new DocValueFormat() { - - @Override - public String getWriteableName() { - return "geo_tile"; - } - - @Override - public void writeTo(StreamOutput out) { - } - - @Override - public String format(long value) { - return GeoTileUtils.stringEncode(value); - } - - @Override - public String format(double value) { - return format((long) value); - } - }; - private static final ObjectParser PARSER; static { PARSER = new ObjectParser<>(GeoTileGridValuesSourceBuilder.TYPE); @@ -135,7 +113,7 @@ protected CompositeValuesSourceConfig innerBuild(SearchContext context, ValuesSo // is specified in the builder. final MappedFieldType fieldType = config.fieldContext() != null ? config.fieldContext().fieldType() : null; CellIdSource cellIdSource = new CellIdSource(geoPoint, precision, GeoTileUtils::longEncode); - return new CompositeValuesSourceConfig(name, fieldType, cellIdSource, GEOTILE, order(), missingBucket()); + return new CompositeValuesSourceConfig(name, fieldType, cellIdSource, DocValueFormat.GEOTILE, order(), missingBucket()); } else { throw new IllegalArgumentException("invalid source, expected geo_point, got " + orig.getClass().getSimpleName()); } diff --git a/server/src/test/java/org/elasticsearch/search/DocValueFormatTests.java b/server/src/test/java/org/elasticsearch/search/DocValueFormatTests.java index cd31487aeb519..df948a5bff71d 100644 --- a/server/src/test/java/org/elasticsearch/search/DocValueFormatTests.java +++ b/server/src/test/java/org/elasticsearch/search/DocValueFormatTests.java @@ -35,6 +35,8 @@ import java.util.ArrayList; import java.util.List; +import static org.elasticsearch.search.aggregations.bucket.geogrid.GeoTileUtils.longEncode; + public class DocValueFormatTests extends ESTestCase { public void testSerialization() throws Exception { @@ -43,6 +45,7 @@ public void testSerialization() throws Exception { entries.add(new Entry(DocValueFormat.class, DocValueFormat.DateTime.NAME, DocValueFormat.DateTime::new)); entries.add(new Entry(DocValueFormat.class, DocValueFormat.Decimal.NAME, DocValueFormat.Decimal::new)); entries.add(new Entry(DocValueFormat.class, DocValueFormat.GEOHASH.getWriteableName(), in -> DocValueFormat.GEOHASH)); + entries.add(new Entry(DocValueFormat.class, DocValueFormat.GEOTILE.getWriteableName(), in -> DocValueFormat.GEOTILE)); entries.add(new Entry(DocValueFormat.class, DocValueFormat.IP.getWriteableName(), in -> DocValueFormat.IP)); entries.add(new Entry(DocValueFormat.class, DocValueFormat.RAW.getWriteableName(), in -> DocValueFormat.RAW)); entries.add(new Entry(DocValueFormat.class, DocValueFormat.BINARY.getWriteableName(), in -> DocValueFormat.BINARY)); @@ -87,6 +90,11 @@ public void testSerialization() throws Exception { in = new NamedWriteableAwareStreamInput(out.bytes().streamInput(), registry); assertSame(DocValueFormat.GEOHASH, in.readNamedWriteable(DocValueFormat.class)); + out = new BytesStreamOutput(); + out.writeNamedWriteable(DocValueFormat.GEOTILE); + in = new NamedWriteableAwareStreamInput(out.bytes().streamInput(), registry); + assertSame(DocValueFormat.GEOTILE, in.readNamedWriteable(DocValueFormat.class)); + out = new BytesStreamOutput(); out.writeNamedWriteable(DocValueFormat.IP); in = new NamedWriteableAwareStreamInput(out.bytes().streamInput(), registry); @@ -147,6 +155,16 @@ public void testDecimalFormat() { assertEquals("859,802.354", formatter.format(0.8598023539251286d * 1_000_000)); } + public void testGeoTileFormat() { + assertEquals("0/0/0", DocValueFormat.GEOTILE.format(longEncode(0, 0, 0))); + assertEquals("15/19114/7333", DocValueFormat.GEOTILE.format(longEncode(30, 70, 15))); + assertEquals("29/536869420/0", DocValueFormat.GEOTILE.format(longEncode(179.999, 89.999, 29))); + assertEquals("29/1491/536870911", DocValueFormat.GEOTILE.format(longEncode(-179.999, -89.999, 29))); + assertEquals("2/2/1", DocValueFormat.GEOTILE.format(longEncode(1, 1, 2))); + assertEquals("1/1/0", DocValueFormat.GEOTILE.format(longEncode(13,95, 1))); + assertEquals("1/1/1", DocValueFormat.GEOTILE.format(longEncode(13,-95, 1))); + } + public void testRawParse() { assertEquals(-1L, DocValueFormat.RAW.parseLong("-1", randomBoolean(), null)); assertEquals(1L, DocValueFormat.RAW.parseLong("1", randomBoolean(), null)); diff --git a/server/src/test/java/org/elasticsearch/search/SearchSortValuesTests.java b/server/src/test/java/org/elasticsearch/search/SearchSortValuesTests.java index 940a34344a4f6..8cec44618c90b 100644 --- a/server/src/test/java/org/elasticsearch/search/SearchSortValuesTests.java +++ b/server/src/test/java/org/elasticsearch/search/SearchSortValuesTests.java @@ -67,7 +67,12 @@ private static Object randomSortValue(XContentType xContentType, boolean transpo } private static DocValueFormat randomDocValueFormat() { - return randomFrom(DocValueFormat.BOOLEAN, DocValueFormat.RAW, DocValueFormat.IP, DocValueFormat.BINARY, DocValueFormat.GEOHASH); + return randomFrom(DocValueFormat.BOOLEAN, + DocValueFormat.RAW, + DocValueFormat.IP, + DocValueFormat.BINARY, + DocValueFormat.GEOHASH, + DocValueFormat.GEOTILE); } @Override