Browse Source

Format runtime geo_points (#85449)

This formats the result of the `fields` section of the `_search` API for
runtime `geo_point` fields using the `format` parameter like we do for
non-runtime `geo_point` fields. This changes the default format for
those fields from `lat, lon` to `geojson` with the option to get `wkt`
or any other format we support.

The fix does so by preserving the `double, double` nature of the
`geo_point` rather than encoding it immediately in the script. Callers can
use the results. The field fetchers use the `double, double` natively,
preserving as much precision as possible. The queries quantize the points
exactly like lucene indexing does. And like the script did before this Pr.

Closes #85245
Nik Everett 3 years ago
parent
commit
3bcee8eaa0
23 changed files with 316 additions and 74 deletions
  1. 6 0
      docs/changelog/85449.yaml
  2. 2 2
      docs/painless/painless-guide/painless-execute-script.asciidoc
  3. 1 1
      modules/lang-painless/src/main/java/org/elasticsearch/painless/action/PainlessExecuteAction.java
  4. 8 1
      modules/runtime-fields-common/build.gradle
  5. 2 2
      modules/runtime-fields-common/src/yamlRestTest/resources/rest-api-spec/test/runtime_fields/100_geo_point.yml
  6. 2 2
      modules/runtime-fields-common/src/yamlRestTest/resources/rest-api-spec/test/runtime_fields/101_geo_point_from_source.yml
  7. 1 1
      modules/runtime-fields-common/src/yamlRestTest/resources/rest-api-spec/test/runtime_fields/102_geo_point_source_in_query.yml
  8. 1 1
      modules/runtime-fields-common/src/yamlRestTest/resources/rest-api-spec/test/runtime_fields/103_geo_point_calculated_at_index.yml
  9. 55 0
      rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search/330_fetch_fields.yml
  10. 32 4
      server/src/main/java/org/elasticsearch/index/fielddata/GeoPointScriptDocValues.java
  11. 1 1
      server/src/main/java/org/elasticsearch/index/mapper/AbstractScriptFieldType.java
  12. 2 2
      server/src/main/java/org/elasticsearch/index/mapper/GeoPointFieldMapper.java
  13. 73 1
      server/src/main/java/org/elasticsearch/index/mapper/GeoPointScriptFieldType.java
  14. 54 15
      server/src/main/java/org/elasticsearch/script/GeoPointFieldScript.java
  15. 2 2
      server/src/main/java/org/elasticsearch/search/runtime/AbstractGeoPointScriptFieldQuery.java
  16. 1 1
      server/src/main/java/org/elasticsearch/search/runtime/GeoPointScriptFieldExistsQuery.java
  17. 15 19
      server/src/main/java/org/elasticsearch/search/runtime/GeoPointScriptFieldGeoShapeQuery.java
  18. 8 0
      server/src/test/java/org/elasticsearch/index/mapper/AbstractScriptFieldTypeTestCase.java
  19. 20 0
      server/src/test/java/org/elasticsearch/index/mapper/GeoPointScriptFieldTypeTests.java
  20. 11 11
      server/src/test/java/org/elasticsearch/index/mapper/GeoPointScriptMapperTests.java
  21. 3 2
      server/src/test/java/org/elasticsearch/search/runtime/GeoPointScriptFieldDistanceFeatureQueryTests.java
  22. 11 3
      server/src/test/java/org/elasticsearch/search/runtime/GeoPointScriptFieldExistsQueryTests.java
  23. 5 3
      server/src/test/java/org/elasticsearch/search/runtime/GeoPointScriptFieldGeoShapeQueryTests.java

+ 6 - 0
docs/changelog/85449.yaml

@@ -0,0 +1,6 @@
+pr: 85449
+summary: Format runtime `geo_points`
+area: Geo
+type: bug
+issues:
+ - 85245

+ 2 - 2
docs/painless/painless-guide/painless-execute-script.asciidoc

@@ -631,8 +631,8 @@ results that are formatted as `coordinates`.
   "result" : [
     {
       "coordinates" : [
-        -71.34000004269183,
-        41.1199999647215
+        -71.34,
+        41.12
       ],
       "type" : "Point"
     }

+ 1 - 1
modules/lang-painless/src/main/java/org/elasticsearch/painless/action/PainlessExecuteAction.java

@@ -626,7 +626,7 @@ public class PainlessExecuteAction extends ActionType<PainlessExecuteAction.Resp
                     );
                     GeoPointFieldScript geoPointFieldScript = leafFactory.newInstance(leafReaderContext);
                     List<GeoPoint> points = new ArrayList<>();
-                    geoPointFieldScript.runGeoPointForDoc(0, gp -> points.add(new GeoPoint(gp)));
+                    geoPointFieldScript.runForDoc(0, gp -> points.add(new GeoPoint(gp)));
                     // convert geo points to the standard format of the fields api
                     Function<List<GeoPoint>, List<Object>> format = GeometryFormatterFactory.getFormatter(
                         GeometryFormatterFactory.GEOJSON,

+ 8 - 1
modules/runtime-fields-common/build.gradle

@@ -20,4 +20,11 @@ dependencies {
   compileOnly project(':modules:lang-painless:spi')
   api project(':libs:elasticsearch-grok')
   api project(':libs:elasticsearch-dissect')
-}
+}
+
+tasks.named("yamlRestTestV7CompatTransform").configure { task ->
+  task.skipTest("runtime_fields/100_geo_point/fetch fields from source", "Format changed. Old format was a bug.")
+  task.skipTest("runtime_fields/101_geo_point_from_source/fetch fields from source", "Format changed. Old format was a bug.")
+  task.skipTest("runtime_fields/102_geo_point_source_in_query/fetch fields from source", "Format changed. Old format was a bug.")
+  task.skipTest("runtime_fields/103_geo_point_calculated_at_index/fetch fields from source", "Format changed. Old format was a bug.")
+}

+ 2 - 2
modules/runtime-fields-common/src/yamlRestTest/resources/rest-api-spec/test/runtime_fields/100_geo_point.yml

@@ -65,8 +65,8 @@ setup:
   - match: {hits.total.value: 6}
   - match: {hits.hits.0.fields.location.0.type: "Point" }
   - match: {hits.hits.0.fields.location.0.coordinates: [34.89, 13.5] }
-  - match: {hits.hits.0.fields.location_from_doc_value: ["13.499999991618097, 34.889999935403466"] }
-  - match: {hits.hits.0.fields.location_from_source: ["13.499999991618097, 34.889999935403466"] }
+  - match: {hits.hits.0.fields.location_from_doc_value: [{type: Point, coordinates: [34.889999935403466, 13.499999991618097]}] }
+  - match: {hits.hits.0.fields.location_from_source: [{type: Point, coordinates: [34.89, 13.5]}] }
 
 ---
 "exists query":

+ 2 - 2
modules/runtime-fields-common/src/yamlRestTest/resources/rest-api-spec/test/runtime_fields/101_geo_point_from_source.yml

@@ -28,7 +28,7 @@ setup:
           {"index":{}}
           {"timestamp": "1998-04-30T14:30:53-05:00", "location" : "-7.9, 120.78"}
           {"index":{}}
-           {"timestamp": "1998-04-30T14:30:54-05:00"}
+          {"timestamp": "1998-04-30T14:30:54-05:00"}
           {"index":{}}
           {"timestamp": "1998-04-30T14:30:55-05:00", "location" : ["-8, 120", "-7, 121.0"]}
           {"index":{}}
@@ -62,7 +62,7 @@ setup:
           sort: timestamp
           fields: [location]
   - match: {hits.total.value: 11}
-  - match: {hits.hits.0.fields.location: ["13.499999991618097, 34.889999935403466"] }
+  - match: {hits.hits.0.fields.location: [{type: Point, coordinates: [34.89, 13.5]}] }
 
 ---
 "exists query":

+ 1 - 1
modules/runtime-fields-common/src/yamlRestTest/resources/rest-api-spec/test/runtime_fields/102_geo_point_source_in_query.yml

@@ -53,7 +53,7 @@ setup:
           sort: timestamp
           fields: [location]
   - match: {hits.total.value: 11}
-  - match: {hits.hits.0.fields.location: ["13.499999991618097, 34.889999935403466"] }
+  - match: {hits.hits.0.fields.location: [{type: Point, coordinates: [34.89, 13.5]}] }
 
 ---
 "exists query":

+ 1 - 1
modules/runtime-fields-common/src/yamlRestTest/resources/rest-api-spec/test/runtime_fields/103_geo_point_calculated_at_index.yml

@@ -68,7 +68,7 @@ setup:
   - match: { hits.hits.0.fields.location_from_doc_value.0.type: "Point" }
   - match: { hits.hits.0.fields.location_from_doc_value.0.coordinates: [ 34.889999935403466, 13.499999991618097 ] }
   - match: { hits.hits.0.fields.location_from_source.0.type: "Point" }
-  - match: { hits.hits.0.fields.location_from_source.0.coordinates: [ 34.889999935403466, 13.499999991618097 ] }
+  - match: { hits.hits.0.fields.location_from_source.0.coordinates: [ 34.89, 13.5 ] }
 
 ---
 "exists query":

+ 55 - 0
rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search/330_fetch_fields.yml

@@ -1065,3 +1065,58 @@ test fetching metadata fields:
   - match: { hits.hits.0.fields._id.0: "1" }
   - match: { hits.hits.0.fields._index.0: "test" }
   - match: { hits.hits.0.fields._version.0: 1 }
+
+---
+fetch geo_point:
+  - do:
+      indices.create:
+        index: test
+        body:
+          settings:
+            index.number_of_shards: 1
+          mappings:
+            properties:
+              field:
+                type: geo_point
+
+  - do:
+      index:
+        index:   test
+        id:      "1"
+        refresh: true
+        body:
+          field:
+            lat: 41.12
+            lon: -71.34
+
+  - do:
+      search:
+        index: test
+        body:
+          fields: [ "*" ]
+  - length: { hits.hits.0.fields: 1 }
+  - match: { hits.hits.0.fields.field.0.type: Point }
+  - match: { hits.hits.0.fields.field.0.coordinates.0: -71.34 }
+  - match: { hits.hits.0.fields.field.0.coordinates.1:  41.12 }
+
+  - do:
+      search:
+        index: test
+        body:
+          fields:
+            - field: field
+              format: geojson
+  - length: { hits.hits.0.fields: 1 }
+  - match: { hits.hits.0.fields.field.0.type: Point }
+  - match: { hits.hits.0.fields.field.0.coordinates.0: -71.34 }
+  - match: { hits.hits.0.fields.field.0.coordinates.1:  41.12 }
+
+  - do:
+      search:
+        index: test
+        body:
+          fields:
+            - field: field
+              format: wkt
+  - length: { hits.hits.0.fields: 1 }
+  - match: { hits.hits.0.fields.field.0: "POINT (-71.34 41.12)" }

+ 32 - 4
server/src/main/java/org/elasticsearch/index/fielddata/GeoPointScriptDocValues.java

@@ -8,10 +8,10 @@
 
 package org.elasticsearch.index.fielddata;
 
+import org.apache.lucene.geo.GeoEncodingUtils;
+import org.apache.lucene.util.IntroSorter;
 import org.elasticsearch.script.GeoPointFieldScript;
 
-import java.util.Arrays;
-
 public final class GeoPointScriptDocValues extends AbstractSortedNumericDocValues {
     private final GeoPointFieldScript script;
     private int cursor;
@@ -26,7 +26,33 @@ public final class GeoPointScriptDocValues extends AbstractSortedNumericDocValue
         if (script.count() == 0) {
             return false;
         }
-        Arrays.sort(script.values(), 0, script.count());
+        new IntroSorter() {
+            private int pivot;
+
+            @Override
+            protected void setPivot(int i) {
+                this.pivot = i;
+            }
+
+            @Override
+            protected void swap(int i, int j) {
+                double tmp = script.lats()[i];
+                script.lats()[i] = script.lats()[j];
+                script.lats()[j] = tmp;
+                tmp = script.lons()[i];
+                script.lons()[i] = script.lons()[j];
+                script.lons()[j] = tmp;
+            }
+
+            @Override
+            protected int comparePivot(int j) {
+                int cmp = Double.compare(script.lats()[pivot], script.lats()[j]);
+                if (cmp != 0) {
+                    return cmp;
+                }
+                return Double.compare(script.lons()[pivot], script.lons()[j]);
+            }
+        }.sort(0, script.count());
         cursor = 0;
         return true;
     }
@@ -38,6 +64,8 @@ public final class GeoPointScriptDocValues extends AbstractSortedNumericDocValue
 
     @Override
     public long nextValue() {
-        return script.values()[cursor++];
+        int lat = GeoEncodingUtils.encodeLatitude(script.lats()[cursor]);
+        int lon = GeoEncodingUtils.encodeLongitude(script.lons()[cursor++]);
+        return Long.valueOf((((long) lat) << 32) | (lon & 0xFFFFFFFFL));
     }
 }

+ 1 - 1
server/src/main/java/org/elasticsearch/index/mapper/AbstractScriptFieldType.java

@@ -172,7 +172,7 @@ abstract class AbstractScriptFieldType<LeafFactory> extends MappedFieldType {
     }
 
     @Override
-    public final ValueFetcher valueFetcher(SearchExecutionContext context, String format) {
+    public ValueFetcher valueFetcher(SearchExecutionContext context, String format) {
         return new DocValueFetcher(docValueFormat(format, null), context.getForField(this));
     }
 

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

@@ -143,7 +143,7 @@ public class GeoPointFieldMapper extends AbstractPointGeometryFieldMapper<GeoPoi
                 ? null
                 : (lookup, ctx, doc, consumer) -> factory.newFactory(name, script.get().getParams(), lookup)
                     .newInstance(ctx)
-                    .runGeoPointForDoc(doc, consumer);
+                    .runForDoc(doc, consumer);
         }
 
         @Override
@@ -290,7 +290,7 @@ public class GeoPointFieldMapper extends AbstractPointGeometryFieldMapper<GeoPoi
 
     public static class GeoPointFieldType extends AbstractGeometryFieldType<GeoPoint> implements GeoShapeQueryable {
 
-        private static final GeoFormatterFactory<GeoPoint> GEO_FORMATTER_FACTORY = new GeoFormatterFactory<>(
+        public static final GeoFormatterFactory<GeoPoint> GEO_FORMATTER_FACTORY = new GeoFormatterFactory<>(
             List.of(new SimpleVectorTileFormatter())
         );
 

+ 73 - 1
server/src/main/java/org/elasticsearch/index/mapper/GeoPointScriptFieldType.java

@@ -8,29 +8,37 @@
 
 package org.elasticsearch.index.mapper;
 
+import org.apache.lucene.geo.GeoEncodingUtils;
 import org.apache.lucene.geo.LatLonGeometry;
 import org.apache.lucene.geo.Point;
+import org.apache.lucene.index.LeafReaderContext;
 import org.apache.lucene.search.MatchNoDocsQuery;
 import org.apache.lucene.search.Query;
 import org.elasticsearch.common.geo.GeoPoint;
 import org.elasticsearch.common.geo.GeoUtils;
+import org.elasticsearch.common.geo.GeometryFormatterFactory;
 import org.elasticsearch.common.geo.ShapeRelation;
 import org.elasticsearch.common.time.DateMathParser;
 import org.elasticsearch.common.unit.DistanceUnit;
 import org.elasticsearch.index.fielddata.FieldDataContext;
 import org.elasticsearch.index.fielddata.GeoPointScriptFieldData;
 import org.elasticsearch.index.query.SearchExecutionContext;
+import org.elasticsearch.script.AbstractLongFieldScript;
 import org.elasticsearch.script.CompositeFieldScript;
 import org.elasticsearch.script.GeoPointFieldScript;
 import org.elasticsearch.script.Script;
 import org.elasticsearch.script.field.GeoPointDocValuesField;
 import org.elasticsearch.search.lookup.SearchLookup;
+import org.elasticsearch.search.lookup.SourceLookup;
 import org.elasticsearch.search.runtime.GeoPointScriptFieldDistanceFeatureQuery;
 import org.elasticsearch.search.runtime.GeoPointScriptFieldExistsQuery;
 import org.elasticsearch.search.runtime.GeoPointScriptFieldGeoShapeQuery;
 
+import java.io.IOException;
 import java.time.ZoneId;
+import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.List;
 import java.util.Map;
 import java.util.function.Function;
 
@@ -131,11 +139,75 @@ public final class GeoPointScriptFieldType extends AbstractScriptFieldType<GeoPo
         double pivotDouble = DistanceUnit.DEFAULT.parse(pivot, DistanceUnit.DEFAULT);
         return new GeoPointScriptFieldDistanceFeatureQuery(
             script,
-            leafFactory(context)::newInstance,
+            valuesEncodedAsLong(context.lookup(), name(), leafFactory(context)::newInstance),
             name(),
             originGeoPoint.lat(),
             originGeoPoint.lon(),
             pivotDouble
         );
     }
+
+    public static Function<LeafReaderContext, AbstractLongFieldScript> valuesEncodedAsLong(
+        SearchLookup lookup,
+        String name,
+        Function<LeafReaderContext, GeoPointFieldScript> delegateLeafFactory
+    ) {
+        return ctx -> {
+            GeoPointFieldScript script = delegateLeafFactory.apply(ctx);
+            return new AbstractLongFieldScript(name, Map.of(), lookup, ctx) {
+                private int docId;
+
+                @Override
+                protected void emitFromObject(Object v) {
+                    throw new UnsupportedOperationException();
+                }
+
+                @Override
+                public void setDocument(int docID) {
+                    super.setDocument(docID);
+                    this.docId = docID;
+                }
+
+                @Override
+                public void execute() {
+                    script.runForDoc(docId);
+                    for (int i = 0; i < script.count(); i++) {
+                        int latitudeEncoded = GeoEncodingUtils.encodeLatitude(script.lats()[i]);
+                        int longitudeEncoded = GeoEncodingUtils.encodeLongitude(script.lons()[i]);
+                        emit((((long) latitudeEncoded) << 32) | (longitudeEncoded & 0xFFFFFFFFL));
+                    }
+                }
+            };
+        };
+    }
+
+    @Override
+    public ValueFetcher valueFetcher(SearchExecutionContext context, String format) {
+        GeoPointFieldScript.LeafFactory leafFactory = leafFactory(context.lookup());
+        Function<List<GeoPoint>, List<Object>> formatter = GeoPointFieldMapper.GeoPointFieldType.GEO_FORMATTER_FACTORY.getFormatter(
+            format != null ? format : GeometryFormatterFactory.GEOJSON,
+            p -> new org.elasticsearch.geometry.Point(p.getLon(), p.getLat())
+        );
+        return new ValueFetcher() {
+            private GeoPointFieldScript script;
+
+            @Override
+            public void setNextReader(LeafReaderContext context) {
+                script = leafFactory.newInstance(context);
+            }
+
+            @Override
+            public List<Object> fetchValues(SourceLookup lookup, List<Object> ignoredValues) throws IOException {
+                script.runForDoc(lookup.docId());
+                if (script.count() == 0) {
+                    return List.of();
+                }
+                List<GeoPoint> points = new ArrayList<>(script.count());
+                for (int i = 0; i < script.count(); i++) {
+                    points.add(new GeoPoint(script.lats()[i], script.lons()[i]));
+                }
+                return formatter.apply(points);
+            }
+        };
+    }
 }

+ 54 - 15
server/src/main/java/org/elasticsearch/script/GeoPointFieldScript.java

@@ -9,8 +9,8 @@
 package org.elasticsearch.script;
 
 import org.apache.lucene.document.LatLonDocValuesField;
-import org.apache.lucene.geo.GeoEncodingUtils;
 import org.apache.lucene.index.LeafReaderContext;
+import org.apache.lucene.util.ArrayUtil;
 import org.elasticsearch.common.geo.GeoPoint;
 import org.elasticsearch.common.geo.GeoUtils;
 import org.elasticsearch.common.xcontent.support.XContentMapValues;
@@ -22,14 +22,11 @@ import java.util.Map;
 import java.util.function.Consumer;
 import java.util.function.Function;
 
-import static org.apache.lucene.geo.GeoEncodingUtils.encodeLatitude;
-import static org.apache.lucene.geo.GeoEncodingUtils.encodeLongitude;
-
 /**
  * Script producing geo points. Similarly to what {@link LatLonDocValuesField} does,
  * it encodes the points as a long value.
  */
-public abstract class GeoPointFieldScript extends AbstractLongFieldScript {
+public abstract class GeoPointFieldScript extends AbstractFieldScript {
     public static final ScriptContext<Factory> CONTEXT = newContext("geo_point_field", Factory.class);
 
     public static final Factory PARSE_FROM_SOURCE = new Factory() {
@@ -80,24 +77,62 @@ public abstract class GeoPointFieldScript extends AbstractLongFieldScript {
         GeoPointFieldScript newInstance(LeafReaderContext ctx);
     }
 
+    private double[] lats = new double[1];
+    private double[] lons = new double[1];
+    private int count;
+
     public GeoPointFieldScript(String fieldName, Map<String, Object> params, SearchLookup searchLookup, LeafReaderContext ctx) {
         super(fieldName, params, searchLookup, ctx);
     }
 
     /**
-     * Consumers must copy the emitted GeoPoint(s) if stored.
+     * Execute the script for the provided {@code docId}.
+     */
+    public final void runForDoc(int docId) {
+        count = 0;
+        setDocument(docId);
+        execute();
+    }
+
+    /**
+     * Execute the script for the provided {@code docId}, passing results to the {@code consumer}
      */
-    public void runGeoPointForDoc(int doc, Consumer<GeoPoint> consumer) {
-        runForDoc(doc);
+    public final void runForDoc(int docId, Consumer<GeoPoint> consumer) {
+        runForDoc(docId);
         GeoPoint point = new GeoPoint();
-        for (int i = 0; i < count(); i++) {
-            final int lat = (int) (values()[i] >>> 32);
-            final int lon = (int) (values()[i] & 0xFFFFFFFF);
-            point.reset(GeoEncodingUtils.decodeLatitude(lat), GeoEncodingUtils.decodeLongitude(lon));
+        for (int i = 0; i < count; i++) {
+            point.reset(lats[i], lons[i]);
             consumer.accept(point);
         }
     }
 
+    /**
+     * Latitude values from the last time {@link #runForDoc(int)} was called. This
+     * array is mutable and will change with the next call of {@link #runForDoc(int)}.
+     * It is also oversized and will contain garbage at all indices at and
+     * above {@link #count()}.
+     */
+    public final double[] lats() {
+        return lats;
+    }
+
+    /**
+     * Longitude values from the last time {@link #runForDoc(int)} was called. This
+     * array is mutable and will change with the next call of {@link #runForDoc(int)}.
+     * It is also oversized and will contain garbage at all indices at and
+     * above {@link #count()}.
+     */
+    public final double[] lons() {
+        return lons;
+    }
+
+    /**
+     * The number of results produced the last time {@link #runForDoc(int)} was called.
+     */
+    public final int count() {
+        return count;
+    }
+
     @Override
     protected List<Object> extractFromSource(String path) {
         Object value = XContentMapValues.extractValue(path, sourceLookup.source());
@@ -142,9 +177,13 @@ public abstract class GeoPointFieldScript extends AbstractLongFieldScript {
     }
 
     protected final void emit(double lat, double lon) {
-        int latitudeEncoded = encodeLatitude(lat);
-        int longitudeEncoded = encodeLongitude(lon);
-        emit((((long) latitudeEncoded) << 32) | (longitudeEncoded & 0xFFFFFFFFL));
+        checkMaxSize(count);
+        if (lats.length < count + 1) {
+            lats = ArrayUtil.grow(lats, count + 1);
+            lons = ArrayUtil.growExact(lons, lats.length);
+        }
+        lats[count] = lat;
+        lons[count++] = lon;
     }
 
     public static class Emit {

+ 2 - 2
server/src/main/java/org/elasticsearch/search/runtime/AbstractGeoPointScriptFieldQuery.java

@@ -23,11 +23,11 @@ abstract class AbstractGeoPointScriptFieldQuery extends AbstractScriptFieldQuery
     @Override
     protected boolean matches(GeoPointFieldScript scriptContext, int docId) {
         scriptContext.runForDoc(docId);
-        return matches(scriptContext.values(), scriptContext.count());
+        return matches(scriptContext.lats(), scriptContext.lons(), scriptContext.count());
     }
 
     /**
      * Does the value match this query?
      */
-    protected abstract boolean matches(long[] values, int count);
+    protected abstract boolean matches(double[] lats, double[] lons, int count);
 }

+ 1 - 1
server/src/main/java/org/elasticsearch/search/runtime/GeoPointScriptFieldExistsQuery.java

@@ -17,7 +17,7 @@ public class GeoPointScriptFieldExistsQuery extends AbstractGeoPointScriptFieldQ
     }
 
     @Override
-    protected boolean matches(long[] values, int count) {
+    protected boolean matches(double[] lats, double[] lons, int count) {
         return count > 0;
     }
 

+ 15 - 19
server/src/main/java/org/elasticsearch/search/runtime/GeoPointScriptFieldGeoShapeQuery.java

@@ -38,8 +38,8 @@ public class GeoPointScriptFieldGeoShapeQuery extends AbstractGeoPointScriptFiel
     }
 
     @Override
-    protected boolean matches(long[] values, int count) {
-        return predicate.matches(values, count);
+    protected boolean matches(double[] lats, double[] lons, int count) {
+        return predicate.matches(lats, lons, count);
     }
 
     @Override
@@ -66,7 +66,7 @@ public class GeoPointScriptFieldGeoShapeQuery extends AbstractGeoPointScriptFiel
 
     @FunctionalInterface
     private interface SpatialPredicate {
-        boolean matches(long[] values, int count);
+        boolean matches(double[] lats, double[] lons, int count);
     }
 
     private static SpatialPredicate getPredicate(ShapeRelation relation, LatLonGeometry... geometries) {
@@ -75,11 +75,10 @@ public class GeoPointScriptFieldGeoShapeQuery extends AbstractGeoPointScriptFiel
                 final GeoEncodingUtils.Component2DPredicate predicate = GeoEncodingUtils.createComponentPredicate(
                     LatLonGeometry.create(geometries)
                 );
-                return (values, count) -> {
+                return (lats, lons, count) -> {
                     for (int i = 0; i < count; i++) {
-                        final long value = values[i];
-                        final int lat = (int) (value >>> 32);
-                        final int lon = (int) (value & 0xFFFFFFFF);
+                        final int lat = GeoEncodingUtils.encodeLatitude(lats[i]);
+                        final int lon = GeoEncodingUtils.encodeLongitude(lons[i]);
                         if (predicate.test(lat, lon)) {
                             return true;
                         }
@@ -91,11 +90,10 @@ public class GeoPointScriptFieldGeoShapeQuery extends AbstractGeoPointScriptFiel
                 final GeoEncodingUtils.Component2DPredicate predicate = GeoEncodingUtils.createComponentPredicate(
                     LatLonGeometry.create(geometries)
                 );
-                return (values, count) -> {
+                return (lats, lons, count) -> {
                     for (int i = 0; i < count; i++) {
-                        final long value = values[i];
-                        final int lat = (int) (value >>> 32);
-                        final int lon = (int) (value & 0xFFFFFFFF);
+                        final int lat = GeoEncodingUtils.encodeLatitude(lats[i]);
+                        final int lon = GeoEncodingUtils.encodeLongitude(lons[i]);
                         if (predicate.test(lat, lon)) {
                             return false;
                         }
@@ -108,11 +106,10 @@ public class GeoPointScriptFieldGeoShapeQuery extends AbstractGeoPointScriptFiel
                 final GeoEncodingUtils.Component2DPredicate predicate = GeoEncodingUtils.createComponentPredicate(
                     LatLonGeometry.create(geometries)
                 );
-                return (values, count) -> {
+                return (lats, lons, count) -> {
                     for (int i = 0; i < count; i++) {
-                        final long value = values[i];
-                        final int lat = (int) (value >>> 32);
-                        final int lon = (int) (value & 0xFFFFFFFF);
+                        final int lat = GeoEncodingUtils.encodeLatitude(lats[i]);
+                        final int lon = GeoEncodingUtils.encodeLongitude(lons[i]);
                         if (predicate.test(lat, lon) == false) {
                             return false;
                         }
@@ -126,12 +123,11 @@ public class GeoPointScriptFieldGeoShapeQuery extends AbstractGeoPointScriptFiel
                 for (int i = 0; i < geometries.length; i++) {
                     component2DS[i] = LatLonGeometry.create(geometries[i]);
                 }
-                return (values, count) -> {
+                return (lats, lons, count) -> {
                     Component2D.WithinRelation answer = Component2D.WithinRelation.DISJOINT;
                     for (int i = 0; i < count; i++) {
-                        final long value = values[i];
-                        final double lat = GeoEncodingUtils.decodeLatitude((int) (value >>> 32));
-                        final double lon = GeoEncodingUtils.decodeLongitude((int) (value & 0xFFFFFFFF));
+                        final int lat = GeoEncodingUtils.encodeLatitude(lats[i]);
+                        final int lon = GeoEncodingUtils.encodeLongitude(lons[i]);
                         for (Component2D component2D : component2DS) {
                             Component2D.WithinRelation withinRelation = component2D.withinPoint(lon, lat);
                             if (withinRelation == Component2D.WithinRelation.NOTWITHIN) {

+ 8 - 0
server/src/test/java/org/elasticsearch/index/mapper/AbstractScriptFieldTypeTestCase.java

@@ -16,7 +16,9 @@ import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.geo.ShapeRelation;
 import org.elasticsearch.common.xcontent.XContentHelper;
 import org.elasticsearch.index.fielddata.FieldDataContext;
+import org.elasticsearch.index.fielddata.IndexFieldDataCache;
 import org.elasticsearch.index.query.SearchExecutionContext;
+import org.elasticsearch.indices.breaker.NoneCircuitBreakerService;
 import org.elasticsearch.script.BooleanFieldScript;
 import org.elasticsearch.script.DateFieldScript;
 import org.elasticsearch.script.DoubleFieldScript;
@@ -39,6 +41,7 @@ import java.util.function.BiConsumer;
 
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
+import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
@@ -210,6 +213,11 @@ public abstract class AbstractScriptFieldTypeTestCase extends MapperServiceTestC
             (mft, lookupSupplier) -> mft.fielddataBuilder(new FieldDataContext("test", lookupSupplier)).build(null, null)
         );
         when(context.lookup()).thenReturn(lookup);
+        when(context.getForField(any())).then(args -> {
+            MappedFieldType ft = args.getArgument(0);
+            return ft.fielddataBuilder(new FieldDataContext("test", context::lookup))
+                .build(new IndexFieldDataCache.None(), new NoneCircuitBreakerService());
+        });
         return context;
     }
 

+ 20 - 0
server/src/test/java/org/elasticsearch/index/mapper/GeoPointScriptFieldTypeTests.java

@@ -103,6 +103,26 @@ public class GeoPointScriptFieldTypeTests extends AbstractNonTextScriptFieldType
         assertThat(e.getMessage(), equalTo("can't sort on geo_point field without using specific sorting feature, like geo_distance"));
     }
 
+    public void testFetch() throws IOException {
+        try (Directory directory = newDirectory(); RandomIndexWriter iw = new RandomIndexWriter(random(), directory)) {
+            iw.addDocument(List.of(new StoredField("_source", new BytesRef("""
+                {"foo": {"lat": 45.0, "lon" : 45.0}}"""))));
+            try (DirectoryReader reader = iw.getReader()) {
+                SearchExecutionContext searchContext = mockContext(true, simpleMappedFieldType());
+                searchContext.lookup().source().setSegmentAndDocument(reader.leaves().get(0), 0);
+                ValueFetcher fetcher = simpleMappedFieldType().valueFetcher(searchContext, randomBoolean() ? null : "geojson");
+                fetcher.setNextReader(reader.leaves().get(0));
+                assertThat(
+                    fetcher.fetchValues(searchContext.lookup().source(), null),
+                    equalTo(List.of(Map.of("type", "Point", "coordinates", List.of(45.0, 45.0))))
+                );
+                fetcher = simpleMappedFieldType().valueFetcher(searchContext, "wkt");
+                fetcher.setNextReader(reader.leaves().get(0));
+                assertThat(fetcher.fetchValues(searchContext.lookup().source(), null), equalTo(List.of("POINT (45.0 45.0)")));
+            }
+        }
+    }
+
     @Override
     public void testUsedInScript() throws IOException {
         try (Directory directory = newDirectory(); RandomIndexWriter iw = new RandomIndexWriter(random(), directory)) {

+ 11 - 11
server/src/test/java/org/elasticsearch/index/mapper/GeoPointScriptMapperTests.java

@@ -18,7 +18,7 @@ import java.util.function.Consumer;
 
 public class GeoPointScriptMapperTests extends MapperScriptTestCase<GeoPointFieldScript.Factory> {
 
-    private static GeoPointFieldScript.Factory factory(Consumer<GeoPointFieldScript> executor) {
+    private static GeoPointFieldScript.Factory factory(Consumer<GeoPointFieldScript.Emit> executor) {
         return new GeoPointFieldScript.Factory() {
             @Override
             public GeoPointFieldScript.LeafFactory newFactory(String fieldName, Map<String, Object> params, SearchLookup searchLookup) {
@@ -28,7 +28,7 @@ public class GeoPointScriptMapperTests extends MapperScriptTestCase<GeoPointFiel
                         return new GeoPointFieldScript(fieldName, params, searchLookup, ctx) {
                             @Override
                             public void execute() {
-                                executor.accept(this);
+                                executor.accept(new Emit(this));
                             }
                         };
                     }
@@ -54,35 +54,35 @@ public class GeoPointScriptMapperTests extends MapperScriptTestCase<GeoPointFiel
 
     @Override
     protected GeoPointFieldScript.Factory singleValueScript() {
-        return factory(s -> s.emit(1));
+        return factory(s -> s.emit(-1, 1));
     }
 
     @Override
     protected GeoPointFieldScript.Factory multipleValuesScript() {
         return factory(s -> {
-            s.emit(1);
-            s.emit(2);
+            s.emit(-1, 1);
+            s.emit(-2, 2);
         });
     }
 
     @Override
     protected void assertMultipleValues(IndexableField[] fields) {
         assertEquals(4, fields.length);
-        assertEquals("LatLonPoint <field:0.0,8.381903171539307E-8>", fields[0].toString());
-        assertEquals("LatLonDocValuesField <field:0.0,8.381903171539307E-8>", fields[1].toString());
-        assertEquals("LatLonPoint <field:0.0,1.6763806343078613E-7>", fields[2].toString());
-        assertEquals("LatLonDocValuesField <field:0.0,1.6763806343078613E-7>", fields[3].toString());
+        assertEquals("LatLonPoint <field:-1.000000024214387,0.9999999403953552>", fields[0].toString());
+        assertEquals("LatLonDocValuesField <field:-1.000000024214387,0.9999999403953552>", fields[1].toString());
+        assertEquals("LatLonPoint <field:-2.000000006519258,1.9999999646097422>", fields[2].toString());
+        assertEquals("LatLonDocValuesField <field:-2.000000006519258,1.9999999646097422>", fields[3].toString());
     }
 
     @Override
     protected void assertDocValuesDisabled(IndexableField[] fields) {
         assertEquals(1, fields.length);
-        assertEquals("LatLonPoint <field:0.0,8.381903171539307E-8>", fields[0].toString());
+        assertEquals("LatLonPoint <field:-1.000000024214387,0.9999999403953552>", fields[0].toString());
     }
 
     @Override
     protected void assertIndexDisabled(IndexableField[] fields) {
         assertEquals(1, fields.length);
-        assertEquals("LatLonDocValuesField <field:0.0,8.381903171539307E-8>", fields[0].toString());
+        assertEquals("LatLonDocValuesField <field:-1.000000024214387,0.9999999403953552>", fields[0].toString());
     }
 }

+ 3 - 2
server/src/test/java/org/elasticsearch/search/runtime/GeoPointScriptFieldDistanceFeatureQueryTests.java

@@ -20,6 +20,7 @@ import org.apache.lucene.tests.index.RandomIndexWriter;
 import org.apache.lucene.util.BytesRef;
 import org.elasticsearch.common.geo.GeoPoint;
 import org.elasticsearch.common.geo.GeoUtils;
+import org.elasticsearch.index.mapper.GeoPointScriptFieldType;
 import org.elasticsearch.script.AbstractLongFieldScript;
 import org.elasticsearch.script.GeoPointFieldScript;
 import org.elasticsearch.script.Script;
@@ -82,7 +83,7 @@ public class GeoPointScriptFieldDistanceFeatureQueryTests extends AbstractScript
             try (DirectoryReader reader = iw.getReader()) {
                 IndexSearcher searcher = newSearcher(reader);
                 SearchLookup searchLookup = new SearchLookup(null, null);
-                Function<LeafReaderContext, AbstractLongFieldScript> leafFactory = ctx -> new GeoPointFieldScript(
+                Function<LeafReaderContext, GeoPointFieldScript> leafFactory = ctx -> new GeoPointFieldScript(
                     "test",
                     Map.of(),
                     searchLookup,
@@ -96,7 +97,7 @@ public class GeoPointScriptFieldDistanceFeatureQueryTests extends AbstractScript
                 };
                 GeoPointScriptFieldDistanceFeatureQuery query = new GeoPointScriptFieldDistanceFeatureQuery(
                     randomScript(),
-                    leafFactory,
+                    GeoPointScriptFieldType.valuesEncodedAsLong(searchLookup, "test", leafFactory),
                     "test",
                     0,
                     0,

+ 11 - 3
server/src/test/java/org/elasticsearch/search/runtime/GeoPointScriptFieldExistsQueryTests.java

@@ -31,9 +31,17 @@ public class GeoPointScriptFieldExistsQueryTests extends AbstractGeoPointScriptF
 
     @Override
     public void testMatches() {
-        assertTrue(createTestInstance().matches(new long[] { 1L }, randomIntBetween(1, Integer.MAX_VALUE)));
-        assertFalse(createTestInstance().matches(new long[0], 0));
-        assertFalse(createTestInstance().matches(new long[1], 0));
+        assertTrue(
+            createTestInstance().matches(
+                new double[] { randomDouble() },
+                new double[] { randomDouble() },
+                randomIntBetween(1, Integer.MAX_VALUE)
+            )
+        );
+        assertFalse(createTestInstance().matches(new double[0], new double[0], 0));
+        assertFalse(createTestInstance().matches(new double[1], new double[0], 0));
+        assertFalse(createTestInstance().matches(new double[0], new double[1], 0));
+        assertFalse(createTestInstance().matches(new double[1], new double[1], 0));
     }
 
     @Override

+ 5 - 3
server/src/test/java/org/elasticsearch/search/runtime/GeoPointScriptFieldGeoShapeQueryTests.java

@@ -57,9 +57,11 @@ public class GeoPointScriptFieldGeoShapeQueryTests extends AbstractGeoPointScrip
 
     @Override
     public void testMatches() {
-        assertTrue(createTestInstance().matches(new long[] { 1L }, randomIntBetween(1, Integer.MAX_VALUE)));
-        assertFalse(createTestInstance().matches(new long[0], 0));
-        assertFalse(createTestInstance().matches(new long[1], 0));
+        // TODO this should actually reject some points
+        assertTrue(createTestInstance().matches(new double[] { 1 }, new double[] { 2 }, randomIntBetween(1, Integer.MAX_VALUE)));
+        assertFalse(createTestInstance().matches(new double[0], new double[0], 0));
+        assertFalse(createTestInstance().matches(new double[1], new double[0], 0));
+        assertFalse(createTestInstance().matches(new double[0], new double[1], 0));
     }
 
     @Override