diff --git a/docs/changelog/85449.yaml b/docs/changelog/85449.yaml new file mode 100644 index 000000000000..1b0ea7723a72 --- /dev/null +++ b/docs/changelog/85449.yaml @@ -0,0 +1,6 @@ +pr: 85449 +summary: Format runtime `geo_points` +area: Geo +type: bug +issues: + - 85245 diff --git a/docs/painless/painless-guide/painless-execute-script.asciidoc b/docs/painless/painless-guide/painless-execute-script.asciidoc index d5da3bf29c97..681b85274628 100644 --- a/docs/painless/painless-guide/painless-execute-script.asciidoc +++ b/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" } diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/action/PainlessExecuteAction.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/action/PainlessExecuteAction.java index 1f6f8a707110..e8310c42cacb 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/action/PainlessExecuteAction.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/action/PainlessExecuteAction.java @@ -626,7 +626,7 @@ public class PainlessExecuteAction extends ActionType 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> format = GeometryFormatterFactory.getFormatter( GeometryFormatterFactory.GEOJSON, diff --git a/modules/runtime-fields-common/build.gradle b/modules/runtime-fields-common/build.gradle index a429e9dd5978..5a2d268cf7a4 100644 --- a/modules/runtime-fields-common/build.gradle +++ b/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') -} \ No newline at end of file +} + +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.") +} diff --git a/modules/runtime-fields-common/src/yamlRestTest/resources/rest-api-spec/test/runtime_fields/100_geo_point.yml b/modules/runtime-fields-common/src/yamlRestTest/resources/rest-api-spec/test/runtime_fields/100_geo_point.yml index c97df3d5a9bb..ec2fc9edfbea 100644 --- a/modules/runtime-fields-common/src/yamlRestTest/resources/rest-api-spec/test/runtime_fields/100_geo_point.yml +++ b/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": diff --git a/modules/runtime-fields-common/src/yamlRestTest/resources/rest-api-spec/test/runtime_fields/101_geo_point_from_source.yml b/modules/runtime-fields-common/src/yamlRestTest/resources/rest-api-spec/test/runtime_fields/101_geo_point_from_source.yml index 3744e68432ee..cfb54d8040cf 100644 --- a/modules/runtime-fields-common/src/yamlRestTest/resources/rest-api-spec/test/runtime_fields/101_geo_point_from_source.yml +++ b/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": diff --git a/modules/runtime-fields-common/src/yamlRestTest/resources/rest-api-spec/test/runtime_fields/102_geo_point_source_in_query.yml b/modules/runtime-fields-common/src/yamlRestTest/resources/rest-api-spec/test/runtime_fields/102_geo_point_source_in_query.yml index 7dc04b086e01..b07f001bcd0c 100644 --- a/modules/runtime-fields-common/src/yamlRestTest/resources/rest-api-spec/test/runtime_fields/102_geo_point_source_in_query.yml +++ b/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": diff --git a/modules/runtime-fields-common/src/yamlRestTest/resources/rest-api-spec/test/runtime_fields/103_geo_point_calculated_at_index.yml b/modules/runtime-fields-common/src/yamlRestTest/resources/rest-api-spec/test/runtime_fields/103_geo_point_calculated_at_index.yml index d47c0200b9e2..486eab827379 100644 --- a/modules/runtime-fields-common/src/yamlRestTest/resources/rest-api-spec/test/runtime_fields/103_geo_point_calculated_at_index.yml +++ b/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": diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search/330_fetch_fields.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search/330_fetch_fields.yml index 808e7b12b795..3015770c6aef 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search/330_fetch_fields.yml +++ b/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)" } diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/GeoPointScriptDocValues.java b/server/src/main/java/org/elasticsearch/index/fielddata/GeoPointScriptDocValues.java index d487cd430d6e..acac799d5fd8 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/GeoPointScriptDocValues.java +++ b/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)); } } diff --git a/server/src/main/java/org/elasticsearch/index/mapper/AbstractScriptFieldType.java b/server/src/main/java/org/elasticsearch/index/mapper/AbstractScriptFieldType.java index 66d1390ea59f..c966ecacaa38 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/AbstractScriptFieldType.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/AbstractScriptFieldType.java @@ -172,7 +172,7 @@ abstract class AbstractScriptFieldType 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)); } diff --git a/server/src/main/java/org/elasticsearch/index/mapper/GeoPointFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/GeoPointFieldMapper.java index 1ee768cd94c4..e550acdcf623 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/GeoPointFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/GeoPointFieldMapper.java @@ -143,7 +143,7 @@ public class GeoPointFieldMapper extends AbstractPointGeometryFieldMapper 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 implements GeoShapeQueryable { - private static final GeoFormatterFactory GEO_FORMATTER_FACTORY = new GeoFormatterFactory<>( + public static final GeoFormatterFactory GEO_FORMATTER_FACTORY = new GeoFormatterFactory<>( List.of(new SimpleVectorTileFormatter()) ); diff --git a/server/src/main/java/org/elasticsearch/index/mapper/GeoPointScriptFieldType.java b/server/src/main/java/org/elasticsearch/index/mapper/GeoPointScriptFieldType.java index 91e83e1644a2..900a2daaa74f 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/GeoPointScriptFieldType.java +++ b/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 valuesEncodedAsLong( + SearchLookup lookup, + String name, + Function 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> 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 fetchValues(SourceLookup lookup, List ignoredValues) throws IOException { + script.runForDoc(lookup.docId()); + if (script.count() == 0) { + return List.of(); + } + List 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); + } + }; + } } diff --git a/server/src/main/java/org/elasticsearch/script/GeoPointFieldScript.java b/server/src/main/java/org/elasticsearch/script/GeoPointFieldScript.java index 5fa1d5c95873..d89cf9d4d069 100644 --- a/server/src/main/java/org/elasticsearch/script/GeoPointFieldScript.java +++ b/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 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 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 void runGeoPointForDoc(int doc, Consumer consumer) { - runForDoc(doc); + 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 final void runForDoc(int docId, Consumer 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 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 { diff --git a/server/src/main/java/org/elasticsearch/search/runtime/AbstractGeoPointScriptFieldQuery.java b/server/src/main/java/org/elasticsearch/search/runtime/AbstractGeoPointScriptFieldQuery.java index d2fb993ec93e..933c7237966e 100644 --- a/server/src/main/java/org/elasticsearch/search/runtime/AbstractGeoPointScriptFieldQuery.java +++ b/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); } diff --git a/server/src/main/java/org/elasticsearch/search/runtime/GeoPointScriptFieldExistsQuery.java b/server/src/main/java/org/elasticsearch/search/runtime/GeoPointScriptFieldExistsQuery.java index 02497ede6c83..cdcff1d899b2 100644 --- a/server/src/main/java/org/elasticsearch/search/runtime/GeoPointScriptFieldExistsQuery.java +++ b/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; } diff --git a/server/src/main/java/org/elasticsearch/search/runtime/GeoPointScriptFieldGeoShapeQuery.java b/server/src/main/java/org/elasticsearch/search/runtime/GeoPointScriptFieldGeoShapeQuery.java index 68b016c57d34..30194730d24a 100644 --- a/server/src/main/java/org/elasticsearch/search/runtime/GeoPointScriptFieldGeoShapeQuery.java +++ b/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) { diff --git a/server/src/test/java/org/elasticsearch/index/mapper/AbstractScriptFieldTypeTestCase.java b/server/src/test/java/org/elasticsearch/index/mapper/AbstractScriptFieldTypeTestCase.java index e8c8aa7ae793..3e0f860c2f6c 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/AbstractScriptFieldTypeTestCase.java +++ b/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; } diff --git a/server/src/test/java/org/elasticsearch/index/mapper/GeoPointScriptFieldTypeTests.java b/server/src/test/java/org/elasticsearch/index/mapper/GeoPointScriptFieldTypeTests.java index 26c2f5a430ed..0ae0796e2d28 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/GeoPointScriptFieldTypeTests.java +++ b/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)) { diff --git a/server/src/test/java/org/elasticsearch/index/mapper/GeoPointScriptMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/GeoPointScriptMapperTests.java index 7ef00c9592c5..70bbf0672daa 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/GeoPointScriptMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/GeoPointScriptMapperTests.java @@ -18,7 +18,7 @@ import java.util.function.Consumer; public class GeoPointScriptMapperTests extends MapperScriptTestCase { - private static GeoPointFieldScript.Factory factory(Consumer executor) { + private static GeoPointFieldScript.Factory factory(Consumer executor) { return new GeoPointFieldScript.Factory() { @Override public GeoPointFieldScript.LeafFactory newFactory(String fieldName, Map params, SearchLookup searchLookup) { @@ -28,7 +28,7 @@ public class GeoPointScriptMapperTests extends MapperScriptTestCase 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 ", fields[0].toString()); - assertEquals("LatLonDocValuesField ", fields[1].toString()); - assertEquals("LatLonPoint ", fields[2].toString()); - assertEquals("LatLonDocValuesField ", fields[3].toString()); + assertEquals("LatLonPoint ", fields[0].toString()); + assertEquals("LatLonDocValuesField ", fields[1].toString()); + assertEquals("LatLonPoint ", fields[2].toString()); + assertEquals("LatLonDocValuesField ", fields[3].toString()); } @Override protected void assertDocValuesDisabled(IndexableField[] fields) { assertEquals(1, fields.length); - assertEquals("LatLonPoint ", fields[0].toString()); + assertEquals("LatLonPoint ", fields[0].toString()); } @Override protected void assertIndexDisabled(IndexableField[] fields) { assertEquals(1, fields.length); - assertEquals("LatLonDocValuesField ", fields[0].toString()); + assertEquals("LatLonDocValuesField ", fields[0].toString()); } } diff --git a/server/src/test/java/org/elasticsearch/search/runtime/GeoPointScriptFieldDistanceFeatureQueryTests.java b/server/src/test/java/org/elasticsearch/search/runtime/GeoPointScriptFieldDistanceFeatureQueryTests.java index d06b2c8ae7ff..b5755dedfe8d 100644 --- a/server/src/test/java/org/elasticsearch/search/runtime/GeoPointScriptFieldDistanceFeatureQueryTests.java +++ b/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 leafFactory = ctx -> new GeoPointFieldScript( + Function 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, diff --git a/server/src/test/java/org/elasticsearch/search/runtime/GeoPointScriptFieldExistsQueryTests.java b/server/src/test/java/org/elasticsearch/search/runtime/GeoPointScriptFieldExistsQueryTests.java index 4099fc9df767..eb20e77aa21a 100644 --- a/server/src/test/java/org/elasticsearch/search/runtime/GeoPointScriptFieldExistsQueryTests.java +++ b/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 diff --git a/server/src/test/java/org/elasticsearch/search/runtime/GeoPointScriptFieldGeoShapeQueryTests.java b/server/src/test/java/org/elasticsearch/search/runtime/GeoPointScriptFieldGeoShapeQueryTests.java index 2dd8586bb92d..866fd8925cf9 100644 --- a/server/src/test/java/org/elasticsearch/search/runtime/GeoPointScriptFieldGeoShapeQueryTests.java +++ b/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