Browse Source

Add a geo point field for the scripting fields api (#81395)

This adds a geo point(s) field for the scripting fields api. This field only supports get(default), 
get(index, default), and iterator right now. This also adds the ability to create new a GeoPoint through 
the allow list, so users can specify a default that makes sense. It does not include reset as this field 
currently does not wrap the data to be read-only.
Jack Conradson 3 years ago
parent
commit
d59038ba74

+ 5 - 0
docs/changelog/81395.yaml

@@ -0,0 +1,5 @@
+pr: 81395
+summary: Add a geo point field for the scripting fields api
+area: Infra/Scripting
+type: enhancement
+issues: []

+ 5 - 0
modules/lang-painless/src/main/resources/org/elasticsearch/painless/org.elasticsearch.script.fields.txt

@@ -83,6 +83,11 @@ class org.elasticsearch.script.field.KeywordDocValuesField @dynamic_type {
   String get(int, String)
 }
 
+class org.elasticsearch.script.field.GeoPointDocValuesField @dynamic_type {
+  GeoPoint get(GeoPoint)
+  GeoPoint get(int, GeoPoint)
+}
+
 class org.elasticsearch.script.field.IpDocValuesField @dynamic_type {
   IPAddress get(IPAddress)
   IPAddress get(int, IPAddress)

+ 2 - 0
modules/lang-painless/src/main/resources/org/elasticsearch/painless/org.elasticsearch.txt

@@ -49,6 +49,8 @@ class org.elasticsearch.painless.api.Debug {
 #### ES Scripting API
 
 class org.elasticsearch.common.geo.GeoPoint {
+  ()
+  (double, double)
   double getLat()
   double getLon()
 }

+ 73 - 0
modules/lang-painless/src/yamlRestTest/resources/rest-api-spec/test/painless/50_script_doc_values.yml

@@ -82,6 +82,7 @@ setup:
               ip: ["10.1.2.3", "2001:db8::2:1"]
               date: [2017-01-01T12:11:12, 2018-01-01T12:11:12]
               nanos: [2015-01-01T12:10:30.123456789Z, 2015-01-01T12:10:30.987654321Z]
+              geo_point: [[-71.34,41.12],[60.32,21.25]]
               keyword: ["one string", "another string"]
               long: [1152921504606846976, 576460752303423488]
               integer: [5, 17, 29]
@@ -447,6 +448,78 @@ 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:
+            query: { term: { _id: 1 } }
+            script_fields:
+              field:
+                script:
+                  source: "field('geo_point').get(new GeoPoint())"
+    - 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:
+            query: { term: { _id: 1 } }
+            script_fields:
+              field:
+                script:
+                  source: "/* avoid yaml stash */ $('geo_point', new GeoPoint())"
+    - 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:
+            query: { term: { _id: 3 } }
+            script_fields:
+              field:
+                script:
+                  source: "field('geo_point').get(new GeoPoint())"
+    - match: { hits.hits.0.fields.field.0.lat: 21.249999990686774 }
+    - match: { hits.hits.0.fields.field.0.lon: 60.319999968633056 }
+
+    - do:
+        search:
+          rest_total_hits_as_int: true
+          body:
+            query: { term: { _id: 3 } }
+            script_fields:
+              field:
+                script:
+                  source: "/* avoid yaml stash */ $('geo_point', new GeoPoint())"
+    - match: { hits.hits.0.fields.field.0.lat: 21.249999990686774 }
+    - match: { hits.hits.0.fields.field.0.lon: 60.319999968633056 }
+
+    - do:
+        search:
+          rest_total_hits_as_int: true
+          body:
+            query: { term: { _id: 3 } }
+            script_fields:
+              field:
+                script:
+                  source: "field('geo_point').get(1, new GeoPoint())"
+    - 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:
+            query: { term: { _id: 2 } }
+            script_fields:
+              field:
+                script:
+                  source: "/* avoid yaml stash */ $('geo_point', new GeoPoint(1.0, 2.0))"
+    - match: { hits.hits.0.fields.field.0.lat: 1.0 }
+    - match: { hits.hits.0.fields.field.0.lon: 2.0 }
+
     - do:
         search:
             rest_total_hits_as_int: true

+ 4 - 96
server/src/main/java/org/elasticsearch/index/fielddata/ScriptDocValues.java

@@ -287,101 +287,9 @@ public abstract class ScriptDocValues<T> extends AbstractList<T> {
 
     public interface GeometrySupplier<T> extends Supplier<T> {
 
-        GeoPoint getCentroid();
+        GeoPoint getInternalCentroid();
 
-        GeoBoundingBox getBoundingBox();
-    }
-
-    public static class GeoPointsSupplier implements GeometrySupplier<GeoPoint> {
-
-        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 GeoPointsSupplier(MultiGeoPointValues in) {
-            this.in = in;
-        }
-
-        @Override
-        public void setNextDocId(int docId) throws IOException {
-            if (in.advanceExact(docId)) {
-                resize(in.docValueCount());
-                if (count == 1) {
-                    setSingleValue();
-                } else {
-                    setMultiValue();
-                }
-            } else {
-                resize(0);
-            }
-        }
-
-        private void setSingleValue() throws IOException {
-            GeoPoint point = in.nextValue();
-            values[0].reset(point.lat(), point.lon());
-            centroid.reset(point.lat(), point.lon());
-            boundingBox.topLeft().reset(point.lat(), point.lon());
-            boundingBox.bottomRight().reset(point.lat(), point.lon());
-        }
-
-        private void setMultiValue() throws IOException {
-            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].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);
-        }
-
-        /**
-         * Set the {@link #size()} and ensure that the {@link #values} array can
-         * store at least that many entries.
-         */
-        private void resize(int newSize) {
-            count = newSize;
-            if (newSize > values.length) {
-                int oldLength = values.length;
-                values = ArrayUtil.grow(values, count);
-                for (int i = oldLength; i < values.length; ++i) {
-                    values[i] = new GeoPoint();
-                }
-            }
-        }
-
-        @Override
-        public GeoPoint getInternal(int index) {
-            return values[index];
-        }
-
-        @Override
-        public GeoPoint getCentroid() {
-            return centroid;
-        }
-
-        @Override
-        public GeoBoundingBox getBoundingBox() {
-            return boundingBox;
-        }
-
-        @Override
-        public int size() {
-            return count;
-        }
+        GeoBoundingBox getInternalBoundingBox();
     }
 
     public static class GeoPoints extends Geometry<GeoPoint> {
@@ -481,7 +389,7 @@ public abstract class ScriptDocValues<T> extends AbstractList<T> {
 
         @Override
         public GeoPoint getCentroid() {
-            return size() == 0 ? null : geometrySupplier.getCentroid();
+            return size() == 0 ? null : geometrySupplier.getInternalCentroid();
         }
 
         @Override
@@ -496,7 +404,7 @@ public abstract class ScriptDocValues<T> extends AbstractList<T> {
 
         @Override
         public GeoBoundingBox getBoundingBox() {
-            return size() == 0 ? null : geometrySupplier.getBoundingBox();
+            return size() == 0 ? null : geometrySupplier.getInternalBoundingBox();
         }
     }
 

+ 2 - 7
server/src/main/java/org/elasticsearch/index/mapper/GeoPointFieldMapper.java

@@ -31,13 +31,12 @@ import org.elasticsearch.geometry.Geometry;
 import org.elasticsearch.geometry.Point;
 import org.elasticsearch.geometry.ShapeType;
 import org.elasticsearch.index.fielddata.IndexFieldData;
-import org.elasticsearch.index.fielddata.ScriptDocValues;
 import org.elasticsearch.index.fielddata.plain.AbstractLatLonPointIndexFieldData;
 import org.elasticsearch.index.query.SearchExecutionContext;
 import org.elasticsearch.script.GeoPointFieldScript;
 import org.elasticsearch.script.Script;
 import org.elasticsearch.script.ScriptCompiler;
-import org.elasticsearch.script.field.DelegateDocValuesField;
+import org.elasticsearch.script.field.GeoPointDocValuesField;
 import org.elasticsearch.search.aggregations.support.CoreValuesSourceType;
 import org.elasticsearch.search.lookup.FieldValues;
 import org.elasticsearch.search.lookup.SearchLookup;
@@ -306,11 +305,7 @@ public class GeoPointFieldMapper extends AbstractPointGeometryFieldMapper<GeoPoi
         @Override
         public IndexFieldData.Builder fielddataBuilder(String fullyQualifiedIndexName, Supplier<SearchLookup> searchLookup) {
             failIfNoDocValues();
-            return new AbstractLatLonPointIndexFieldData.Builder(
-                name(),
-                CoreValuesSourceType.GEOPOINT,
-                (dv, n) -> new DelegateDocValuesField(new ScriptDocValues.GeoPoints(new ScriptDocValues.GeoPointsSupplier(dv)), n)
-            );
+            return new AbstractLatLonPointIndexFieldData.Builder(name(), CoreValuesSourceType.GEOPOINT, GeoPointDocValuesField::new);
         }
 
         @Override

+ 2 - 7
server/src/main/java/org/elasticsearch/index/mapper/GeoPointScriptFieldType.java

@@ -20,12 +20,11 @@ import org.elasticsearch.common.time.DateMathParser;
 import org.elasticsearch.common.unit.DistanceUnit;
 import org.elasticsearch.geometry.Geometry;
 import org.elasticsearch.index.fielddata.GeoPointScriptFieldData;
-import org.elasticsearch.index.fielddata.ScriptDocValues;
 import org.elasticsearch.index.query.SearchExecutionContext;
 import org.elasticsearch.script.CompositeFieldScript;
 import org.elasticsearch.script.GeoPointFieldScript;
 import org.elasticsearch.script.Script;
-import org.elasticsearch.script.field.DelegateDocValuesField;
+import org.elasticsearch.script.field.GeoPointDocValuesField;
 import org.elasticsearch.search.lookup.SearchLookup;
 import org.elasticsearch.search.runtime.GeoPointScriptFieldDistanceFeatureQuery;
 import org.elasticsearch.search.runtime.GeoPointScriptFieldExistsQuery;
@@ -98,11 +97,7 @@ public final class GeoPointScriptFieldType extends AbstractScriptFieldType<GeoPo
 
     @Override
     public GeoPointScriptFieldData.Builder fielddataBuilder(String fullyQualifiedIndexName, Supplier<SearchLookup> searchLookup) {
-        return new GeoPointScriptFieldData.Builder(
-            name(),
-            leafFactory(searchLookup.get()),
-            (dv, n) -> new DelegateDocValuesField(new ScriptDocValues.GeoPoints(new ScriptDocValues.GeoPointsSupplier(dv)), n)
-        );
+        return new GeoPointScriptFieldData.Builder(name(), leafFactory(searchLookup.get()), GeoPointDocValuesField::new);
     }
 
     @Override

+ 166 - 0
server/src/main/java/org/elasticsearch/script/field/GeoPointDocValuesField.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 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.script.field;
+
+import org.apache.lucene.util.ArrayUtil;
+import org.elasticsearch.common.geo.GeoBoundingBox;
+import org.elasticsearch.common.geo.GeoPoint;
+import org.elasticsearch.index.fielddata.MultiGeoPointValues;
+import org.elasticsearch.index.fielddata.ScriptDocValues;
+
+import java.io.IOException;
+import java.util.Iterator;
+import java.util.NoSuchElementException;
+
+public class GeoPointDocValuesField implements DocValuesField<GeoPoint>, ScriptDocValues.GeometrySupplier<GeoPoint> {
+
+    protected final MultiGeoPointValues input;
+    protected final String name;
+
+    protected GeoPoint[] values = new GeoPoint[0];
+    protected int count;
+
+    // maintain bwc by making centroid and bounding box available to ScriptDocValues.GeoPoints
+    private ScriptDocValues.GeoPoints geoPoints = null;
+    private final GeoPoint centroid = new GeoPoint();
+    private final GeoBoundingBox boundingBox = new GeoBoundingBox(new GeoPoint(), new GeoPoint());
+
+    public GeoPointDocValuesField(MultiGeoPointValues input, String name) {
+        this.input = input;
+        this.name = name;
+    }
+
+    @Override
+    public void setNextDocId(int docId) throws IOException {
+        if (input.advanceExact(docId)) {
+            resize(input.docValueCount());
+            if (count == 1) {
+                setSingleValue();
+            } else {
+                setMultiValue();
+            }
+        } else {
+            resize(0);
+        }
+    }
+
+    private void resize(int newSize) {
+        count = newSize;
+        if (newSize > values.length) {
+            int oldLength = values.length;
+            values = ArrayUtil.grow(values, count);
+            for (int i = oldLength; i < values.length; ++i) {
+                values[i] = new GeoPoint();
+            }
+        }
+    }
+
+    private void setSingleValue() throws IOException {
+        GeoPoint point = input.nextValue();
+        values[0].reset(point.lat(), point.lon());
+        centroid.reset(point.lat(), point.lon());
+        boundingBox.topLeft().reset(point.lat(), point.lon());
+        boundingBox.bottomRight().reset(point.lat(), point.lon());
+    }
+
+    private void setMultiValue() throws IOException {
+        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 = input.nextValue();
+            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);
+    }
+
+    @Override
+    public ScriptDocValues<GeoPoint> getScriptDocValues() {
+        if (geoPoints == null) {
+            geoPoints = new ScriptDocValues.GeoPoints(this);
+        }
+
+        return geoPoints;
+    }
+
+    @Override
+    public GeoPoint getInternal(int index) {
+        return values[index];
+    }
+
+    // maintain bwc by making centroid available to ScriptDocValues.GeoPoints
+    @Override
+    public GeoPoint getInternalCentroid() {
+        return centroid;
+    }
+
+    // maintain bwc by making bounding box available to ScriptDocValues.GeoPoints
+    @Override
+    public GeoBoundingBox getInternalBoundingBox() {
+        return boundingBox;
+    }
+
+    @Override
+    public String getName() {
+        return name;
+    }
+
+    @Override
+    public boolean isEmpty() {
+        return count == 0;
+    }
+
+    @Override
+    public int size() {
+        return count;
+    }
+
+    public GeoPoint get(GeoPoint defaultValue) {
+        return get(0, defaultValue);
+    }
+
+    public GeoPoint get(int index, GeoPoint defaultValue) {
+        if (isEmpty() || index < 0 || index >= count) {
+            return defaultValue;
+        }
+
+        return values[index];
+    }
+
+    @Override
+    public Iterator<GeoPoint> iterator() {
+        return new Iterator<GeoPoint>() {
+            private int index = 0;
+
+            @Override
+            public boolean hasNext() {
+                return index < count;
+            }
+
+            @Override
+            public GeoPoint next() {
+                if (hasNext() == false) {
+                    throw new NoSuchElementException();
+                }
+                return values[index++];
+            }
+        };
+    }
+}

+ 5 - 5
server/src/test/java/org/elasticsearch/index/fielddata/ScriptDocValuesGeoPointsTests.java

@@ -11,7 +11,7 @@ package org.elasticsearch.index.fielddata;
 import org.elasticsearch.common.geo.GeoPoint;
 import org.elasticsearch.common.geo.GeoUtils;
 import org.elasticsearch.index.fielddata.ScriptDocValues.GeoPoints;
-import org.elasticsearch.index.fielddata.ScriptDocValues.GeoPointsSupplier;
+import org.elasticsearch.script.field.GeoPointDocValuesField;
 import org.elasticsearch.test.ESTestCase;
 
 import java.io.IOException;
@@ -63,7 +63,7 @@ public class ScriptDocValuesGeoPointsTests extends ESTestCase {
 
         GeoPoint[][] points = { { new GeoPoint(lat1, lon1), new GeoPoint(lat2, lon2) } };
         final MultiGeoPointValues values = wrap(points);
-        final ScriptDocValues.GeoPoints script = new ScriptDocValues.GeoPoints(new GeoPointsSupplier(values));
+        final ScriptDocValues.GeoPoints script = (GeoPoints) new GeoPointDocValuesField(values, "test").getScriptDocValues();
 
         script.getSupplier().setNextDocId(1);
         assertEquals(true, script.isEmpty());
@@ -81,11 +81,11 @@ public class ScriptDocValuesGeoPointsTests extends ESTestCase {
         final double lon = randomLon();
         GeoPoint[][] points = { { new GeoPoint(lat, lon) } };
         final MultiGeoPointValues values = wrap(points);
-        final ScriptDocValues.GeoPoints script = new ScriptDocValues.GeoPoints(new GeoPointsSupplier(values));
+        final ScriptDocValues.GeoPoints script = (GeoPoints) new GeoPointDocValuesField(values, "test").getScriptDocValues();
         script.getSupplier().setNextDocId(0);
 
         GeoPoint[][] points2 = { new GeoPoint[0] };
-        final ScriptDocValues.GeoPoints emptyScript = new ScriptDocValues.GeoPoints(new GeoPointsSupplier(wrap(points2)));
+        final ScriptDocValues.GeoPoints emptyScript = (GeoPoints) new GeoPointDocValuesField(wrap(points2), "test").getScriptDocValues();
         emptyScript.getSupplier().setNextDocId(0);
 
         final double otherLat = randomLat();
@@ -116,7 +116,7 @@ public class ScriptDocValuesGeoPointsTests extends ESTestCase {
                 points[d][i] = new GeoPoint(randomLat(), randomLon());
             }
         }
-        final ScriptDocValues.GeoPoints geoPoints = new GeoPoints(new GeoPointsSupplier(wrap(points)));
+        final ScriptDocValues.GeoPoints geoPoints = (GeoPoints) new GeoPointDocValuesField(wrap(points), "test").getScriptDocValues();
         for (int d = 0; d < points.length; d++) {
             geoPoints.getSupplier().setNextDocId(d);
             if (points[d].length > 0) {

+ 4 - 4
x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/fielddata/plain/AbstractAtomicGeoShapeShapeFieldData.java

@@ -104,12 +104,12 @@ public abstract class AbstractAtomicGeoShapeShapeFieldData implements LeafGeoSha
         }
 
         @Override
-        public GeoPoint getCentroid() {
+        public GeoPoint getInternalCentroid() {
             return centroid;
         }
 
         @Override
-        public GeoBoundingBox getBoundingBox() {
+        public GeoBoundingBox getInternalBoundingBox() {
             return boundingBox;
         }
     }
@@ -130,7 +130,7 @@ public abstract class AbstractAtomicGeoShapeShapeFieldData implements LeafGeoSha
 
         @Override
         public GeoPoint getCentroid() {
-            return gsSupplier.getInternal() == null ? null : gsSupplier.getCentroid();
+            return gsSupplier.getInternal() == null ? null : gsSupplier.getInternalCentroid();
         }
 
         @Override
@@ -145,7 +145,7 @@ public abstract class AbstractAtomicGeoShapeShapeFieldData implements LeafGeoSha
 
         @Override
         public GeoBoundingBox getBoundingBox() {
-            return gsSupplier.getInternal() == null ? null : gsSupplier.getBoundingBox();
+            return gsSupplier.getInternal() == null ? null : gsSupplier.getInternalBoundingBox();
         }
 
         @Override