From d13edc0671f6a16b58ff22cf26304882eb585929 Mon Sep 17 00:00:00 2001 From: iverase Date: Mon, 10 May 2021 15:07:41 +0200 Subject: [PATCH 1/7] Add painless script support for Geoshape field --- .../painless/spi/org.elasticsearch.txt | 7 + .../search/geo/GeoPointScriptDocValuesIT.java | 166 ++++++++++++++++++ .../index/fielddata/ScriptDocValues.java | 69 +++++++- .../search/GeoShapeScriptDocValuesIT.java | 126 +++++++++++++ .../index/fielddata/GeoShapeValues.java | 31 ++-- .../AbstractAtomicGeoShapeShapeFieldData.java | 50 +++++- .../LatLonShapeDVAtomicShapeFieldData.java | 8 +- .../xpack/spatial/util/GeoTestUtils.java | 7 +- 8 files changed, 440 insertions(+), 24 deletions(-) create mode 100644 server/src/internalClusterTest/java/org/elasticsearch/search/geo/GeoPointScriptDocValuesIT.java create mode 100644 x-pack/plugin/spatial/src/internalClusterTest/java/org/elasticsearch/xpack/spatial/search/GeoShapeScriptDocValuesIT.java diff --git a/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/org.elasticsearch.txt b/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/org.elasticsearch.txt index 03d28c297d5bd..4763bdf8e4648 100644 --- a/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/org.elasticsearch.txt +++ b/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/org.elasticsearch.txt @@ -148,6 +148,13 @@ class org.elasticsearch.index.fielddata.ScriptDocValues$Doubles { double getValue() } +class org.elasticsearch.index.fielddata.ScriptDocValues$Geometry { + double getCentroidLat(); + double getCentroidLon(); + double width(); + double height(); +} + class org.elasticsearch.index.fielddata.ScriptDocValues$GeoPoints { org.elasticsearch.common.geo.GeoPoint get(int) org.elasticsearch.common.geo.GeoPoint getValue() diff --git a/server/src/internalClusterTest/java/org/elasticsearch/search/geo/GeoPointScriptDocValuesIT.java b/server/src/internalClusterTest/java/org/elasticsearch/search/geo/GeoPointScriptDocValuesIT.java new file mode 100644 index 0000000000000..61bd2a66d1c58 --- /dev/null +++ b/server/src/internalClusterTest/java/org/elasticsearch/search/geo/GeoPointScriptDocValuesIT.java @@ -0,0 +1,166 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +package org.elasticsearch.search.geo; + +import org.apache.lucene.geo.GeoEncodingUtils; +import org.elasticsearch.action.search.SearchResponse; +import org.elasticsearch.common.document.DocumentField; +import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.common.xcontent.XContentFactory; +import org.elasticsearch.geo.GeometryTestUtils; +import org.elasticsearch.index.fielddata.ScriptDocValues; +import org.elasticsearch.plugins.Plugin; +import org.elasticsearch.script.MockScriptPlugin; +import org.elasticsearch.script.Script; +import org.elasticsearch.script.ScriptType; +import org.elasticsearch.test.ESSingleNodeTestCase; +import org.junit.Before; + +import java.io.IOException; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; +import java.util.function.Function; + +import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder; +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchResponse; +import static org.hamcrest.Matchers.equalTo; + +public class GeoPointScriptDocValuesIT extends ESSingleNodeTestCase { + + @Override + protected Collection> getPlugins() { + return Arrays.asList(CustomScriptPlugin.class); + } + + public static class CustomScriptPlugin extends MockScriptPlugin { + + @Override + protected Map, Object>> pluginScripts() { + Map, Object>> scripts = new HashMap<>(); + + scripts.put("lat", vars -> script(vars, ScriptDocValues.Geometry::getCentroidLat)); + scripts.put("lon", vars -> script(vars, ScriptDocValues.Geometry::getCentroidLon)); + scripts.put("height", vars -> script(vars, ScriptDocValues.Geometry::height)); + scripts.put("width", vars -> script(vars, ScriptDocValues.Geometry::width)); + return scripts; + } + + static Double script(Map vars, Function, Double> distance) { + Map doc = (Map) vars.get("doc"); + return distance.apply((ScriptDocValues.Geometry) doc.get("location")); + } + } + + @Override + protected boolean forbidPrivateIndexSettings() { + return false; + } + + @Before + public void setupTestIndex() throws IOException { + XContentBuilder xContentBuilder = XContentFactory.jsonBuilder().startObject().startObject("_doc") + .startObject("properties").startObject("location").field("type", "geo_point"); + xContentBuilder.endObject().endObject().endObject().endObject(); + assertAcked(client().admin().indices().prepareCreate("test").setMapping(xContentBuilder)); + ensureGreen(); + } + + public void testRandomPoint() throws Exception { + final double lat = GeometryTestUtils.randomLat(); + final double lon = GeometryTestUtils.randomLon(); + client().prepareIndex("test").setId("1") + .setSource(jsonBuilder().startObject() + .field("name", "TestPosition") + .field("location", new double[]{lon, lat}) + .endObject()) + .get(); + + client().admin().indices().prepareRefresh("test").get(); + + SearchResponse searchResponse = client().prepareSearch().addStoredField("_source") + .addScriptField("lat", new Script(ScriptType.INLINE, CustomScriptPlugin.NAME, "lat", Collections.emptyMap())) + .addScriptField("lon", new Script(ScriptType.INLINE, CustomScriptPlugin.NAME, "lon", Collections.emptyMap())) + .addScriptField("height", new Script(ScriptType.INLINE, CustomScriptPlugin.NAME, "height", Collections.emptyMap())) + .addScriptField("width", new Script(ScriptType.INLINE, CustomScriptPlugin.NAME, "width", Collections.emptyMap())) + .get(); + assertSearchResponse(searchResponse); + + final double qLat = GeoEncodingUtils.decodeLatitude(GeoEncodingUtils.encodeLatitude(lat)); + final double qLon = GeoEncodingUtils.decodeLongitude(GeoEncodingUtils.encodeLongitude(lon)); + + Map fields = searchResponse.getHits().getHits()[0].getFields(); + assertThat(fields.get("lat").getValue(), equalTo(qLat)); + assertThat(fields.get("lon").getValue(), equalTo(qLon)); + assertThat(fields.get("height").getValue(), equalTo(0d)); + assertThat(fields.get("width").getValue(), equalTo(0d)); + + client().prepareDelete("test", "1").get(); + client().admin().indices().prepareRefresh("test").get(); + } + + public void testRandomMultiPoint() throws Exception { + final int size = randomIntBetween(2, 20); + final double[] lats = new double[size]; + final double[] lons = new double[size]; + for (int i = 0; i < size; i++) { + lats[i] = GeometryTestUtils.randomLat(); + lons[i] = GeometryTestUtils.randomLon(); + } + + final double[][] values = new double[size][]; + for (int i = 0; i < size; i++) { + values[i] = new double[]{lons[i], lats[i]}; + } + + XContentBuilder builder = jsonBuilder().startObject() + .field("name", "TestPosition") + .field("location", values).endObject(); + client().prepareIndex("test").setId("1").setSource(builder).get(); + + client().admin().indices().prepareRefresh("test").get(); + + SearchResponse searchResponse = client().prepareSearch().addStoredField("_source") + .addScriptField("lat", new Script(ScriptType.INLINE, CustomScriptPlugin.NAME, "lat", Collections.emptyMap())) + .addScriptField("lon", new Script(ScriptType.INLINE, CustomScriptPlugin.NAME, "lon", Collections.emptyMap())) + .addScriptField("height", new Script(ScriptType.INLINE, CustomScriptPlugin.NAME, "height", Collections.emptyMap())) + .addScriptField("width", new Script(ScriptType.INLINE, CustomScriptPlugin.NAME, "width", Collections.emptyMap())) + .get(); + assertSearchResponse(searchResponse); + + for (int i = 0; i < size; i++) { + lats[i] = GeoEncodingUtils.decodeLatitude(GeoEncodingUtils.encodeLatitude(lats[i])); + lons[i] = GeoEncodingUtils.decodeLongitude(GeoEncodingUtils.encodeLongitude(lons[i])); + } + + final double centroidLon = Arrays.stream(lons).sum() / size; + final double centroidLat = Arrays.stream(lats).sum() / size; + final double width = Arrays.stream(lons).max().getAsDouble() - Arrays.stream(lons).min().getAsDouble(); + final double height = Arrays.stream(lats).max().getAsDouble() - Arrays.stream(lats).min().getAsDouble(); + + Map fields = searchResponse.getHits().getHits()[0].getFields(); + assertThat(fields.get("lat").getValue(), equalTo(centroidLat)); + assertThat(fields.get("lon").getValue(), equalTo(centroidLon)); + assertThat(fields.get("height").getValue(), equalTo(height)); + assertThat(fields.get("width").getValue(), equalTo(width)); + + client().prepareDelete("test", "1").get(); + client().admin().indices().prepareRefresh("test").get(); + } +} diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/ScriptDocValues.java b/server/src/main/java/org/elasticsearch/index/fielddata/ScriptDocValues.java index 652e290e72721..9561f4db12d66 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/ScriptDocValues.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/ScriptDocValues.java @@ -247,7 +247,18 @@ public int size() { } } - public static final class GeoPoints extends ScriptDocValues { + public abstract static class Geometry extends ScriptDocValues { + /** Centroid latitude */ + public abstract double getCentroidLat(); + /** Centroid longitude */ + public abstract double getCentroidLon(); + /** width of the geometry in degrees */ + public abstract double width(); + /** height of the geometry in degrees */ + public abstract double height(); + } + + public static final class GeoPoints extends Geometry { private final MultiGeoPointValues in; private GeoPoint[] values = new GeoPoint[0]; @@ -364,6 +375,62 @@ public double geohashDistanceWithDefault(String geohash, double defaultValue) { } return geohashDistance(geohash); } + + @Override + public double getCentroidLat() { + if (count == 1) { + return getLat(); + } else { + double centroidLat = 0; + for (int i = 0; i < count; i++) { + centroidLat += values[i].getLat(); + } + return centroidLat / count; + } + } + + @Override + public double getCentroidLon() { + if (count == 1) { + return getLon(); + } else { + double centroidLat = 0; + for (int i = 0; i < count; i++) { + centroidLat += values[i].getLon(); + } + return centroidLat / count; + } + } + + @Override + public double width() { + if (count == 1) { + return 0; + } else { + double maxLon = Double.NEGATIVE_INFINITY; + double minLon = Double.POSITIVE_INFINITY; + for (int i = 0; i < count; i++) { + maxLon = Math.max(maxLon, values[i].getLon()); + minLon = Math.min(minLon, values[i].getLon()); + } + return maxLon - minLon; + } + } + + @Override + public double height() { + if (count == 1) { + return 0; + } else { + double maxLat = Double.NEGATIVE_INFINITY; + double minLat = Double.POSITIVE_INFINITY; + for (int i = 0; i < count; i++) { + maxLat = Math.max(maxLat, values[i].getLat()); + minLat = Math.min(minLat, values[i].getLat()); + } + return maxLat - minLat; + } + } } public static final class Booleans extends ScriptDocValues { diff --git a/x-pack/plugin/spatial/src/internalClusterTest/java/org/elasticsearch/xpack/spatial/search/GeoShapeScriptDocValuesIT.java b/x-pack/plugin/spatial/src/internalClusterTest/java/org/elasticsearch/xpack/spatial/search/GeoShapeScriptDocValuesIT.java new file mode 100644 index 0000000000000..82e05c325ff11 --- /dev/null +++ b/x-pack/plugin/spatial/src/internalClusterTest/java/org/elasticsearch/xpack/spatial/search/GeoShapeScriptDocValuesIT.java @@ -0,0 +1,126 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +package org.elasticsearch.xpack.spatial.search; + +import org.elasticsearch.action.search.SearchResponse; +import org.elasticsearch.common.document.DocumentField; +import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.common.xcontent.XContentFactory; +import org.elasticsearch.geo.GeometryTestUtils; +import org.elasticsearch.geometry.Geometry; +import org.elasticsearch.geometry.utils.WellKnownText; +import org.elasticsearch.index.fielddata.ScriptDocValues; +import org.elasticsearch.index.mapper.GeoShapeIndexer; +import org.elasticsearch.plugins.Plugin; +import org.elasticsearch.script.MockScriptPlugin; +import org.elasticsearch.script.Script; +import org.elasticsearch.script.ScriptType; +import org.elasticsearch.test.ESSingleNodeTestCase; +import org.elasticsearch.xpack.core.LocalStateCompositeXPackPlugin; +import org.elasticsearch.xpack.spatial.LocalStateSpatialPlugin; +import org.elasticsearch.xpack.spatial.index.fielddata.GeoShapeValues; +import org.elasticsearch.xpack.spatial.util.GeoTestUtils; +import org.junit.Before; + +import java.io.IOException; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; +import java.util.function.Function; + +import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder; +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchResponse; +import static org.hamcrest.Matchers.equalTo; + +public class GeoShapeScriptDocValuesIT extends ESSingleNodeTestCase { + + @Override + protected Collection> getPlugins() { + return Arrays.asList(LocalStateSpatialPlugin.class, LocalStateCompositeXPackPlugin.class, CustomScriptPlugin.class); + } + + public static class CustomScriptPlugin extends MockScriptPlugin { + + @Override + protected Map, Object>> pluginScripts() { + Map, Object>> scripts = new HashMap<>(); + + scripts.put("lat", vars -> script(vars, ScriptDocValues.Geometry::getCentroidLat)); + scripts.put("lon", vars -> script(vars, ScriptDocValues.Geometry::getCentroidLon)); + scripts.put("height", vars -> script(vars, ScriptDocValues.Geometry::height)); + scripts.put("width", vars -> script(vars, ScriptDocValues.Geometry::width)); + return scripts; + } + + static Double script(Map vars, Function, Double> distance) { + Map doc = (Map) vars.get("doc"); + return distance.apply((ScriptDocValues.Geometry) doc.get("location")); + } + } + + @Override + protected boolean forbidPrivateIndexSettings() { + return false; + } + + @Before + public void setupTestIndex() throws IOException { + XContentBuilder xContentBuilder = XContentFactory.jsonBuilder().startObject().startObject("_doc") + .startObject("properties").startObject("location").field("type", "geo_shape"); + xContentBuilder.endObject().endObject().endObject().endObject(); + assertAcked(client().admin().indices().prepareCreate("test").setMapping(xContentBuilder)); + ensureGreen(); + } + + public void testRandomShape() throws Exception { + GeoShapeIndexer indexer = new GeoShapeIndexer(true, "test"); + Geometry geometry = indexer.prepareForIndexing(randomValueOtherThanMany(g -> { + try { + indexer.prepareForIndexing(g); + return false; + } catch (Exception e) { + return true; + } + }, () -> GeometryTestUtils.randomGeometry(false))); + client().prepareIndex("test").setId("1") + .setSource(jsonBuilder().startObject() + .field("name", "TestPosition") + .field("location", WellKnownText.INSTANCE.toWKT(geometry)) + .endObject()) + .get(); + + client().admin().indices().prepareRefresh("test").get(); + + GeoShapeValues.GeoShapeValue value = GeoTestUtils.geoShapeValue(geometry); + + SearchResponse searchResponse = client().prepareSearch().addStoredField("_source") + .addScriptField("lat", new Script(ScriptType.INLINE, CustomScriptPlugin.NAME, "lat", Collections.emptyMap())) + .addScriptField("lon", new Script(ScriptType.INLINE, CustomScriptPlugin.NAME, "lon", Collections.emptyMap())) + .addScriptField("height", new Script(ScriptType.INLINE, CustomScriptPlugin.NAME, "height", Collections.emptyMap())) + .addScriptField("width", new Script(ScriptType.INLINE, CustomScriptPlugin.NAME, "width", Collections.emptyMap())) + .get(); + assertSearchResponse(searchResponse); + Map fields = searchResponse.getHits().getHits()[0].getFields(); + assertThat(fields.get("lat").getValue(), equalTo(value.lat())); + assertThat(fields.get("lon").getValue(), equalTo(value.lon())); + assertThat(fields.get("height").getValue(), equalTo(value.boundingBox().maxY() - value.boundingBox().minY())); + assertThat(fields.get("width").getValue(), equalTo(value.boundingBox().maxX() - value.boundingBox().minX())); + + } +} diff --git a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/fielddata/GeoShapeValues.java b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/fielddata/GeoShapeValues.java index bdec83de9bd5c..acd13eb92fb21 100644 --- a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/fielddata/GeoShapeValues.java +++ b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/fielddata/GeoShapeValues.java @@ -7,6 +7,7 @@ package org.elasticsearch.xpack.spatial.index.fielddata; +import org.apache.lucene.util.BytesRef; import org.elasticsearch.index.mapper.GeoShapeIndexer; import org.elasticsearch.geometry.Geometry; import org.elasticsearch.geometry.Rectangle; @@ -37,7 +38,7 @@ public abstract class GeoShapeValues { public static GeoShapeValues EMPTY = new GeoShapeValues() { - private GeoShapeValuesSourceType DEFAULT_VALUES_SOURCE_TYPE = GeoShapeValuesSourceType.instance(); + private final GeoShapeValuesSourceType DEFAULT_VALUES_SOURCE_TYPE = GeoShapeValuesSourceType.instance(); @Override public boolean advanceExact(int doc) { return false; @@ -82,19 +83,26 @@ protected GeoShapeValues() { * the Geo decoder */ public static class GeoShapeValue { private static final WellKnownText MISSING_GEOMETRY_PARSER = new WellKnownText(true, new GeographyValidator(true)); - + private static final GeoShapeIndexer MISSING_GEOSHAPE_INDEXER = new GeoShapeIndexer(true, "missing"); private final GeometryDocValueReader reader; private final BoundingBox boundingBox; private final Tile2DVisitor tile2DVisitor; - public GeoShapeValue(GeometryDocValueReader reader) { - this.reader = reader; + public GeoShapeValue() { + this.reader = new GeometryDocValueReader(); this.boundingBox = new BoundingBox(); - tile2DVisitor = new Tile2DVisitor(); + this.tile2DVisitor = new Tile2DVisitor(); + } + + /** + * reset the geometry. + */ + public void reset(BytesRef bytesRef) { + this.reader.reset(bytesRef); + this.boundingBox.reset(reader.getExtent(), CoordinateEncoder.GEO); } public BoundingBox boundingBox() { - boundingBox.reset(reader.getExtent(), CoordinateEncoder.GEO); return boundingBox; } @@ -132,13 +140,12 @@ public double lon() { public static GeoShapeValue missing(String missing) { try { - final GeoShapeIndexer indexer = new GeoShapeIndexer(true, "missing"); - final Geometry geometry = indexer.prepareForIndexing(MISSING_GEOMETRY_PARSER.fromWKT(missing)); + final Geometry geometry = MISSING_GEOSHAPE_INDEXER.prepareForIndexing(MISSING_GEOMETRY_PARSER.fromWKT(missing)); final BinaryGeoShapeDocValuesField field = new BinaryGeoShapeDocValuesField("missing"); - field.add(indexer.indexShape(geometry), geometry); - final GeometryDocValueReader reader = new GeometryDocValueReader(); - reader.reset(field.binaryValue()); - return new GeoShapeValue(reader); + field.add(MISSING_GEOSHAPE_INDEXER.indexShape(geometry), geometry); + final GeoShapeValue value = new GeoShapeValue(); + value.reset(field.binaryValue()); + return value; } catch (IOException | ParseException e) { throw new IllegalArgumentException("Can't apply missing value [" + missing + "]", e); } diff --git a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/fielddata/plain/AbstractAtomicGeoShapeShapeFieldData.java b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/fielddata/plain/AbstractAtomicGeoShapeShapeFieldData.java index 8794b13c73211..b308e3f6165f5 100644 --- a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/fielddata/plain/AbstractAtomicGeoShapeShapeFieldData.java +++ b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/fielddata/plain/AbstractAtomicGeoShapeShapeFieldData.java @@ -13,6 +13,7 @@ import org.elasticsearch.xpack.spatial.index.fielddata.GeoShapeValues; import org.elasticsearch.xpack.spatial.index.fielddata.LeafGeoShapeFieldData; +import java.io.IOException; import java.util.Collection; import java.util.Collections; @@ -24,8 +25,8 @@ public final SortedBinaryDocValues getBytesValues() { } @Override - public final ScriptDocValues.BytesRefs getScriptValues() { - throw new UnsupportedOperationException("scripts are not supported by geo_shape doc values"); + public final ScriptDocValues.Geometry getScriptValues() { + return new GeoShapeScriptValues(getGeoShapeValues()); } public static LeafGeoShapeFieldData empty(final int maxDoc) { @@ -51,4 +52,49 @@ public GeoShapeValues getGeoShapeValues() { } }; } + + private static final class GeoShapeScriptValues extends ScriptDocValues.Geometry { + + private final GeoShapeValues in; + private GeoShapeValues.GeoShapeValue value; + + private GeoShapeScriptValues(GeoShapeValues in) { + this.in = in; + } + + @Override + public void setNextDocId(int docId) throws IOException { + value = in.advanceExact(docId) ? in.value() : null; + } + + @Override + public double getCentroidLat() { + return value.lat(); + } + + @Override + public double getCentroidLon() { + return value.lon(); + } + + @Override + public double width() { + return value.boundingBox().maxX() - value.boundingBox().minX(); + } + + @Override + public double height() { + return value.boundingBox().maxY() - value.boundingBox().minY(); + } + + @Override + public GeoShapeValues.GeoShapeValue get(int index) { + return value; + } + + @Override + public int size() { + return 1; + } + } } diff --git a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/fielddata/plain/LatLonShapeDVAtomicShapeFieldData.java b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/fielddata/plain/LatLonShapeDVAtomicShapeFieldData.java index f8da671431e82..b41918ffdcc51 100644 --- a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/fielddata/plain/LatLonShapeDVAtomicShapeFieldData.java +++ b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/fielddata/plain/LatLonShapeDVAtomicShapeFieldData.java @@ -11,10 +11,8 @@ import org.apache.lucene.index.DocValues; import org.apache.lucene.index.LeafReader; import org.apache.lucene.util.Accountable; -import org.apache.lucene.util.BytesRef; import org.elasticsearch.search.aggregations.support.ValuesSourceType; import org.elasticsearch.xpack.spatial.index.fielddata.GeoShapeValues; -import org.elasticsearch.xpack.spatial.index.fielddata.GeometryDocValueReader; import org.elasticsearch.xpack.spatial.search.aggregations.support.GeoShapeValuesSourceType; import java.io.IOException; @@ -50,8 +48,7 @@ public void close() { public GeoShapeValues getGeoShapeValues() { try { final BinaryDocValues binaryValues = DocValues.getBinary(reader, fieldName); - final GeometryDocValueReader reader = new GeometryDocValueReader(); - final GeoShapeValues.GeoShapeValue geoShapeValue = new GeoShapeValues.GeoShapeValue(reader); + final GeoShapeValues.GeoShapeValue geoShapeValue = new GeoShapeValues.GeoShapeValue(); return new GeoShapeValues() { @Override @@ -66,8 +63,7 @@ public ValuesSourceType valuesSourceType() { @Override public GeoShapeValue value() throws IOException { - final BytesRef encoded = binaryValues.binaryValue(); - reader.reset(encoded); + geoShapeValue.reset(binaryValues.binaryValue()); return geoShapeValue; } }; diff --git a/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/util/GeoTestUtils.java b/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/util/GeoTestUtils.java index 3c775b7c66652..f484e9c982ed8 100644 --- a/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/util/GeoTestUtils.java +++ b/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/util/GeoTestUtils.java @@ -55,9 +55,10 @@ public static BinaryGeoShapeDocValuesField binaryGeoShapeDocValuesField(String n return field; } - public static GeoShapeValues.GeoShapeValue geoShapeValue(Geometry geometry) throws IOException { - GeometryDocValueReader reader = geometryDocValueReader(geometry, CoordinateEncoder.GEO); - return new GeoShapeValues.GeoShapeValue(reader); + public static GeoShapeValues.GeoShapeValue geoShapeValue(Geometry geometry) { + GeoShapeValues.GeoShapeValue value = new GeoShapeValues.GeoShapeValue(); + value.reset(binaryGeoShapeDocValuesField("test", geometry).binaryValue()); + return value; } public static GeoBoundingBox randomBBox() { From 7eb560ecf2c5455c73278cdcf15a13b7972445c1 Mon Sep 17 00:00:00 2001 From: iverase Date: Tue, 11 May 2021 12:04:40 +0200 Subject: [PATCH 2/7] Make new interface more generic --- .../transforms/pivot/GeoTileGroupSource.java | 2 +- .../painless/spi/org.elasticsearch.txt | 14 ++- .../test/painless/50_script_doc_values.yml | 23 +++++ .../search/geo/GeoPointScriptDocValuesIT.java | 33 +++++-- .../common/geo/GeoBoundingBox.java | 2 +- .../index/fielddata/ScriptDocValues.java | 89 ++++++------------- .../GeoTileGridValuesSourceBuilder.java | 2 +- .../geogrid/GeoGridAggregationBuilder.java | 2 +- .../metrics/InternalGeoBounds.java | 2 +- .../aggregations/metrics/ParsedGeoBounds.java | 2 +- .../transforms/pivot/GeoTileGroupSource.java | 2 +- .../search/GeoShapeScriptDocValuesIT.java | 33 +++++-- .../AbstractAtomicGeoShapeShapeFieldData.java | 33 +++---- .../test/70_script_doc_values.yml | 48 ++++++++++ 14 files changed, 186 insertions(+), 101 deletions(-) create mode 100644 x-pack/plugin/spatial/src/yamlRestTest/resources/rest-api-spec/test/70_script_doc_values.yml diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/transform/transforms/pivot/GeoTileGroupSource.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/transform/transforms/pivot/GeoTileGroupSource.java index ff4538310e591..0700774cea697 100644 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/transform/transforms/pivot/GeoTileGroupSource.java +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/transform/transforms/pivot/GeoTileGroupSource.java @@ -90,7 +90,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws builder.field(PRECISION.getPreferredName(), precision); } if (geoBoundingBox != null) { - geoBoundingBox.toXContent(builder, params); + builder.field(GeoBoundingBox.BOUNDS_FIELD.getPreferredName(), geoBoundingBox.toXContent(builder, params)); } builder.endObject(); return builder; diff --git a/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/org.elasticsearch.txt b/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/org.elasticsearch.txt index 4763bdf8e4648..8faf59e4cb842 100644 --- a/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/org.elasticsearch.txt +++ b/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/org.elasticsearch.txt @@ -53,6 +53,14 @@ class org.elasticsearch.common.geo.GeoPoint { double getLon() } +class org.elasticsearch.common.geo.GeoBoundingBox { + double top() + double bottom() + double left() + double right() +} + + class org.elasticsearch.index.fielddata.ScriptDocValues$Strings { String get(int) String getValue() @@ -149,10 +157,8 @@ class org.elasticsearch.index.fielddata.ScriptDocValues$Doubles { } class org.elasticsearch.index.fielddata.ScriptDocValues$Geometry { - double getCentroidLat(); - double getCentroidLon(); - double width(); - double height(); + org.elasticsearch.common.geo.GeoPoint getCentroid() + org.elasticsearch.common.geo.GeoBoundingBox getBoundingBox() } class org.elasticsearch.index.fielddata.ScriptDocValues$GeoPoints { diff --git a/modules/lang-painless/src/yamlRestTest/resources/rest-api-spec/test/painless/50_script_doc_values.yml b/modules/lang-painless/src/yamlRestTest/resources/rest-api-spec/test/painless/50_script_doc_values.yml index fa55b47b803dd..d944815ac716a 100644 --- a/modules/lang-painless/src/yamlRestTest/resources/rest-api-spec/test/painless/50_script_doc_values.yml +++ b/modules/lang-painless/src/yamlRestTest/resources/rest-api-spec/test/painless/50_script_doc_values.yml @@ -132,6 +132,29 @@ setup: - match: { hits.hits.0.fields.field.0.lat: 41.1199999647215 } - match: { hits.hits.0.fields.field.0.lon: -71.34000004269183 } + - do: + search: + rest_total_hits_as_int: true + body: + script_fields: + centroid: + script: + source: "doc['geo_point'].getCentroid()" + - match: { hits.hits.0.fields.centroid.0.lat: 41.1199999647215 } + - match: { hits.hits.0.fields.centroid.0.lon: -71.34000004269183 } + + - do: + search: + rest_total_hits_as_int: true + body: + script_fields: + bbox: + script: + source: "doc['geo_point'].getBoundingBox()" + - match: { hits.hits.0.fields.bbox.0.top_left.lat: 41.1199999647215 } + - match: { hits.hits.0.fields.bbox.0.top_left.lon: -71.34000004269183 } + - match: { hits.hits.0.fields.bbox.0.bottom_right.lat: 41.1199999647215 } + - match: { hits.hits.0.fields.bbox.0.bottom_right.lon: -71.34000004269183 } --- "ip": - do: diff --git a/server/src/internalClusterTest/java/org/elasticsearch/search/geo/GeoPointScriptDocValuesIT.java b/server/src/internalClusterTest/java/org/elasticsearch/search/geo/GeoPointScriptDocValuesIT.java index 61bd2a66d1c58..d5a473fdf361a 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/search/geo/GeoPointScriptDocValuesIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/search/geo/GeoPointScriptDocValuesIT.java @@ -18,6 +18,8 @@ import org.apache.lucene.geo.GeoEncodingUtils; import org.elasticsearch.action.search.SearchResponse; import org.elasticsearch.common.document.DocumentField; +import org.elasticsearch.common.geo.GeoBoundingBox; +import org.elasticsearch.common.geo.GeoPoint; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.geo.GeometryTestUtils; @@ -55,16 +57,33 @@ public static class CustomScriptPlugin extends MockScriptPlugin { protected Map, Object>> pluginScripts() { Map, Object>> scripts = new HashMap<>(); - scripts.put("lat", vars -> script(vars, ScriptDocValues.Geometry::getCentroidLat)); - scripts.put("lon", vars -> script(vars, ScriptDocValues.Geometry::getCentroidLon)); - scripts.put("height", vars -> script(vars, ScriptDocValues.Geometry::height)); - scripts.put("width", vars -> script(vars, ScriptDocValues.Geometry::width)); + scripts.put("lat", vars -> scriptLat(vars, ScriptDocValues.Geometry::getCentroid)); + scripts.put("lon", vars -> scriptLon(vars, ScriptDocValues.Geometry::getCentroid)); + scripts.put("height", vars -> scriptHeight(vars, ScriptDocValues.Geometry::getBoundingBox)); + scripts.put("width", vars -> scriptWidth(vars, ScriptDocValues.Geometry::getBoundingBox)); return scripts; } - static Double script(Map vars, Function, Double> distance) { - Map doc = (Map) vars.get("doc"); - return distance.apply((ScriptDocValues.Geometry) doc.get("location")); + static Double scriptHeight(Map vars, Function, GeoBoundingBox> bbox) { + Map doc = (Map) vars.get("doc"); + GeoBoundingBox boundingBox = bbox.apply((ScriptDocValues.Geometry) doc.get("location")); + return boundingBox.top() - boundingBox.bottom(); + } + + static Double scriptWidth(Map vars, Function, GeoBoundingBox> bbox) { + Map doc = (Map) vars.get("doc"); + GeoBoundingBox boundingBox = bbox.apply((ScriptDocValues.Geometry) doc.get("location")); + return boundingBox.right() - boundingBox.left(); + } + + static Double scriptLat(Map vars, Function, GeoPoint> centroid) { + Map doc = (Map) vars.get("doc"); + return centroid.apply((ScriptDocValues.Geometry) doc.get("location")).lat(); + } + + static Double scriptLon(Map vars, Function, GeoPoint> centroid) { + Map doc = (Map) vars.get("doc"); + return centroid.apply((ScriptDocValues.Geometry) doc.get("location")).lon(); } } diff --git a/server/src/main/java/org/elasticsearch/common/geo/GeoBoundingBox.java b/server/src/main/java/org/elasticsearch/common/geo/GeoBoundingBox.java index ec33b36d95b32..aa5b29e70e4f1 100644 --- a/server/src/main/java/org/elasticsearch/common/geo/GeoBoundingBox.java +++ b/server/src/main/java/org/elasticsearch/common/geo/GeoBoundingBox.java @@ -88,7 +88,7 @@ public double right() { @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { - builder.startObject(BOUNDS_FIELD.getPreferredName()); + builder.startObject(); toXContentFragment(builder, true); builder.endObject(); return builder; diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/ScriptDocValues.java b/server/src/main/java/org/elasticsearch/index/fielddata/ScriptDocValues.java index 9561f4db12d66..6a123283770ce 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/ScriptDocValues.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/ScriptDocValues.java @@ -12,6 +12,7 @@ import org.apache.lucene.util.ArrayUtil; import org.apache.lucene.util.BytesRef; import org.apache.lucene.util.BytesRefBuilder; +import org.elasticsearch.common.geo.GeoBoundingBox; import org.elasticsearch.common.geo.GeoPoint; import org.elasticsearch.common.geo.GeoUtils; import org.elasticsearch.common.time.DateUtils; @@ -248,20 +249,19 @@ public int size() { } public abstract static class Geometry extends ScriptDocValues { - /** Centroid latitude */ - public abstract double getCentroidLat(); - /** Centroid longitude */ - public abstract double getCentroidLon(); - /** width of the geometry in degrees */ - public abstract double width(); - /** height of the geometry in degrees */ - public abstract double height(); + /** Returns the centroid of this geometry */ + public abstract GeoPoint getCentroid(); + /** Returns the bounding box of this geometry */ + public abstract GeoBoundingBox getBoundingBox(); + } public static final class GeoPoints extends Geometry { private final MultiGeoPointValues in; private GeoPoint[] values = new GeoPoint[0]; + private final GeoPoint centroid = new GeoPoint(); + private final GeoBoundingBox boundingBox = new GeoBoundingBox(new GeoPoint(), new GeoPoint()); private int count; public GeoPoints(MultiGeoPointValues in) { @@ -272,10 +272,25 @@ public GeoPoints(MultiGeoPointValues in) { public void setNextDocId(int docId) throws IOException { if (in.advanceExact(docId)) { resize(in.docValueCount()); + double centroidLat = 0; + double centroidLon = 0; + double maxLon = Double.NEGATIVE_INFINITY; + double minLon = Double.POSITIVE_INFINITY; + double maxLat = Double.NEGATIVE_INFINITY; + double minLat = Double.POSITIVE_INFINITY; for (int i = 0; i < count; i++) { GeoPoint point = in.nextValue(); - values[i] = new GeoPoint(point.lat(), point.lon()); + values[i].reset(point.lat(), point.lon()); + centroidLat += point.getLat(); + centroidLon += point.getLon(); + maxLon = Math.max(maxLon, values[i].getLon()); + minLon = Math.min(minLon, values[i].getLon()); + maxLat = Math.max(maxLat, values[i].getLat()); + minLat = Math.min(minLat, values[i].getLat()); } + centroid.reset(centroidLat / count, centroidLon / count); + boundingBox.topLeft().reset(maxLat, minLon); + boundingBox.bottomRight().reset(minLat, maxLon); } else { resize(0); } @@ -291,7 +306,7 @@ protected void resize(int newSize) { int oldLength = values.length; values = ArrayUtil.grow(values, count); for (int i = oldLength; i < values.length; ++i) { - values[i] = new GeoPoint(); + values[i] = new GeoPoint(); } } } @@ -377,59 +392,13 @@ public double geohashDistanceWithDefault(String geohash, double defaultValue) { } @Override - public double getCentroidLat() { - if (count == 1) { - return getLat(); - } else { - double centroidLat = 0; - for (int i = 0; i < count; i++) { - centroidLat += values[i].getLat(); - } - return centroidLat / count; - } - } - - @Override - public double getCentroidLon() { - if (count == 1) { - return getLon(); - } else { - double centroidLat = 0; - for (int i = 0; i < count; i++) { - centroidLat += values[i].getLon(); - } - return centroidLat / count; - } - } - - @Override - public double width() { - if (count == 1) { - return 0; - } else { - double maxLon = Double.NEGATIVE_INFINITY; - double minLon = Double.POSITIVE_INFINITY; - for (int i = 0; i < count; i++) { - maxLon = Math.max(maxLon, values[i].getLon()); - minLon = Math.min(minLon, values[i].getLon()); - } - return maxLon - minLon; - } + public GeoPoint getCentroid() { + return centroid; } @Override - public double height() { - if (count == 1) { - return 0; - } else { - double maxLat = Double.NEGATIVE_INFINITY; - double minLat = Double.POSITIVE_INFINITY; - for (int i = 0; i < count; i++) { - maxLat = Math.max(maxLat, values[i].getLat()); - minLat = Math.min(minLat, values[i].getLat()); - } - return maxLat - minLat; - } + public GeoBoundingBox getBoundingBox() { + return boundingBox; } } 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 9238a2fc400e4..5e714491005b2 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 @@ -154,7 +154,7 @@ protected void innerWriteTo(StreamOutput out) throws IOException { protected void doXContentBody(XContentBuilder builder, Params params) throws IOException { builder.field("precision", precision); if (geoBoundingBox.isUnbounded() == false) { - geoBoundingBox.toXContent(builder, params); + builder.field(GeoBoundingBox.BOUNDS_FIELD.getPreferredName(), geoBoundingBox.toXContent(builder, params)); } } diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoGridAggregationBuilder.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoGridAggregationBuilder.java index aa5e998a4665b..85e1218bf1ef2 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoGridAggregationBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoGridAggregationBuilder.java @@ -201,7 +201,7 @@ protected XContentBuilder doXContentBody(XContentBuilder builder, Params params) builder.field(FIELD_SHARD_SIZE.getPreferredName(), shardSize); } if (geoBoundingBox.isUnbounded() == false) { - geoBoundingBox.toXContent(builder, params); + builder.field(GeoBoundingBox.BOUNDS_FIELD.getPreferredName(), geoBoundingBox.toXContent(builder, params)); } return builder; } diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/InternalGeoBounds.java b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/InternalGeoBounds.java index 7cd61768f9cf9..296da91a390f6 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/InternalGeoBounds.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/InternalGeoBounds.java @@ -161,7 +161,7 @@ public Object getProperty(List path) { public XContentBuilder doXContentBody(XContentBuilder builder, Params params) throws IOException { GeoBoundingBox bbox = resolveGeoBoundingBox(); if (bbox != null) { - bbox.toXContent(builder, params); + builder.field(GeoBoundingBox.BOUNDS_FIELD.getPreferredName(), bbox.toXContent(builder, params)); } return builder; } diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/ParsedGeoBounds.java b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/ParsedGeoBounds.java index 5755c210b87ce..6504be2385629 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/ParsedGeoBounds.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/ParsedGeoBounds.java @@ -41,7 +41,7 @@ public String getType() { @Override public XContentBuilder doXContentBody(XContentBuilder builder, Params params) throws IOException { if (geoBoundingBox != null) { - geoBoundingBox.toXContent(builder, params); + builder.field(GeoBoundingBox.BOUNDS_FIELD.getPreferredName(), geoBoundingBox.toXContent(builder, params)); } return builder; } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/transform/transforms/pivot/GeoTileGroupSource.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/transform/transforms/pivot/GeoTileGroupSource.java index d39ada712d9c6..e22f43a74c69c 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/transform/transforms/pivot/GeoTileGroupSource.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/transform/transforms/pivot/GeoTileGroupSource.java @@ -105,7 +105,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws builder.field(PRECISION.getPreferredName(), precision); } if (geoBoundingBox != null) { - geoBoundingBox.toXContent(builder, params); + builder.field(GeoBoundingBox.BOUNDS_FIELD.getPreferredName(), geoBoundingBox.toXContent(builder, params)); } builder.endObject(); return builder; diff --git a/x-pack/plugin/spatial/src/internalClusterTest/java/org/elasticsearch/xpack/spatial/search/GeoShapeScriptDocValuesIT.java b/x-pack/plugin/spatial/src/internalClusterTest/java/org/elasticsearch/xpack/spatial/search/GeoShapeScriptDocValuesIT.java index 82e05c325ff11..05b73be3d7ce7 100644 --- a/x-pack/plugin/spatial/src/internalClusterTest/java/org/elasticsearch/xpack/spatial/search/GeoShapeScriptDocValuesIT.java +++ b/x-pack/plugin/spatial/src/internalClusterTest/java/org/elasticsearch/xpack/spatial/search/GeoShapeScriptDocValuesIT.java @@ -17,6 +17,8 @@ import org.elasticsearch.action.search.SearchResponse; import org.elasticsearch.common.document.DocumentField; +import org.elasticsearch.common.geo.GeoBoundingBox; +import org.elasticsearch.common.geo.GeoPoint; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.geo.GeometryTestUtils; @@ -61,16 +63,33 @@ public static class CustomScriptPlugin extends MockScriptPlugin { protected Map, Object>> pluginScripts() { Map, Object>> scripts = new HashMap<>(); - scripts.put("lat", vars -> script(vars, ScriptDocValues.Geometry::getCentroidLat)); - scripts.put("lon", vars -> script(vars, ScriptDocValues.Geometry::getCentroidLon)); - scripts.put("height", vars -> script(vars, ScriptDocValues.Geometry::height)); - scripts.put("width", vars -> script(vars, ScriptDocValues.Geometry::width)); + scripts.put("lat", vars -> scriptLat(vars, ScriptDocValues.Geometry::getCentroid)); + scripts.put("lon", vars -> scriptLon(vars, ScriptDocValues.Geometry::getCentroid)); + scripts.put("height", vars -> scriptHeight(vars, ScriptDocValues.Geometry::getBoundingBox)); + scripts.put("width", vars -> scriptWidth(vars, ScriptDocValues.Geometry::getBoundingBox)); return scripts; } - static Double script(Map vars, Function, Double> distance) { - Map doc = (Map) vars.get("doc"); - return distance.apply((ScriptDocValues.Geometry) doc.get("location")); + static Double scriptHeight(Map vars, Function, GeoBoundingBox> bbox) { + Map doc = (Map) vars.get("doc"); + GeoBoundingBox boundingBox = bbox.apply((ScriptDocValues.Geometry) doc.get("location")); + return boundingBox.top() - boundingBox.bottom(); + } + + static Double scriptWidth(Map vars, Function, GeoBoundingBox> bbox) { + Map doc = (Map) vars.get("doc"); + GeoBoundingBox boundingBox = bbox.apply((ScriptDocValues.Geometry) doc.get("location")); + return boundingBox.right() - boundingBox.left(); + } + + static Double scriptLat(Map vars, Function, GeoPoint> centroid) { + Map doc = (Map) vars.get("doc"); + return centroid.apply((ScriptDocValues.Geometry) doc.get("location")).lat(); + } + + static Double scriptLon(Map vars, Function, GeoPoint> centroid) { + Map doc = (Map) vars.get("doc"); + return centroid.apply((ScriptDocValues.Geometry) doc.get("location")).lon(); } } diff --git a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/fielddata/plain/AbstractAtomicGeoShapeShapeFieldData.java b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/fielddata/plain/AbstractAtomicGeoShapeShapeFieldData.java index b308e3f6165f5..c9fa1c9841975 100644 --- a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/fielddata/plain/AbstractAtomicGeoShapeShapeFieldData.java +++ b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/fielddata/plain/AbstractAtomicGeoShapeShapeFieldData.java @@ -8,6 +8,8 @@ package org.elasticsearch.xpack.spatial.index.fielddata.plain; import org.apache.lucene.util.Accountable; +import org.elasticsearch.common.geo.GeoBoundingBox; +import org.elasticsearch.common.geo.GeoPoint; import org.elasticsearch.index.fielddata.ScriptDocValues; import org.elasticsearch.index.fielddata.SortedBinaryDocValues; import org.elasticsearch.xpack.spatial.index.fielddata.GeoShapeValues; @@ -56,6 +58,8 @@ public GeoShapeValues getGeoShapeValues() { private static final class GeoShapeScriptValues extends ScriptDocValues.Geometry { private final GeoShapeValues in; + private final GeoPoint centroid = new GeoPoint(); + private final GeoBoundingBox boundingBox = new GeoBoundingBox(new GeoPoint(), new GeoPoint()); private GeoShapeValues.GeoShapeValue value; private GeoShapeScriptValues(GeoShapeValues in) { @@ -64,27 +68,24 @@ private GeoShapeScriptValues(GeoShapeValues in) { @Override public void setNextDocId(int docId) throws IOException { - value = in.advanceExact(docId) ? in.value() : null; - } - - @Override - public double getCentroidLat() { - return value.lat(); - } - - @Override - public double getCentroidLon() { - return value.lon(); + if (in.advanceExact(docId)) { + value = in.value(); + centroid.reset(value.lat(), value.lon()); + boundingBox.topLeft().reset(value.boundingBox().maxY(), value.boundingBox().minX()); + boundingBox.bottomRight().reset(value.boundingBox().minY(), value.boundingBox().maxX()); + } else { + value = null; + } } @Override - public double width() { - return value.boundingBox().maxX() - value.boundingBox().minX(); + public GeoPoint getCentroid() { + return centroid; } @Override - public double height() { - return value.boundingBox().maxY() - value.boundingBox().minY(); + public GeoBoundingBox getBoundingBox() { + return boundingBox; } @Override @@ -94,7 +95,7 @@ public GeoShapeValues.GeoShapeValue get(int index) { @Override public int size() { - return 1; + return value == null ? 0 : 1; } } } diff --git a/x-pack/plugin/spatial/src/yamlRestTest/resources/rest-api-spec/test/70_script_doc_values.yml b/x-pack/plugin/spatial/src/yamlRestTest/resources/rest-api-spec/test/70_script_doc_values.yml new file mode 100644 index 0000000000000..67c4c11d5d787 --- /dev/null +++ b/x-pack/plugin/spatial/src/yamlRestTest/resources/rest-api-spec/test/70_script_doc_values.yml @@ -0,0 +1,48 @@ +setup: + - do: + indices.create: + index: test + body: + settings: + number_of_shards: 1 + mappings: + properties: + geo_shape: + type: geo_shape + + - do: + index: + index: test + id: 1 + body: + geo_shape: "POLYGON((24.04725 59.942,24.04825 59.94125,24.04875 59.94125,24.04875 59.94175,24.048 59.9425,24.0475 59.94275,24.0465 59.94225,24.046 59.94225,24.04575 59.9425,24.04525 59.94225,24.04725 59.942))" + - do: + indices.refresh: {} + +--- +"centroid": + - do: + search: + rest_total_hits_as_int: true + body: + script_fields: + centroid: + script: + source: "doc['geo_shape'].getCentroid()" + - match: { hits.hits.0.fields.centroid.0.lat: 59.942043484188616 } + - match: { hits.hits.0.fields.centroid.0.lon: 24.047588920220733 } + +--- +"bounding box": + - do: + search: + rest_total_hits_as_int: true + body: + script_fields: + bbox: + script: + source: "doc['geo_shape'].getBoundingBox()" + - match: { hits.hits.0.fields.bbox.0.top_left.lat: 59.942749994806945 } + - match: { hits.hits.0.fields.bbox.0.top_left.lon: 24.045249950140715 } + - match: { hits.hits.0.fields.bbox.0.bottom_right.lat: 59.94124996941537 } + - match: { hits.hits.0.fields.bbox.0.bottom_right.lon: 24.048749981448054 } From 4febdc3aaa6afd047137edb8481e9ec2b1e5147e Mon Sep 17 00:00:00 2001 From: iverase Date: Tue, 11 May 2021 13:08:44 +0200 Subject: [PATCH 3/7] Make GeoBoundingBox a ToXContentFragment --- .../client/transform/transforms/pivot/GeoTileGroupSource.java | 4 +++- .../java/org/elasticsearch/common/geo/GeoBoundingBox.java | 4 ++-- .../bucket/composite/GeoTileGridValuesSourceBuilder.java | 4 +++- .../bucket/geogrid/GeoGridAggregationBuilder.java | 4 +++- .../search/aggregations/metrics/InternalGeoBounds.java | 4 +++- .../search/aggregations/metrics/ParsedGeoBounds.java | 4 +++- .../core/transform/transforms/pivot/GeoTileGroupSource.java | 4 +++- 7 files changed, 20 insertions(+), 8 deletions(-) diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/transform/transforms/pivot/GeoTileGroupSource.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/transform/transforms/pivot/GeoTileGroupSource.java index 0700774cea697..477f4e4f048d5 100644 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/transform/transforms/pivot/GeoTileGroupSource.java +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/transform/transforms/pivot/GeoTileGroupSource.java @@ -90,7 +90,9 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws builder.field(PRECISION.getPreferredName(), precision); } if (geoBoundingBox != null) { - builder.field(GeoBoundingBox.BOUNDS_FIELD.getPreferredName(), geoBoundingBox.toXContent(builder, params)); + builder.startObject(GeoBoundingBox.BOUNDS_FIELD.getPreferredName()); + geoBoundingBox.toXContentFragment(builder, true); + builder.endObject(); } builder.endObject(); return builder; diff --git a/server/src/main/java/org/elasticsearch/common/geo/GeoBoundingBox.java b/server/src/main/java/org/elasticsearch/common/geo/GeoBoundingBox.java index aa5b29e70e4f1..0cb4617eea27a 100644 --- a/server/src/main/java/org/elasticsearch/common/geo/GeoBoundingBox.java +++ b/server/src/main/java/org/elasticsearch/common/geo/GeoBoundingBox.java @@ -12,7 +12,7 @@ import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.io.stream.Writeable; -import org.elasticsearch.common.xcontent.ToXContentObject; +import org.elasticsearch.common.xcontent.ToXContentFragment; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.geometry.Geometry; @@ -29,7 +29,7 @@ * A class representing a Geo-Bounding-Box for use by Geo queries and aggregations * that deal with extents/rectangles representing rectangular areas of interest. */ -public class GeoBoundingBox implements ToXContentObject, Writeable { +public class GeoBoundingBox implements ToXContentFragment, Writeable { private static final WellKnownText WKT_PARSER = new WellKnownText(true, new StandardValidator(true)); static final ParseField TOP_RIGHT_FIELD = new ParseField("top_right"); static final ParseField BOTTOM_LEFT_FIELD = new ParseField("bottom_left"); 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 5e714491005b2..190b476469870 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 @@ -154,7 +154,9 @@ protected void innerWriteTo(StreamOutput out) throws IOException { protected void doXContentBody(XContentBuilder builder, Params params) throws IOException { builder.field("precision", precision); if (geoBoundingBox.isUnbounded() == false) { - builder.field(GeoBoundingBox.BOUNDS_FIELD.getPreferredName(), geoBoundingBox.toXContent(builder, params)); + builder.startObject(GeoBoundingBox.BOUNDS_FIELD.getPreferredName()); + geoBoundingBox.toXContentFragment(builder, true); + builder.endObject(); } } diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoGridAggregationBuilder.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoGridAggregationBuilder.java index 85e1218bf1ef2..2ff200fa83f64 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoGridAggregationBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoGridAggregationBuilder.java @@ -201,7 +201,9 @@ protected XContentBuilder doXContentBody(XContentBuilder builder, Params params) builder.field(FIELD_SHARD_SIZE.getPreferredName(), shardSize); } if (geoBoundingBox.isUnbounded() == false) { - builder.field(GeoBoundingBox.BOUNDS_FIELD.getPreferredName(), geoBoundingBox.toXContent(builder, params)); + builder.startObject(GeoBoundingBox.BOUNDS_FIELD.getPreferredName()); + geoBoundingBox.toXContentFragment(builder, true); + builder.endObject(); } return builder; } diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/InternalGeoBounds.java b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/InternalGeoBounds.java index 296da91a390f6..3f96a266a16a6 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/InternalGeoBounds.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/InternalGeoBounds.java @@ -161,7 +161,9 @@ public Object getProperty(List path) { public XContentBuilder doXContentBody(XContentBuilder builder, Params params) throws IOException { GeoBoundingBox bbox = resolveGeoBoundingBox(); if (bbox != null) { - builder.field(GeoBoundingBox.BOUNDS_FIELD.getPreferredName(), bbox.toXContent(builder, params)); + builder.startObject(GeoBoundingBox.BOUNDS_FIELD.getPreferredName()); + bbox.toXContentFragment(builder, true); + builder.endObject(); } return builder; } diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/ParsedGeoBounds.java b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/ParsedGeoBounds.java index 6504be2385629..b89634b8c2b4f 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/ParsedGeoBounds.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/ParsedGeoBounds.java @@ -41,7 +41,9 @@ public String getType() { @Override public XContentBuilder doXContentBody(XContentBuilder builder, Params params) throws IOException { if (geoBoundingBox != null) { - builder.field(GeoBoundingBox.BOUNDS_FIELD.getPreferredName(), geoBoundingBox.toXContent(builder, params)); + builder.startObject(GeoBoundingBox.BOUNDS_FIELD.getPreferredName()); + geoBoundingBox.toXContentFragment(builder, true); + builder.endObject(); } return builder; } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/transform/transforms/pivot/GeoTileGroupSource.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/transform/transforms/pivot/GeoTileGroupSource.java index e22f43a74c69c..3de24af8e878c 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/transform/transforms/pivot/GeoTileGroupSource.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/transform/transforms/pivot/GeoTileGroupSource.java @@ -105,7 +105,9 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws builder.field(PRECISION.getPreferredName(), precision); } if (geoBoundingBox != null) { - builder.field(GeoBoundingBox.BOUNDS_FIELD.getPreferredName(), geoBoundingBox.toXContent(builder, params)); + builder.startObject(GeoBoundingBox.BOUNDS_FIELD.getPreferredName()); + geoBoundingBox.toXContentFragment(builder, true); + builder.endObject(); } builder.endObject(); return builder; From 8be69d036ab5a0036e6fd3f567241c9f9756f77c Mon Sep 17 00:00:00 2001 From: iverase Date: Tue, 11 May 2021 17:53:53 +0200 Subject: [PATCH 4/7] Use geopoints for geoboundingbox --- .../painless/spi/org.elasticsearch.txt | 6 ++---- .../test/painless/50_script_doc_values.yml | 17 +++++++++++++++++ .../search/geo/GeoPointScriptDocValuesIT.java | 4 ++-- .../search/GeoShapeScriptDocValuesIT.java | 4 ++-- .../test/70_script_doc_values.yml | 18 ++++++++++++++++++ 5 files changed, 41 insertions(+), 8 deletions(-) diff --git a/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/org.elasticsearch.txt b/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/org.elasticsearch.txt index 8faf59e4cb842..e3b70de98a67e 100644 --- a/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/org.elasticsearch.txt +++ b/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/org.elasticsearch.txt @@ -54,10 +54,8 @@ class org.elasticsearch.common.geo.GeoPoint { } class org.elasticsearch.common.geo.GeoBoundingBox { - double top() - double bottom() - double left() - double right() + org.elasticsearch.common.geo.GeoPoint topLeft() + org.elasticsearch.common.geo.GeoPoint bottomRight() } diff --git a/modules/lang-painless/src/yamlRestTest/resources/rest-api-spec/test/painless/50_script_doc_values.yml b/modules/lang-painless/src/yamlRestTest/resources/rest-api-spec/test/painless/50_script_doc_values.yml index d944815ac716a..292eb2a845856 100644 --- a/modules/lang-painless/src/yamlRestTest/resources/rest-api-spec/test/painless/50_script_doc_values.yml +++ b/modules/lang-painless/src/yamlRestTest/resources/rest-api-spec/test/painless/50_script_doc_values.yml @@ -155,6 +155,23 @@ setup: - match: { hits.hits.0.fields.bbox.0.top_left.lon: -71.34000004269183 } - match: { hits.hits.0.fields.bbox.0.bottom_right.lat: 41.1199999647215 } - match: { hits.hits.0.fields.bbox.0.bottom_right.lon: -71.34000004269183 } + + - do: + search: + rest_total_hits_as_int: true + body: + script_fields: + topLeft: + script: + source: "doc['geo_point'].getBoundingBox().topLeft()" + bottomRight: + script: + source: "doc['geo_point'].getBoundingBox().bottomRight()" + - match: { hits.hits.0.fields.topLeft.0.lat: 41.1199999647215 } + - match: { hits.hits.0.fields.topLeft.0.lon: -71.34000004269183 } + - match: { hits.hits.0.fields.bottomRight.0.lat: 41.1199999647215 } + - match: { hits.hits.0.fields.bottomRight.0.lon: -71.34000004269183 } + --- "ip": - do: diff --git a/server/src/internalClusterTest/java/org/elasticsearch/search/geo/GeoPointScriptDocValuesIT.java b/server/src/internalClusterTest/java/org/elasticsearch/search/geo/GeoPointScriptDocValuesIT.java index d5a473fdf361a..a4e3f72631c1e 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/search/geo/GeoPointScriptDocValuesIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/search/geo/GeoPointScriptDocValuesIT.java @@ -67,13 +67,13 @@ protected Map, Object>> pluginScripts() { static Double scriptHeight(Map vars, Function, GeoBoundingBox> bbox) { Map doc = (Map) vars.get("doc"); GeoBoundingBox boundingBox = bbox.apply((ScriptDocValues.Geometry) doc.get("location")); - return boundingBox.top() - boundingBox.bottom(); + return boundingBox.topLeft().lat() - boundingBox.bottomRight().lat(); } static Double scriptWidth(Map vars, Function, GeoBoundingBox> bbox) { Map doc = (Map) vars.get("doc"); GeoBoundingBox boundingBox = bbox.apply((ScriptDocValues.Geometry) doc.get("location")); - return boundingBox.right() - boundingBox.left(); + return boundingBox.bottomRight().lon() - boundingBox.topLeft().lon(); } static Double scriptLat(Map vars, Function, GeoPoint> centroid) { diff --git a/x-pack/plugin/spatial/src/internalClusterTest/java/org/elasticsearch/xpack/spatial/search/GeoShapeScriptDocValuesIT.java b/x-pack/plugin/spatial/src/internalClusterTest/java/org/elasticsearch/xpack/spatial/search/GeoShapeScriptDocValuesIT.java index 05b73be3d7ce7..32061a673b104 100644 --- a/x-pack/plugin/spatial/src/internalClusterTest/java/org/elasticsearch/xpack/spatial/search/GeoShapeScriptDocValuesIT.java +++ b/x-pack/plugin/spatial/src/internalClusterTest/java/org/elasticsearch/xpack/spatial/search/GeoShapeScriptDocValuesIT.java @@ -73,13 +73,13 @@ protected Map, Object>> pluginScripts() { static Double scriptHeight(Map vars, Function, GeoBoundingBox> bbox) { Map doc = (Map) vars.get("doc"); GeoBoundingBox boundingBox = bbox.apply((ScriptDocValues.Geometry) doc.get("location")); - return boundingBox.top() - boundingBox.bottom(); + return boundingBox.topLeft().lat() - boundingBox.bottomRight().lat(); } static Double scriptWidth(Map vars, Function, GeoBoundingBox> bbox) { Map doc = (Map) vars.get("doc"); GeoBoundingBox boundingBox = bbox.apply((ScriptDocValues.Geometry) doc.get("location")); - return boundingBox.right() - boundingBox.left(); + return boundingBox.bottomRight().lon() - boundingBox.topLeft().lon(); } static Double scriptLat(Map vars, Function, GeoPoint> centroid) { diff --git a/x-pack/plugin/spatial/src/yamlRestTest/resources/rest-api-spec/test/70_script_doc_values.yml b/x-pack/plugin/spatial/src/yamlRestTest/resources/rest-api-spec/test/70_script_doc_values.yml index 67c4c11d5d787..9cd5297cd5452 100644 --- a/x-pack/plugin/spatial/src/yamlRestTest/resources/rest-api-spec/test/70_script_doc_values.yml +++ b/x-pack/plugin/spatial/src/yamlRestTest/resources/rest-api-spec/test/70_script_doc_values.yml @@ -46,3 +46,21 @@ setup: - match: { hits.hits.0.fields.bbox.0.top_left.lon: 24.045249950140715 } - match: { hits.hits.0.fields.bbox.0.bottom_right.lat: 59.94124996941537 } - match: { hits.hits.0.fields.bbox.0.bottom_right.lon: 24.048749981448054 } + +--- +"bounding box points": + - do: + search: + rest_total_hits_as_int: true + body: + script_fields: + topLeft: + script: + source: "doc['geo_shape'].getBoundingBox().topLeft()" + bottomRight: + script: + source: "doc['geo_shape'].getBoundingBox().bottomRight()" + - match: { hits.hits.0.fields.topLeft.0.lat: 59.942749994806945 } + - match: { hits.hits.0.fields.topLeft.0.lon: 24.045249950140715 } + - match: { hits.hits.0.fields.bottomRight.0.lat: 59.94124996941537 } + - match: { hits.hits.0.fields.bottomRight.0.lon: 24.048749981448054 } From 31d6b9b74bd881527c377f8417bf2ac3c5d2fa85 Mon Sep 17 00:00:00 2001 From: iverase Date: Tue, 11 May 2021 18:18:01 +0200 Subject: [PATCH 5/7] Add dimensional type --- .../painless/spi/org.elasticsearch.txt | 1 + .../test/painless/50_script_doc_values.yml | 10 ++++++++++ .../index/fielddata/ScriptDocValues.java | 12 +++++++++--- .../plain/AbstractAtomicGeoShapeShapeFieldData.java | 5 +++++ .../rest-api-spec/test/70_script_doc_values.yml | 13 +++++++++++++ 5 files changed, 38 insertions(+), 3 deletions(-) diff --git a/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/org.elasticsearch.txt b/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/org.elasticsearch.txt index e3b70de98a67e..1adda3bcef102 100644 --- a/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/org.elasticsearch.txt +++ b/modules/lang-painless/src/main/resources/org/elasticsearch/painless/spi/org.elasticsearch.txt @@ -155,6 +155,7 @@ class org.elasticsearch.index.fielddata.ScriptDocValues$Doubles { } class org.elasticsearch.index.fielddata.ScriptDocValues$Geometry { + int getDimensionalType() org.elasticsearch.common.geo.GeoPoint getCentroid() org.elasticsearch.common.geo.GeoBoundingBox getBoundingBox() } diff --git a/modules/lang-painless/src/yamlRestTest/resources/rest-api-spec/test/painless/50_script_doc_values.yml b/modules/lang-painless/src/yamlRestTest/resources/rest-api-spec/test/painless/50_script_doc_values.yml index 292eb2a845856..7c7a7390107ee 100644 --- a/modules/lang-painless/src/yamlRestTest/resources/rest-api-spec/test/painless/50_script_doc_values.yml +++ b/modules/lang-painless/src/yamlRestTest/resources/rest-api-spec/test/painless/50_script_doc_values.yml @@ -172,6 +172,16 @@ setup: - match: { hits.hits.0.fields.bottomRight.0.lat: 41.1199999647215 } - match: { hits.hits.0.fields.bottomRight.0.lon: -71.34000004269183 } + - do: + search: + rest_total_hits_as_int: true + body: + script_fields: + type: + script: + source: "doc['geo_point'].getDimensionalType()" + - match: { hits.hits.0.fields.type.0: 0 } + --- "ip": - do: diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/ScriptDocValues.java b/server/src/main/java/org/elasticsearch/index/fielddata/ScriptDocValues.java index 6a123283770ce..d6b71381eba87 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/ScriptDocValues.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/ScriptDocValues.java @@ -249,11 +249,12 @@ public int size() { } public abstract static class Geometry extends ScriptDocValues { - /** Returns the centroid of this geometry */ - public abstract GeoPoint getCentroid(); + /** Returns the dimensional type of this geometry */ + public abstract int getDimensionalType(); /** Returns the bounding box of this geometry */ public abstract GeoBoundingBox getBoundingBox(); - + /** Returns the centroid of this geometry */ + public abstract GeoPoint getCentroid(); } public static final class GeoPoints extends Geometry { @@ -391,6 +392,11 @@ public double geohashDistanceWithDefault(String geohash, double defaultValue) { return geohashDistance(geohash); } + @Override + public int getDimensionalType() { + return 0; + } + @Override public GeoPoint getCentroid() { return centroid; diff --git a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/fielddata/plain/AbstractAtomicGeoShapeShapeFieldData.java b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/fielddata/plain/AbstractAtomicGeoShapeShapeFieldData.java index c9fa1c9841975..9e846c9e4478b 100644 --- a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/fielddata/plain/AbstractAtomicGeoShapeShapeFieldData.java +++ b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/fielddata/plain/AbstractAtomicGeoShapeShapeFieldData.java @@ -78,6 +78,11 @@ public void setNextDocId(int docId) throws IOException { } } + @Override + public int getDimensionalType() { + return value.dimensionalShapeType().ordinal(); + } + @Override public GeoPoint getCentroid() { return centroid; diff --git a/x-pack/plugin/spatial/src/yamlRestTest/resources/rest-api-spec/test/70_script_doc_values.yml b/x-pack/plugin/spatial/src/yamlRestTest/resources/rest-api-spec/test/70_script_doc_values.yml index 9cd5297cd5452..91e97d6620cf7 100644 --- a/x-pack/plugin/spatial/src/yamlRestTest/resources/rest-api-spec/test/70_script_doc_values.yml +++ b/x-pack/plugin/spatial/src/yamlRestTest/resources/rest-api-spec/test/70_script_doc_values.yml @@ -64,3 +64,16 @@ setup: - match: { hits.hits.0.fields.topLeft.0.lon: 24.045249950140715 } - match: { hits.hits.0.fields.bottomRight.0.lat: 59.94124996941537 } - match: { hits.hits.0.fields.bottomRight.0.lon: 24.048749981448054 } + +--- +"dimensional type": + - do: + search: + rest_total_hits_as_int: true + body: + script_fields: + type: + script: + source: "doc['geo_shape'].getDimensionalType()" + + - match: { hits.hits.0.fields.type.0: 2 } From b994a045a7c7853f55e2bac77bd6be21dd360c27 Mon Sep 17 00:00:00 2001 From: iverase Date: Wed, 12 May 2021 09:17:12 +0200 Subject: [PATCH 6/7] handle null values properly --- .../search/geo/GeoPointScriptDocValuesIT.java | 83 ++++++++++++++----- .../index/fielddata/ScriptDocValues.java | 6 +- .../search/GeoShapeScriptDocValuesIT.java | 83 +++++++++++++++---- .../AbstractAtomicGeoShapeShapeFieldData.java | 6 +- 4 files changed, 138 insertions(+), 40 deletions(-) diff --git a/server/src/internalClusterTest/java/org/elasticsearch/search/geo/GeoPointScriptDocValuesIT.java b/server/src/internalClusterTest/java/org/elasticsearch/search/geo/GeoPointScriptDocValuesIT.java index a4e3f72631c1e..8bff25ae2a0ff 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/search/geo/GeoPointScriptDocValuesIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/search/geo/GeoPointScriptDocValuesIT.java @@ -19,7 +19,6 @@ import org.elasticsearch.action.search.SearchResponse; import org.elasticsearch.common.document.DocumentField; import org.elasticsearch.common.geo.GeoBoundingBox; -import org.elasticsearch.common.geo.GeoPoint; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.geo.GeometryTestUtils; @@ -29,6 +28,7 @@ import org.elasticsearch.script.Script; import org.elasticsearch.script.ScriptType; import org.elasticsearch.test.ESSingleNodeTestCase; +import org.hamcrest.Matchers; import org.junit.Before; import java.io.IOException; @@ -57,33 +57,59 @@ public static class CustomScriptPlugin extends MockScriptPlugin { protected Map, Object>> pluginScripts() { Map, Object>> scripts = new HashMap<>(); - scripts.put("lat", vars -> scriptLat(vars, ScriptDocValues.Geometry::getCentroid)); - scripts.put("lon", vars -> scriptLon(vars, ScriptDocValues.Geometry::getCentroid)); - scripts.put("height", vars -> scriptHeight(vars, ScriptDocValues.Geometry::getBoundingBox)); - scripts.put("width", vars -> scriptWidth(vars, ScriptDocValues.Geometry::getBoundingBox)); + scripts.put("lat", this::scriptLat); + scripts.put("lon", this::scriptLon); + scripts.put("height", this::scriptHeight); + scripts.put("width", this::scriptWidth); return scripts; } - static Double scriptHeight(Map vars, Function, GeoBoundingBox> bbox) { + private double scriptHeight(Map vars) { Map doc = (Map) vars.get("doc"); - GeoBoundingBox boundingBox = bbox.apply((ScriptDocValues.Geometry) doc.get("location")); - return boundingBox.topLeft().lat() - boundingBox.bottomRight().lat(); + ScriptDocValues.Geometry geometry = assertGeometry(doc); + if (geometry.size() == 0) { + return Double.NaN; + } else { + GeoBoundingBox boundingBox = geometry.getBoundingBox(); + return boundingBox.topLeft().lat() - boundingBox.bottomRight().lat(); + } } - static Double scriptWidth(Map vars, Function, GeoBoundingBox> bbox) { + private double scriptWidth(Map vars) { Map doc = (Map) vars.get("doc"); - GeoBoundingBox boundingBox = bbox.apply((ScriptDocValues.Geometry) doc.get("location")); - return boundingBox.bottomRight().lon() - boundingBox.topLeft().lon(); + ScriptDocValues.Geometry geometry = assertGeometry(doc); + if (geometry.size() == 0) { + return Double.NaN; + } else { + GeoBoundingBox boundingBox = geometry.getBoundingBox(); + return boundingBox.bottomRight().lon() - boundingBox.topLeft().lon(); + } } - static Double scriptLat(Map vars, Function, GeoPoint> centroid) { + private double scriptLat(Map vars) { Map doc = (Map) vars.get("doc"); - return centroid.apply((ScriptDocValues.Geometry) doc.get("location")).lat(); + ScriptDocValues.Geometry geometry = assertGeometry(doc); + return geometry.size() == 0 ? Double.NaN : geometry.getCentroid().lat(); } - static Double scriptLon(Map vars, Function, GeoPoint> centroid) { + private double scriptLon(Map vars) { Map doc = (Map) vars.get("doc"); - return centroid.apply((ScriptDocValues.Geometry) doc.get("location")).lon(); + ScriptDocValues.Geometry geometry = assertGeometry(doc); + return geometry.size() == 0 ? Double.NaN : geometry.getCentroid().lon(); + } + + private ScriptDocValues.Geometry assertGeometry(Map doc) { + ScriptDocValues.Geometry geometry = (ScriptDocValues.Geometry) doc.get("location"); + if (geometry.size() == 0) { + assertThat(geometry.getBoundingBox(), Matchers.nullValue()); + assertThat(geometry.getCentroid(), Matchers.nullValue()); + assertThat(geometry.getDimensionalType(), equalTo(-1)); + } else { + assertThat(geometry.getBoundingBox(), Matchers.notNullValue()); + assertThat(geometry.getCentroid(), Matchers.notNullValue()); + assertThat(geometry.getDimensionalType(), equalTo(0)); + } + return geometry; } } @@ -129,9 +155,6 @@ public void testRandomPoint() throws Exception { assertThat(fields.get("lon").getValue(), equalTo(qLon)); assertThat(fields.get("height").getValue(), equalTo(0d)); assertThat(fields.get("width").getValue(), equalTo(0d)); - - client().prepareDelete("test", "1").get(); - client().admin().indices().prepareRefresh("test").get(); } public void testRandomMultiPoint() throws Exception { @@ -178,8 +201,30 @@ public void testRandomMultiPoint() throws Exception { assertThat(fields.get("lon").getValue(), equalTo(centroidLon)); assertThat(fields.get("height").getValue(), equalTo(height)); assertThat(fields.get("width").getValue(), equalTo(width)); + } + + public void testNullPoint() throws Exception { + client().prepareIndex("test").setId("1") + .setSource(jsonBuilder().startObject() + .field("name", "TestPosition") + .nullField("location") + .endObject()) + .get(); - client().prepareDelete("test", "1").get(); client().admin().indices().prepareRefresh("test").get(); + + SearchResponse searchResponse = client().prepareSearch().addStoredField("_source") + .addScriptField("lat", new Script(ScriptType.INLINE, CustomScriptPlugin.NAME, "lat", Collections.emptyMap())) + .addScriptField("lon", new Script(ScriptType.INLINE, CustomScriptPlugin.NAME, "lon", Collections.emptyMap())) + .addScriptField("height", new Script(ScriptType.INLINE, CustomScriptPlugin.NAME, "height", Collections.emptyMap())) + .addScriptField("width", new Script(ScriptType.INLINE, CustomScriptPlugin.NAME, "width", Collections.emptyMap())) + .get(); + assertSearchResponse(searchResponse); + + Map fields = searchResponse.getHits().getHits()[0].getFields(); + assertThat(fields.get("lat").getValue(), equalTo(Double.NaN)); + assertThat(fields.get("lon").getValue(), equalTo(Double.NaN)); + assertThat(fields.get("height").getValue(), equalTo(Double.NaN)); + assertThat(fields.get("width").getValue(), equalTo(Double.NaN)); } } diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/ScriptDocValues.java b/server/src/main/java/org/elasticsearch/index/fielddata/ScriptDocValues.java index d6b71381eba87..4413ef8dd1963 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/ScriptDocValues.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/ScriptDocValues.java @@ -394,17 +394,17 @@ public double geohashDistanceWithDefault(String geohash, double defaultValue) { @Override public int getDimensionalType() { - return 0; + return size() == 0 ? -1 : 0; } @Override public GeoPoint getCentroid() { - return centroid; + return size() == 0 ? null : centroid; } @Override public GeoBoundingBox getBoundingBox() { - return boundingBox; + return size() == 0 ? null : boundingBox; } } diff --git a/x-pack/plugin/spatial/src/internalClusterTest/java/org/elasticsearch/xpack/spatial/search/GeoShapeScriptDocValuesIT.java b/x-pack/plugin/spatial/src/internalClusterTest/java/org/elasticsearch/xpack/spatial/search/GeoShapeScriptDocValuesIT.java index 32061a673b104..689b0dc64d16c 100644 --- a/x-pack/plugin/spatial/src/internalClusterTest/java/org/elasticsearch/xpack/spatial/search/GeoShapeScriptDocValuesIT.java +++ b/x-pack/plugin/spatial/src/internalClusterTest/java/org/elasticsearch/xpack/spatial/search/GeoShapeScriptDocValuesIT.java @@ -18,7 +18,6 @@ import org.elasticsearch.action.search.SearchResponse; import org.elasticsearch.common.document.DocumentField; import org.elasticsearch.common.geo.GeoBoundingBox; -import org.elasticsearch.common.geo.GeoPoint; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.geo.GeometryTestUtils; @@ -35,6 +34,7 @@ import org.elasticsearch.xpack.spatial.LocalStateSpatialPlugin; import org.elasticsearch.xpack.spatial.index.fielddata.GeoShapeValues; import org.elasticsearch.xpack.spatial.util.GeoTestUtils; +import org.hamcrest.Matchers; import org.junit.Before; import java.io.IOException; @@ -49,6 +49,8 @@ import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchResponse; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.greaterThanOrEqualTo; +import static org.hamcrest.Matchers.lessThanOrEqualTo; public class GeoShapeScriptDocValuesIT extends ESSingleNodeTestCase { @@ -63,33 +65,61 @@ public static class CustomScriptPlugin extends MockScriptPlugin { protected Map, Object>> pluginScripts() { Map, Object>> scripts = new HashMap<>(); - scripts.put("lat", vars -> scriptLat(vars, ScriptDocValues.Geometry::getCentroid)); - scripts.put("lon", vars -> scriptLon(vars, ScriptDocValues.Geometry::getCentroid)); - scripts.put("height", vars -> scriptHeight(vars, ScriptDocValues.Geometry::getBoundingBox)); - scripts.put("width", vars -> scriptWidth(vars, ScriptDocValues.Geometry::getBoundingBox)); + scripts.put("lat", this::scriptLat); + scripts.put("lon", this::scriptLon); + scripts.put("height", this::scriptHeight); + scripts.put("width", this::scriptWidth); return scripts; } - static Double scriptHeight(Map vars, Function, GeoBoundingBox> bbox) { + + private double scriptHeight(Map vars) { Map doc = (Map) vars.get("doc"); - GeoBoundingBox boundingBox = bbox.apply((ScriptDocValues.Geometry) doc.get("location")); - return boundingBox.topLeft().lat() - boundingBox.bottomRight().lat(); + ScriptDocValues.Geometry geometry = assertGeometry(doc); + if (geometry.size() == 0) { + return Double.NaN; + } else { + GeoBoundingBox boundingBox = geometry.getBoundingBox(); + return boundingBox.topLeft().lat() - boundingBox.bottomRight().lat(); + } } - static Double scriptWidth(Map vars, Function, GeoBoundingBox> bbox) { + private double scriptWidth(Map vars) { Map doc = (Map) vars.get("doc"); - GeoBoundingBox boundingBox = bbox.apply((ScriptDocValues.Geometry) doc.get("location")); - return boundingBox.bottomRight().lon() - boundingBox.topLeft().lon(); + ScriptDocValues.Geometry geometry = assertGeometry(doc); + if (geometry.size() == 0) { + return Double.NaN; + } else { + GeoBoundingBox boundingBox = geometry.getBoundingBox(); + return boundingBox.bottomRight().lon() - boundingBox.topLeft().lon(); + } } - static Double scriptLat(Map vars, Function, GeoPoint> centroid) { + private double scriptLat(Map vars) { Map doc = (Map) vars.get("doc"); - return centroid.apply((ScriptDocValues.Geometry) doc.get("location")).lat(); + ScriptDocValues.Geometry geometry = assertGeometry(doc); + return geometry.size() == 0 ? Double.NaN : geometry.getCentroid().lat(); } - static Double scriptLon(Map vars, Function, GeoPoint> centroid) { + private double scriptLon(Map vars) { Map doc = (Map) vars.get("doc"); - return centroid.apply((ScriptDocValues.Geometry) doc.get("location")).lon(); + ScriptDocValues.Geometry geometry = assertGeometry(doc); + return geometry.size() == 0 ? Double.NaN : geometry.getCentroid().lon(); + } + + private ScriptDocValues.Geometry assertGeometry(Map doc) { + ScriptDocValues.Geometry geometry = (ScriptDocValues.Geometry) doc.get("location"); + if (geometry.size() == 0) { + assertThat(geometry.getBoundingBox(), Matchers.nullValue()); + assertThat(geometry.getCentroid(), Matchers.nullValue()); + assertThat(geometry.getDimensionalType(), equalTo(-1)); + } else { + assertThat(geometry.getBoundingBox(), Matchers.notNullValue()); + assertThat(geometry.getCentroid(), Matchers.notNullValue()); + assertThat(geometry.getDimensionalType(), greaterThanOrEqualTo(0)); + assertThat(geometry.getDimensionalType(), lessThanOrEqualTo(2)); + } + return geometry; } } @@ -140,6 +170,29 @@ public void testRandomShape() throws Exception { assertThat(fields.get("lon").getValue(), equalTo(value.lon())); assertThat(fields.get("height").getValue(), equalTo(value.boundingBox().maxY() - value.boundingBox().minY())); assertThat(fields.get("width").getValue(), equalTo(value.boundingBox().maxX() - value.boundingBox().minX())); + } + public void testNullShape() throws Exception { + client().prepareIndex("test").setId("1") + .setSource(jsonBuilder().startObject() + .field("name", "TestPosition") + .nullField("location") + .endObject()) + .get(); + + client().admin().indices().prepareRefresh("test").get(); + + SearchResponse searchResponse = client().prepareSearch().addStoredField("_source") + .addScriptField("lat", new Script(ScriptType.INLINE, CustomScriptPlugin.NAME, "lat", Collections.emptyMap())) + .addScriptField("lon", new Script(ScriptType.INLINE, CustomScriptPlugin.NAME, "lon", Collections.emptyMap())) + .addScriptField("height", new Script(ScriptType.INLINE, CustomScriptPlugin.NAME, "height", Collections.emptyMap())) + .addScriptField("width", new Script(ScriptType.INLINE, CustomScriptPlugin.NAME, "width", Collections.emptyMap())) + .get(); + assertSearchResponse(searchResponse); + Map fields = searchResponse.getHits().getHits()[0].getFields(); + assertThat(fields.get("lat").getValue(), equalTo(Double.NaN)); + assertThat(fields.get("lon").getValue(), equalTo(Double.NaN)); + assertThat(fields.get("height").getValue(), equalTo(Double.NaN)); + assertThat(fields.get("width").getValue(), equalTo(Double.NaN)); } } diff --git a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/fielddata/plain/AbstractAtomicGeoShapeShapeFieldData.java b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/fielddata/plain/AbstractAtomicGeoShapeShapeFieldData.java index 9e846c9e4478b..59ed40ad9041f 100644 --- a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/fielddata/plain/AbstractAtomicGeoShapeShapeFieldData.java +++ b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/fielddata/plain/AbstractAtomicGeoShapeShapeFieldData.java @@ -80,17 +80,17 @@ public void setNextDocId(int docId) throws IOException { @Override public int getDimensionalType() { - return value.dimensionalShapeType().ordinal(); + return value == null ? -1 : value.dimensionalShapeType().ordinal(); } @Override public GeoPoint getCentroid() { - return centroid; + return value == null ? null : centroid; } @Override public GeoBoundingBox getBoundingBox() { - return boundingBox; + return value == null ? null : boundingBox; } @Override From 35154e75f856b635ba67cdd0ce2ee0f75d5530e7 Mon Sep 17 00:00:00 2001 From: iverase Date: Wed, 12 May 2021 09:29:34 +0200 Subject: [PATCH 7/7] geoshapevalue now implements ToXContentFragment --- .../spatial/index/fielddata/GeoShapeValues.java | 9 ++++++++- .../rest-api-spec/test/70_script_doc_values.yml | 14 ++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/fielddata/GeoShapeValues.java b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/fielddata/GeoShapeValues.java index acd13eb92fb21..5de7f3dc92751 100644 --- a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/fielddata/GeoShapeValues.java +++ b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/fielddata/GeoShapeValues.java @@ -8,6 +8,8 @@ package org.elasticsearch.xpack.spatial.index.fielddata; import org.apache.lucene.util.BytesRef; +import org.elasticsearch.common.xcontent.ToXContentFragment; +import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.index.mapper.GeoShapeIndexer; import org.elasticsearch.geometry.Geometry; import org.elasticsearch.geometry.Rectangle; @@ -81,7 +83,7 @@ protected GeoShapeValues() { /** thin wrapper around a {@link GeometryDocValueReader} which encodes / decodes values using * the Geo decoder */ - public static class GeoShapeValue { + public static class GeoShapeValue implements ToXContentFragment { private static final WellKnownText MISSING_GEOMETRY_PARSER = new WellKnownText(true, new GeographyValidator(true)); private static final GeoShapeIndexer MISSING_GEOSHAPE_INDEXER = new GeoShapeIndexer(true, "missing"); private final GeometryDocValueReader reader; @@ -150,6 +152,11 @@ public static GeoShapeValue missing(String missing) { throw new IllegalArgumentException("Can't apply missing value [" + missing + "]", e); } } + + @Override + public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { + throw new IllegalArgumentException("cannot write xcontent for geo_shape doc value"); + } } public static class BoundingBox { diff --git a/x-pack/plugin/spatial/src/yamlRestTest/resources/rest-api-spec/test/70_script_doc_values.yml b/x-pack/plugin/spatial/src/yamlRestTest/resources/rest-api-spec/test/70_script_doc_values.yml index 91e97d6620cf7..ecde387a7a4cf 100644 --- a/x-pack/plugin/spatial/src/yamlRestTest/resources/rest-api-spec/test/70_script_doc_values.yml +++ b/x-pack/plugin/spatial/src/yamlRestTest/resources/rest-api-spec/test/70_script_doc_values.yml @@ -77,3 +77,17 @@ setup: source: "doc['geo_shape'].getDimensionalType()" - match: { hits.hits.0.fields.type.0: 2 } + +--- +"geoshape value": + - do: + catch: /illegal_argument_exception/ + search: + rest_total_hits_as_int: true + body: + script_fields: + type: + script: + source: "doc['geo_shape'].get(0)" + + - match: { error.root_cause.0.reason: "cannot write xcontent for geo_shape doc value" }