Use FallbackSyntheticSourceBlockLoader for point and geo_point

docs/changelog/125816.yaml

@@ -0,0 +1,5 @@
+pr: 125816
+summary: Use `FallbackSyntheticSourceBlockLoader` for point and `geo_point`
+area: Mapping
+type: enhancement
+issues: []

server/src/main/java/org/elasticsearch/index/mapper/AbstractPointGeometryFieldMapper.java

@@ -22,8 +22,6 @@
 import java.util.function.Function;
 import java.util.function.Supplier;
 
-import static org.elasticsearch.index.mapper.MappedFieldType.FieldExtractPreference.DOC_VALUES;
-
 /** Base class for spatial fields that only support indexing points */
 public abstract class AbstractPointGeometryFieldMapper<T> extends AbstractGeometryFieldMapper<T> {
 
@@ -148,7 +146,6 @@ protected void parseAndConsumeFromObject(
     }
 
     public abstract static class AbstractPointFieldType<T extends SpatialPoint> extends AbstractGeometryFieldType<T> {
-
         protected AbstractPointFieldType(
             String name,
             boolean indexed,
@@ -165,13 +162,5 @@ protected AbstractPointFieldType(
         protected Object nullValueAsSource(T nullValue) {
             return nullValue == null ? null : nullValue.toWKT();
         }
-
-        @Override
-        public BlockLoader blockLoader(BlockLoaderContext blContext) {
-            if (blContext.fieldExtractPreference() == DOC_VALUES && hasDocValues()) {
-                return new BlockDocValuesReader.LongsBlockLoader(name());
-            }
-            return blockLoaderFromSource(blContext);
-        }
     }
 }

server/src/main/java/org/elasticsearch/index/mapper/GeoPointFieldMapper.java

@@ -67,6 +67,8 @@
 import java.util.Set;
 import java.util.function.Function;
 
+import static org.elasticsearch.index.mapper.MappedFieldType.FieldExtractPreference.DOC_VALUES;
+
 /**
  * Field Mapper for geo_point types.
  *
@@ -224,7 +226,8 @@ public FieldMapper build(MapperBuilderContext context) {
                 scriptValues(),
                 meta.get(),
                 metric.get(),
-                indexMode
+                indexMode,
+                context.isSourceSynthetic()
             );
             hasScript = script.get() != null;
             onScriptError = onScriptErrorParam.get();
@@ -370,6 +373,7 @@ public static class GeoPointFieldType extends AbstractPointFieldType<GeoPoint> i
 
         private final FieldValues<GeoPoint> scriptValues;
         private final IndexMode indexMode;
+        private final boolean isSyntheticSource;
 
         private GeoPointFieldType(
             String name,
@@ -381,17 +385,19 @@ private GeoPointFieldType(
             FieldValues<GeoPoint> scriptValues,
             Map<String, String> meta,
             TimeSeriesParams.MetricType metricType,
-            IndexMode indexMode
+            IndexMode indexMode,
+            boolean isSyntheticSource
         ) {
             super(name, indexed, stored, hasDocValues, parser, nullValue, meta);
             this.scriptValues = scriptValues;
             this.metricType = metricType;
             this.indexMode = indexMode;
+            this.isSyntheticSource = isSyntheticSource;
         }
 
         // only used in test
         public GeoPointFieldType(String name, TimeSeriesParams.MetricType metricType, IndexMode indexMode) {
-            this(name, true, false, true, null, null, null, Collections.emptyMap(), metricType, indexMode);
+            this(name, true, false, true, null, null, null, Collections.emptyMap(), metricType, indexMode, false);
         }
 
         // only used in test
@@ -524,6 +530,28 @@ public Query distanceFeatureQuery(Object origin, String pivot, SearchExecutionCo
         public TimeSeriesParams.MetricType getMetricType() {
             return metricType;
         }
+
+        @Override
+        public BlockLoader blockLoader(BlockLoaderContext blContext) {
+            if (blContext.fieldExtractPreference() == DOC_VALUES && hasDocValues()) {
+                return new BlockDocValuesReader.LongsBlockLoader(name());
+            }
+
+            // There are two scenarios possible once we arrive here:
+            //
+            // * Stored source - we'll just use blockLoaderFromSource
+            // * Synthetic source. However, because of the fieldExtractPreference() check above it is still possible that doc_values are
+            // present here.
+            // So we have two subcases:
+            // - doc_values are enabled - _ignored_source field does not exist since we have doc_values. We will use
+            // blockLoaderFromSource which reads "native" synthetic source.
+            // - doc_values are disabled - we know that _ignored_source field is present and use a special block loader.
+            if (isSyntheticSource && hasDocValues() == false) {
+                return blockLoaderFromFallbackSyntheticSource(blContext);
+            }
+
+            return blockLoaderFromSource(blContext);
+        }
     }
 
     /** GeoPoint parser implementation */

server/src/test/java/org/elasticsearch/index/mapper/blockloader/BooleanFieldBlockLoaderTests.java

@@ -23,7 +23,7 @@ public BooleanFieldBlockLoaderTests(Params params) {
 
     @Override
     @SuppressWarnings("unchecked")
-    protected Object expected(Map<String, Object> fieldMapping, Object value) {
+    protected Object expected(Map<String, Object> fieldMapping, Object value, TestContext testContext) {
         var nullValue = switch (fieldMapping.get("null_value")) {
             case Boolean b -> b;
             case String s -> Boolean.parseBoolean(s);

server/src/test/java/org/elasticsearch/index/mapper/blockloader/DateFieldBlockLoaderTests.java

@@ -29,7 +29,7 @@ public DateFieldBlockLoaderTests(Params params) {
 
     @Override
     @SuppressWarnings("unchecked")
-    protected Object expected(Map<String, Object> fieldMapping, Object value) {
+    protected Object expected(Map<String, Object> fieldMapping, Object value, TestContext testContext) {
         var format = (String) fieldMapping.get("format");
         var nullValue = fieldMapping.get("null_value") != null ? format(fieldMapping.get("null_value"), format) : null;
 

server/src/test/java/org/elasticsearch/index/mapper/blockloader/GeoPointFieldBlockLoaderTests.java

@@ -0,0 +1,207 @@
+/*
+ * 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", the "GNU Affero General Public License v3.0 only", 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", the "GNU Affero General Public
+ * License v3.0 only", or the "Server Side Public License, v 1".
+ */
+
+package org.elasticsearch.index.mapper.blockloader;
+
+import org.apache.lucene.util.BytesRef;
+import org.elasticsearch.common.geo.GeoPoint;
+import org.elasticsearch.geometry.Point;
+import org.elasticsearch.geometry.utils.WellKnownBinary;
+import org.elasticsearch.index.mapper.BlockLoaderTestCase;
+import org.elasticsearch.index.mapper.MappedFieldType;
+
+import java.nio.ByteOrder;
+import java.util.ArrayList;
+import java.util.Comparator;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+
+public class GeoPointFieldBlockLoaderTests extends BlockLoaderTestCase {
+    public GeoPointFieldBlockLoaderTests(BlockLoaderTestCase.Params params) {
+        super("geo_point", params);
+    }
+
+    @Override
+    @SuppressWarnings("unchecked")
+    protected Object expected(Map<String, Object> fieldMapping, Object value, TestContext testContext) {
+        var extractedFieldValues = (ExtractedFieldValues) value;
+        var values = extractedFieldValues.values();
+
+        var nullValue = switch (fieldMapping.get("null_value")) {
+            case String s -> convert(s, null);
+            case null -> null;
+            default -> throw new IllegalStateException("Unexpected null_value format");
+        };
+
+        if (params.preference() == MappedFieldType.FieldExtractPreference.DOC_VALUES && hasDocValues(fieldMapping, true)) {
+            if (values instanceof List<?> == false) {
+                var point = convert(values, nullValue);
+                return point != null ? point.getEncoded() : null;
+            }
+
+            var resultList = ((List<Object>) values).stream()
+                .map(v -> convert(v, nullValue))
+                .filter(Objects::nonNull)
+                .map(GeoPoint::getEncoded)
+                .sorted()
+                .toList();
+            return maybeFoldList(resultList);
+        }
+
+        if (params.syntheticSource() == false) {
+            return exactValuesFromSource(values, nullValue);
+        }
+
+        // Usually implementation of block loader from source adjusts values read from source
+        // so that they look the same as doc_values would (like reducing precision).
+        // geo_point does not do that and because of that we need to handle all these cases below.
+        // If we are reading from stored source or fallback synthetic source we get the same exact data as source.
+        // But if we are using "normal" synthetic source we get lesser precision data from doc_values.
+        // That is unless "synthetic_source_keep" forces fallback synthetic source again.
+
+        if (testContext.forceFallbackSyntheticSource()) {
+            return exactValuesFromSource(values, nullValue);
+        }
+
+        String syntheticSourceKeep = (String) fieldMapping.getOrDefault("synthetic_source_keep", "none");
+        if (syntheticSourceKeep.equals("all")) {
+            return exactValuesFromSource(values, nullValue);
+        }
+        if (syntheticSourceKeep.equals("arrays") && extractedFieldValues.documentHasObjectArrays()) {
+            return exactValuesFromSource(values, nullValue);
+        }
+
+        // synthetic source and doc_values are present
+        if (hasDocValues(fieldMapping, true)) {
+            if (values instanceof List<?> == false) {
+                return toWKB(normalize(convert(values, nullValue)));
+            }
+
+            var resultList = ((List<Object>) values).stream()
+                .map(v -> convert(v, nullValue))
+                .filter(Objects::nonNull)
+                .sorted(Comparator.comparingLong(GeoPoint::getEncoded))
+                .map(p -> toWKB(normalize(p)))
+                .toList();
+            return maybeFoldList(resultList);
+        }
+
+        // synthetic source but no doc_values so using fallback synthetic source
+        return exactValuesFromSource(values, nullValue);
+    }
+
+    @SuppressWarnings("unchecked")
+    private Object exactValuesFromSource(Object value, GeoPoint nullValue) {
+        if (value instanceof List<?> == false) {
+            return toWKB(convert(value, nullValue));
+        }
+
+        var resultList = ((List<Object>) value).stream().map(v -> convert(v, nullValue)).filter(Objects::nonNull).map(this::toWKB).toList();
+        return maybeFoldList(resultList);
+    }
+
+    private record ExtractedFieldValues(Object values, boolean documentHasObjectArrays) {}
+
+    @Override
+    protected Object getFieldValue(Map<String, Object> document, String fieldName) {
+        var extracted = new ArrayList<>();
+        var documentHasObjectArrays = processLevel(document, fieldName, extracted, false);
+
+        if (extracted.size() == 1) {
+            return new ExtractedFieldValues(extracted.get(0), documentHasObjectArrays);
+        }
+
+        return new ExtractedFieldValues(extracted, documentHasObjectArrays);
+    }
+
+    @SuppressWarnings("unchecked")
+    private boolean processLevel(Map<String, Object> level, String field, ArrayList<Object> extracted, boolean documentHasObjectArrays) {
+        if (field.contains(".") == false) {
+            var value = level.get(field);
+            processLeafLevel(value, extracted);
+            return documentHasObjectArrays;
+        }
+
+        var nameInLevel = field.split("\\.")[0];
+        var entry = level.get(nameInLevel);
+        if (entry instanceof Map<?, ?> m) {
+            return processLevel((Map<String, Object>) m, field.substring(field.indexOf('.') + 1), extracted, documentHasObjectArrays);
+        }
+        if (entry instanceof List<?> l) {
+            for (var object : l) {
+                processLevel((Map<String, Object>) object, field.substring(field.indexOf('.') + 1), extracted, true);
+            }
+            return true;
+        }
+
+        assert false : "unexpected document structure";
+        return false;
+    }
+
+    private void processLeafLevel(Object value, ArrayList<Object> extracted) {
+        if (value instanceof List<?> l) {
+            if (l.size() > 0 && l.get(0) instanceof Double) {
+                // this must be a single point in array form
+                // we'll put it into a different form here to make our lives a bit easier while implementing `expected`
+                extracted.add(Map.of("type", "point", "coordinates", l));
+            } else {
+                // this is actually an array of points but there could still be points in array form inside
+                for (var arrayValue : l) {
+                    processLeafLevel(arrayValue, extracted);
+                }
+            }
+        } else {
+            extracted.add(value);
+        }
+    }
+
+    @SuppressWarnings("unchecked")
+    private GeoPoint convert(Object value, GeoPoint nullValue) {
+        if (value == null) {
+            return nullValue;
+        }
+
+        if (value instanceof String s) {
+            try {
+                return new GeoPoint(s);
+            } catch (Exception e) {
+                return null;
+            }
+        }
+
+        if (value instanceof Map<?, ?> m) {
+            if (m.get("type") != null) {
+                var coordinates = (List<Double>) m.get("coordinates");
+                // Order is GeoJSON is lon,lat
+                return new GeoPoint(coordinates.get(1), coordinates.get(0));
+            } else {
+                return new GeoPoint((Double) m.get("lat"), (Double) m.get("lon"));
+            }
+        }
+
+        // Malformed values are excluded
+        return null;
+    }
+
+    private GeoPoint normalize(GeoPoint point) {
+        if (point == null) {
+            return null;
+        }
+        return point.resetFromEncoded(point.getEncoded());
+    }
+
+    private BytesRef toWKB(GeoPoint point) {
+        if (point == null) {
+            return null;
+        }
+
+        return new BytesRef(WellKnownBinary.toWKB(new Point(point.getX(), point.getY()), ByteOrder.LITTLE_ENDIAN));
+    }
+}

server/src/test/java/org/elasticsearch/index/mapper/blockloader/KeywordFieldBlockLoaderTests.java

@@ -27,7 +27,7 @@ public KeywordFieldBlockLoaderTests(Params params) {
 
     @SuppressWarnings("unchecked")
     @Override
-    protected Object expected(Map<String, Object> fieldMapping, Object value) {
+    protected Object expected(Map<String, Object> fieldMapping, Object value, TestContext testContext) {
         var nullValue = (String) fieldMapping.get("null_value");
 
         var ignoreAbove = fieldMapping.get("ignore_above") == null