[8.x] Use FallbackSyntheticSourceBlockLoader for shape and geo_shape (#124927)

docs/changelog/124927.yaml

@@ -0,0 +1,5 @@
+pr: 124927
+summary: Use `FallbackSyntheticSourceBlockLoader` for `shape` and `geo_shape`
+area: Mapping
+type: enhancement
+issues: []

modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/extras/ScaledFloatFieldMapper.java

@@ -342,7 +342,7 @@ public Builder builder(BlockFactory factory, int expectedCount) {
         private FallbackSyntheticSourceBlockLoader.Reader<?> fallbackSyntheticSourceBlockLoaderReader() {
             var nullValueAdjusted = nullValue != null ? adjustSourceValue(nullValue, scalingFactor) : null;
 
-            return new FallbackSyntheticSourceBlockLoader.ReaderWithNullValueSupport<Double>(nullValue) {
+            return new FallbackSyntheticSourceBlockLoader.SingleValueReader<Double>(nullValue) {
                 @Override
                 public void convertValue(Object value, List<Double> accumulator) {
                     if (coerce && value.equals("")) {

modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/extras/ScaledFloatFieldBlockLoaderTests.java

@@ -18,8 +18,8 @@
 import java.util.Map;
 
 public class ScaledFloatFieldBlockLoaderTests extends NumberFieldBlockLoaderTestCase<Double> {
-    public ScaledFloatFieldBlockLoaderTests() {
-        super(FieldType.SCALED_FLOAT);
+    public ScaledFloatFieldBlockLoaderTests(Params params) {
+        super(FieldType.SCALED_FLOAT, params);
     }
 
     @Override

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

@@ -9,6 +9,7 @@
 package org.elasticsearch.index.mapper;
 
 import org.apache.lucene.search.Query;
+import org.apache.lucene.util.BytesRef;
 import org.elasticsearch.common.CheckedBiConsumer;
 import org.elasticsearch.common.Explicit;
 import org.elasticsearch.common.geo.GeometryFormatterFactory;
@@ -67,12 +68,16 @@ public abstract void parse(XContentParser parser, CheckedConsumer<T, IOException
 
         private void fetchFromSource(Object sourceMap, Consumer<T> consumer) {
             try (XContentParser parser = wrapObject(sourceMap)) {
-                parse(parser, v -> consumer.accept(normalizeFromSource(v)), NoopMalformedValueHandler.INSTANCE);
+                parseFromSource(parser, consumer);
             } catch (IOException e) {
                 throw new UncheckedIOException(e);
             }
         }
 
+        private void parseFromSource(XContentParser parser, Consumer<T> consumer) throws IOException {
+            parse(parser, v -> consumer.accept(normalizeFromSource(v)), NoopMalformedValueHandler.INSTANCE);
+        }
+
         /**
          * Normalize a geometry when reading from source. When reading from source we can skip
          * some expensive steps as the geometry has already been indexed.
@@ -187,6 +192,80 @@ protected BlockLoader blockLoaderFromSource(BlockLoaderContext blContext) {
         }
 
         protected abstract Object nullValueAsSource(T nullValue);
+
+        protected BlockLoader blockLoaderFromFallbackSyntheticSource(BlockLoaderContext blContext) {
+            return new FallbackSyntheticSourceBlockLoader(new GeometriesFallbackSyntheticSourceReader(), name()) {
+                @Override
+                public Builder builder(BlockFactory factory, int expectedCount) {
+                    return factory.bytesRefs(expectedCount);
+                }
+            };
+        }
+
+        private class GeometriesFallbackSyntheticSourceReader implements FallbackSyntheticSourceBlockLoader.Reader<BytesRef> {
+            private final Function<List<T>, List<Object>> formatter;
+
+            private GeometriesFallbackSyntheticSourceReader() {
+                this.formatter = getFormatter(GeometryFormatterFactory.WKB);
+            }
+
+            @Override
+            public void convertValue(Object value, List<BytesRef> accumulator) {
+                final List<T> values = new ArrayList<>();
+
+                geometryParser.fetchFromSource(value, v -> {
+                    if (v != null) {
+                        values.add(v);
+                    } else if (nullValue != null) {
+                        values.add(nullValue);
+                    }
+                });
+                var formatted = formatter.apply(values);
+
+                for (var formattedValue : formatted) {
+                    if (formattedValue instanceof byte[] wkb) {
+                        accumulator.add(new BytesRef(wkb));
+                    } else {
+                        throw new IllegalArgumentException(
+                            "Unsupported source type for spatial geometry: " + formattedValue.getClass().getSimpleName()
+                        );
+                    }
+                }
+            }
+
+            @Override
+            public void parse(XContentParser parser, List<BytesRef> accumulator) throws IOException {
+                final List<T> values = new ArrayList<>();
+
+                geometryParser.parseFromSource(parser, v -> {
+                    if (v != null) {
+                        values.add(v);
+                    } else if (nullValue != null) {
+                        values.add(nullValue);
+                    }
+                });
+                var formatted = formatter.apply(values);
+
+                for (var formattedValue : formatted) {
+                    if (formattedValue instanceof byte[] wkb) {
+                        accumulator.add(new BytesRef(wkb));
+                    } else {
+                        throw new IllegalArgumentException(
+                            "Unsupported source type for spatial geometry: " + formattedValue.getClass().getSimpleName()
+                        );
+                    }
+                }
+            }
+
+            @Override
+            public void writeToBlock(List<BytesRef> values, BlockLoader.Builder blockBuilder) {
+                var bytesRefBuilder = (BlockLoader.BytesRefBuilder) blockBuilder;
+
+                for (var value : values) {
+                    bytesRefBuilder.appendBytesRef(value);
+                }
+            }
+        }
     }
 
     private final Explicit<Boolean> ignoreMalformed;

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

@@ -337,7 +337,7 @@ public Builder builder(BlockFactory factory, int expectedCount) {
         }
 
         private FallbackSyntheticSourceBlockLoader.Reader<?> fallbackSyntheticSourceBlockLoaderReader() {
-            return new FallbackSyntheticSourceBlockLoader.ReaderWithNullValueSupport<Boolean>(nullValue) {
+            return new FallbackSyntheticSourceBlockLoader.SingleValueReader<Boolean>(nullValue) {
                 @Override
                 public void convertValue(Object value, List<Boolean> accumulator) {
                     try {
@@ -366,10 +366,10 @@ protected void parseNonNullValue(XContentParser parser, List<Boolean> accumulato
 
                 @Override
                 public void writeToBlock(List<Boolean> values, BlockLoader.Builder blockBuilder) {
-                    var longBuilder = (BlockLoader.BooleanBuilder) blockBuilder;
+                    var booleanBuilder = (BlockLoader.BooleanBuilder) blockBuilder;
 
                     for (var value : values) {
-                        longBuilder.appendBoolean(value);
+                        booleanBuilder.appendBoolean(value);
                     }
                 }
             };

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

@@ -871,7 +871,7 @@ public Builder builder(BlockFactory factory, int expectedCount) {
         private FallbackSyntheticSourceBlockLoader.Reader<?> fallbackSyntheticSourceBlockLoaderReader() {
             Function<String, Long> dateParser = this::parse;
 
-            return new FallbackSyntheticSourceBlockLoader.ReaderWithNullValueSupport<Long>(nullValue) {
+            return new FallbackSyntheticSourceBlockLoader.SingleValueReader<Long>(nullValue) {
                 @Override
                 public void convertValue(Object value, List<Long> accumulator) {
                     try {

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

@@ -235,13 +235,6 @@ private void parseFieldFromParent(IgnoredSourceFieldMapper.NameValue nameValue,
         }
 
         private void parseWithReader(XContentParser parser, List<T> blockValues) throws IOException {
-            if (parser.currentToken() == XContentParser.Token.START_ARRAY) {
-                while (parser.nextToken() != XContentParser.Token.END_ARRAY) {
-                    reader.parse(parser, blockValues);
-                }
-                return;
-            }
-
             reader.parse(parser, blockValues);
         }
 
@@ -274,10 +267,15 @@ public interface Reader<T> {
         void writeToBlock(List<T> values, Builder blockBuilder);
     }
 
-    public abstract static class ReaderWithNullValueSupport<T> implements Reader<T> {
+    /**
+     * Reader for field types that don't parse arrays (arrays are always treated as multiple values)
+     * as opposed to field types that treat arrays as special cases (for example point).
+     * @param <T>
+     */
+    public abstract static class SingleValueReader<T> implements Reader<T> {
         private final Object nullValue;
 
-        public ReaderWithNullValueSupport(Object nullValue) {
+        public SingleValueReader(Object nullValue) {
             this.nullValue = nullValue;
         }
 
@@ -289,6 +287,18 @@ public void parse(XContentParser parser, List<T> accumulator) throws IOException
                 }
                 return;
             }
+            if (parser.currentToken() == XContentParser.Token.START_ARRAY) {
+                while (parser.nextToken() != XContentParser.Token.END_ARRAY) {
+                    if (parser.currentToken() == XContentParser.Token.VALUE_NULL) {
+                        if (nullValue != null) {
+                            convertValue(nullValue, accumulator);
+                        }
+                    } else {
+                        parseNonNullValue(parser, accumulator);
+                    }
+                }
+                return;
+            }
 
             parseNonNullValue(parser, accumulator);
         }

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

@@ -669,7 +669,7 @@ public Builder builder(BlockFactory factory, int expectedCount) {
 
         private FallbackSyntheticSourceBlockLoader.Reader<?> fallbackSyntheticSourceBlockLoaderReader() {
             var nullValueBytes = nullValue != null ? new BytesRef(nullValue) : null;
-            return new FallbackSyntheticSourceBlockLoader.ReaderWithNullValueSupport<BytesRef>(nullValueBytes) {
+            return new FallbackSyntheticSourceBlockLoader.SingleValueReader<BytesRef>(nullValueBytes) {
                 @Override
                 public void convertValue(Object value, List<BytesRef> accumulator) {
                     String stringValue = ((BytesRef) value).utf8ToString();

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

@@ -1738,8 +1738,7 @@ public Builder builder(BlockFactory factory, int expectedCount) {
             };
         }
 
-        abstract static class NumberFallbackSyntheticSourceReader extends FallbackSyntheticSourceBlockLoader.ReaderWithNullValueSupport<
-            Number> {
+        abstract static class NumberFallbackSyntheticSourceReader extends FallbackSyntheticSourceBlockLoader.SingleValueReader<Number> {
             private final NumberType type;
             private final Number nullValue;
             private final boolean coerce;

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

@@ -17,13 +17,13 @@
 import java.util.Objects;
 
 public class BooleanFieldBlockLoaderTests extends BlockLoaderTestCase {
-    public BooleanFieldBlockLoaderTests() {
-        super(FieldType.BOOLEAN);
+    public BooleanFieldBlockLoaderTests(Params params) {
+        super(FieldType.BOOLEAN.toString(), params);
     }
 
     @Override
     @SuppressWarnings("unchecked")
-    protected Object expected(Map<String, Object> fieldMapping, Object value, boolean syntheticSource) {
+    protected Object expected(Map<String, Object> fieldMapping, Object value) {
         var rawNullValue = fieldMapping.get("null_value");
 
         Boolean nullValue;

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

@@ -15,8 +15,8 @@
 import java.util.Map;
 
 public class ByteFieldBlockLoaderTests extends NumberFieldBlockLoaderTestCase<Integer> {
-    public ByteFieldBlockLoaderTests() {
-        super(FieldType.BYTE);
+    public ByteFieldBlockLoaderTests(Params params) {
+        super(FieldType.BYTE, params);
     }
 
     @Override

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

@@ -23,13 +23,13 @@
 import java.util.Objects;
 
 public class DateFieldBlockLoaderTests extends BlockLoaderTestCase {
-    public DateFieldBlockLoaderTests() {
-        super(FieldType.DATE);
+    public DateFieldBlockLoaderTests(Params params) {
+        super(FieldType.DATE.toString(), params);
     }
 
     @Override
     @SuppressWarnings("unchecked")
-    protected Object expected(Map<String, Object> fieldMapping, Object value, boolean syntheticSource) {
+    protected Object expected(Map<String, Object> fieldMapping, Object value) {
         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/DoubleFieldBlockLoaderTests.java

@@ -15,8 +15,8 @@
 import java.util.Map;
 
 public class DoubleFieldBlockLoaderTests extends NumberFieldBlockLoaderTestCase<Double> {
-    public DoubleFieldBlockLoaderTests() {
-        super(FieldType.DOUBLE);
+    public DoubleFieldBlockLoaderTests(Params params) {
+        super(FieldType.DOUBLE, params);
     }
 
     @Override

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

@@ -15,8 +15,8 @@
 import java.util.Map;
 
 public class FloatFieldBlockLoaderTests extends NumberFieldBlockLoaderTestCase<Double> {
-    public FloatFieldBlockLoaderTests() {
-        super(FieldType.FLOAT);
+    public FloatFieldBlockLoaderTests(Params params) {
+        super(FieldType.FLOAT, params);
     }
 
     @Override

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

@@ -16,8 +16,8 @@
 import java.util.Map;
 
 public class HalfFloatFieldBlockLoaderTests extends NumberFieldBlockLoaderTestCase<Double> {
-    public HalfFloatFieldBlockLoaderTests() {
-        super(FieldType.HALF_FLOAT);
+    public HalfFloatFieldBlockLoaderTests(Params params) {
+        super(FieldType.HALF_FLOAT, params);
     }
 
     @Override

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

@@ -15,8 +15,8 @@
 import java.util.Map;
 
 public class IntegerFieldBlockLoaderTests extends NumberFieldBlockLoaderTestCase<Integer> {
-    public IntegerFieldBlockLoaderTests() {
-        super(FieldType.INTEGER);
+    public IntegerFieldBlockLoaderTests(Params params) {
+        super(FieldType.INTEGER, params);
     }
 
     @Override

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

@@ -20,13 +20,13 @@
 import java.util.stream.Stream;
 
 public class KeywordFieldBlockLoaderTests extends BlockLoaderTestCase {
-    public KeywordFieldBlockLoaderTests() {
-        super(FieldType.KEYWORD);
+    public KeywordFieldBlockLoaderTests(Params params) {
+        super(FieldType.KEYWORD.toString(), params);
     }
 
     @SuppressWarnings("unchecked")
     @Override
-    protected Object expected(Map<String, Object> fieldMapping, Object value, boolean syntheticSource) {
+    protected Object expected(Map<String, Object> fieldMapping, Object value) {
         var nullValue = (String) fieldMapping.get("null_value");
 
         var ignoreAbove = fieldMapping.get("ignore_above") == null
@@ -44,7 +44,8 @@ protected Object expected(Map<String, Object> fieldMapping, Object value, boolea
         Function<Stream<String>, Stream<BytesRef>> convertValues = s -> s.map(v -> convert(v, nullValue, ignoreAbove))
             .filter(Objects::nonNull);
 
-        if ((boolean) fieldMapping.getOrDefault("doc_values", false)) {
+        boolean hasDocValues = hasDocValues(fieldMapping, true);
+        if (hasDocValues) {
             // Sorted and no duplicates
             var resultList = convertValues.andThen(Stream::distinct)
                 .andThen(Stream::sorted)

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

@@ -15,8 +15,8 @@
 import java.util.Map;
 
 public class LongFieldBlockLoaderTests extends NumberFieldBlockLoaderTestCase<Long> {
-    public LongFieldBlockLoaderTests() {
-        super(FieldType.LONG);
+    public LongFieldBlockLoaderTests(Params params) {
+        super(FieldType.LONG, params);
     }
 
     @Override

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

@@ -15,8 +15,8 @@
 import java.util.Map;
 
 public class ShortFieldBlockLoaderTests extends NumberFieldBlockLoaderTestCase<Integer> {
-    public ShortFieldBlockLoaderTests() {
-        super(FieldType.SHORT);
+    public ShortFieldBlockLoaderTests(Params params) {
+        super(FieldType.SHORT, params);
     }
 
     @Override