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

* Use FallbackSyntheticSourceBlockLoader for shape and geo_shape (elastic#124927)

(cherry picked from commit 033d28e)

# Conflicts:
#	server/src/test/java/org/elasticsearch/index/mapper/blockloader/BooleanFieldBlockLoaderTests.java
#	server/src/test/java/org/elasticsearch/index/mapper/blockloader/DateFieldBlockLoaderTests.java
#	server/src/test/java/org/elasticsearch/index/mapper/blockloader/KeywordFieldBlockLoaderTests.java
#	test/framework/src/main/java/org/elasticsearch/index/mapper/BlockLoaderTestCase.java
#	test/framework/src/main/java/org/elasticsearch/index/mapper/NumberFieldBlockLoaderTestCase.java

* fix merge

* fix

* fix more

* fix more

* properly sync BlockLoaderTestCase

* iter

* iter

* [CI] Auto commit changes from spotless

* f

* f

---------

Co-authored-by: elasticsearchmachine <[email protected]>

Author: lkts

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;