Improve block loader for source only runtime fields of type double. (#134629)

martijnvg · web-flow · commit 96402aa7a168 · 2025-09-12T12:16:38.000-04:00
By using the FallbackSyntheticSourceBlockLoader instead of generic DoubleScriptBlockLoader. Relates to #134121
diff --git a/docs/changelog/134629.yaml b/docs/changelog/134629.yaml
@@ -0,0 +1,5 @@
+pr: 134629
+summary: Improve block loader for source only runtime fields of type double
+area: Mapping
+type: enhancement
+issues: []
diff --git a/server/src/main/java/org/elasticsearch/index/mapper/AbstractScriptFieldType.java b/server/src/main/java/org/elasticsearch/index/mapper/AbstractScriptFieldType.java
@@ -37,6 +37,8 @@
 import java.util.Locale;
 import java.util.Map;
 import java.util.Objects;
+import java.util.function.BiConsumer;
+import java.util.function.BiFunction;
 import java.util.function.Function;
 
 import static org.elasticsearch.search.SearchService.ALLOW_EXPENSIVE_QUERIES;
@@ -203,6 +205,45 @@ protected final LeafFactory leafFactory(SearchLookup searchLookup) {
         }
     }
 
+    /**
+     * Returns synthetic source fallback block loader if source mode is synthetic, runtime field is source only and field is only mapped
+     * as a runtime field.
+     */
+    protected final FallbackSyntheticSourceBlockLoader fallbackSyntheticSourceBlockLoader(
+        BlockLoaderContext blContext,
+        NumberFieldMapper.NumberType numberType,
+        BiFunction<BlockLoader.BlockFactory, Integer, BlockLoader.Builder> builderSupplier,
+        BiConsumer<List<Number>, BlockLoader.Builder> writeToBlock
+    ) {
+        var indexSettings = blContext.indexSettings();
+        // A runtime and normal field can share the same name.
+        // In that case there is no ignored source entry, and so we need to fail back to LongScriptBlockLoader.
+        // We could optimize this, but at this stage feels like a rare scenario.
+        if (isParsedFromSource
+            && indexSettings.getIndexMappingSourceMode() == SourceFieldMapper.Mode.SYNTHETIC
+            && blContext.lookup().onlyMappedAsRuntimeField(name())) {
+            var reader = new NumberFieldMapper.NumberType.NumberFallbackSyntheticSourceReader(numberType, null, true) {
+                @Override
+                public void writeToBlock(List<Number> values, BlockLoader.Builder blockBuilder) {
+                    writeToBlock.accept(values, blockBuilder);
+                }
+            };
+
+            return new FallbackSyntheticSourceBlockLoader(
+                reader,
+                name(),
+                IgnoredSourceFieldMapper.ignoredSourceFormat(indexSettings.getIndexVersionCreated())
+            ) {
+                @Override
+                public Builder builder(BlockFactory factory, int expectedCount) {
+                    return builderSupplier.apply(factory, expectedCount);
+                }
+            };
+        } else {
+            return null;
+        }
+    }
+
     /**
      * Create a script leaf factory for queries.
      */
diff --git a/server/src/main/java/org/elasticsearch/index/mapper/DoubleScriptFieldType.java b/server/src/main/java/org/elasticsearch/index/mapper/DoubleScriptFieldType.java
@@ -109,7 +109,22 @@ public DocValueFormat docValueFormat(String format, ZoneId timeZone) {
 
     @Override
     public BlockLoader blockLoader(BlockLoaderContext blContext) {
-        return new DoubleScriptBlockDocValuesReader.DoubleScriptBlockLoader(leafFactory(blContext.lookup()));
+        var fallbackSyntheticSourceBlockLoader = fallbackSyntheticSourceBlockLoader(
+            blContext,
+            NumberType.DOUBLE,
+            BlockLoader.BlockFactory::doubles,
+            (values, blockBuilder) -> {
+                var builder = (BlockLoader.DoubleBuilder) blockBuilder;
+                for (var value : values) {
+                    builder.appendDouble(value.doubleValue());
+                }
+            }
+        );
+        if (fallbackSyntheticSourceBlockLoader != null) {
+            return fallbackSyntheticSourceBlockLoader;
+        } else {
+            return new DoubleScriptBlockDocValuesReader.DoubleScriptBlockLoader(leafFactory(blContext.lookup()));
+        }
     }
 
     @Override
diff --git a/server/src/main/java/org/elasticsearch/index/mapper/LongScriptFieldType.java b/server/src/main/java/org/elasticsearch/index/mapper/LongScriptFieldType.java
@@ -30,7 +30,6 @@
 
 import java.time.ZoneId;
 import java.util.Collection;
-import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.function.Function;
@@ -110,32 +109,19 @@ public DocValueFormat docValueFormat(String format, ZoneId timeZone) {
 
     @Override
     public BlockLoader blockLoader(BlockLoaderContext blContext) {
-        var indexSettings = blContext.indexSettings();
-        if (isParsedFromSource && indexSettings.getIndexMappingSourceMode() == SourceFieldMapper.Mode.SYNTHETIC
-        // A runtime and normal field can share the same name.
-        // In that case there is no ignored source entry, and so we need to fail back to LongScriptBlockLoader.
-        // We could optimize this, but at this stage feels like a rare scenario.
-            && blContext.lookup().onlyMappedAsRuntimeField(name())) {
-            var reader = new NumberType.NumberFallbackSyntheticSourceReader(NumberType.LONG, null, true) {
-                @Override
-                public void writeToBlock(List<Number> values, BlockLoader.Builder blockBuilder) {
-                    var builder = (BlockLoader.LongBuilder) blockBuilder;
-                    for (var value : values) {
-                        builder.appendLong(value.longValue());
-                    }
+        var fallbackSyntheticSourceBlockLoader = fallbackSyntheticSourceBlockLoader(
+            blContext,
+            NumberType.LONG,
+            BlockLoader.BlockFactory::longs,
+            (values, blockBuilder) -> {
+                var builder = (BlockLoader.LongBuilder) blockBuilder;
+                for (var value : values) {
+                    builder.appendLong(value.longValue());
                 }
-            };
-
-            return new FallbackSyntheticSourceBlockLoader(
-                reader,
-                name(),
-                IgnoredSourceFieldMapper.ignoredSourceFormat(indexSettings.getIndexVersionCreated())
-            ) {
-                @Override
-                public Builder builder(BlockFactory factory, int expectedCount) {
-                    return factory.longs(expectedCount);
-                }
-            };
+            }
+        );
+        if (fallbackSyntheticSourceBlockLoader != null) {
+            return fallbackSyntheticSourceBlockLoader;
         } else {
             return new LongScriptBlockDocValuesReader.LongScriptBlockLoader(leafFactory(blContext.lookup()));
         }
diff --git a/server/src/test/java/org/elasticsearch/index/mapper/DoubleScriptFieldTypeTests.java b/server/src/test/java/org/elasticsearch/index/mapper/DoubleScriptFieldTypeTests.java
@@ -28,6 +28,7 @@
 import org.apache.lucene.tests.index.RandomIndexWriter;
 import org.apache.lucene.util.BytesRef;
 import org.elasticsearch.common.lucene.search.function.ScriptScoreQuery;
+import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.index.IndexVersion;
 import org.elasticsearch.index.fielddata.DoubleScriptFieldData;
 import org.elasticsearch.index.fielddata.ScriptDocValues;
@@ -40,15 +41,19 @@
 import org.elasticsearch.script.ScriptFactory;
 import org.elasticsearch.script.ScriptType;
 import org.elasticsearch.search.MultiValueMode;
+import org.elasticsearch.search.lookup.SearchLookup;
 
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
 
 import static java.util.Collections.emptyMap;
+import static org.elasticsearch.index.mapper.LongScriptFieldTypeTests.createDocumentWithIgnoredSource;
 import static org.hamcrest.Matchers.containsInAnyOrder;
 import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.instanceOf;
+import static org.hamcrest.Matchers.nullValue;
 
 public class DoubleScriptFieldTypeTests extends AbstractNonTextScriptFieldTypeTestCase {
 
@@ -269,6 +274,69 @@ public void testBlockLoader() throws IOException {
         }
     }
 
+    public void testBlockLoaderSourceOnlyRuntimeField() throws IOException {
+        try (
+            Directory directory = newDirectory();
+            RandomIndexWriter iw = new RandomIndexWriter(random(), directory, newIndexWriterConfig().setMergePolicy(NoMergePolicy.INSTANCE))
+        ) {
+            iw.addDocuments(
+                List.of(
+                    List.of(new StoredField("_source", new BytesRef("{\"test\": [1.1]}"))),
+                    List.of(new StoredField("_source", new BytesRef("{\"test\": [2.1]}")))
+                )
+            );
+            try (DirectoryReader reader = iw.getReader()) {
+                DoubleScriptFieldType fieldType = simpleSourceOnlyMappedFieldType();
+
+                // Assert implementations:
+                BlockLoader loader = fieldType.blockLoader(blContext(Settings.EMPTY, true));
+                assertThat(loader, instanceOf(DoubleScriptBlockDocValuesReader.DoubleScriptBlockLoader.class));
+                // ignored source doesn't support column at a time loading:
+                var columnAtATimeLoader = loader.columnAtATimeReader(reader.leaves().getFirst());
+                assertThat(columnAtATimeLoader, instanceOf(DoubleScriptBlockDocValuesReader.class));
+                var rowStrideReader = loader.rowStrideReader(reader.leaves().getFirst());
+                assertThat(rowStrideReader, instanceOf(DoubleScriptBlockDocValuesReader.class));
+
+                // Assert values:
+                assertThat(blockLoaderReadValuesFromColumnAtATimeReader(reader, fieldType, 0), equalTo(List.of(1.1, 2.1)));
+                assertThat(blockLoaderReadValuesFromColumnAtATimeReader(reader, fieldType, 1), equalTo(List.of(2.1)));
+                assertThat(blockLoaderReadValuesFromRowStrideReader(reader, fieldType), equalTo(List.of(1.1, 2.1)));
+            }
+        }
+    }
+
+    public void testBlockLoaderSourceOnlyRuntimeFieldWithSyntheticSource() throws IOException {
+        var settings = Settings.builder().put("index.mapping.source.mode", "synthetic").build();
+        try (
+            Directory directory = newDirectory();
+            RandomIndexWriter iw = new RandomIndexWriter(random(), directory, newIndexWriterConfig().setMergePolicy(NoMergePolicy.INSTANCE))
+        ) {
+
+            var document1 = createDocumentWithIgnoredSource("[1.1]");
+            var document2 = createDocumentWithIgnoredSource("[2.1]");
+
+            iw.addDocuments(List.of(document1, document2));
+            try (DirectoryReader reader = iw.getReader()) {
+                DoubleScriptFieldType fieldType = simpleSourceOnlyMappedFieldType();
+
+                // Assert implementations:
+                BlockLoader loader = fieldType.blockLoader(blContext(settings, true));
+                assertThat(loader, instanceOf(FallbackSyntheticSourceBlockLoader.class));
+                // ignored source doesn't support column at a time loading:
+                var columnAtATimeLoader = loader.columnAtATimeReader(reader.leaves().getFirst());
+                assertThat(columnAtATimeLoader, nullValue());
+                var rowStrideReader = loader.rowStrideReader(reader.leaves().getFirst());
+                assertThat(
+                    rowStrideReader.getClass().getName(),
+                    equalTo("org.elasticsearch.index.mapper.FallbackSyntheticSourceBlockLoader$IgnoredSourceRowStrideReader")
+                );
+
+                // Assert values:
+                assertThat(blockLoaderReadValuesFromRowStrideReader(settings, reader, fieldType, true), equalTo(List.of(1.1, 2.1)));
+            }
+        }
+    }
+
     @Override
     protected Query randomTermsQuery(MappedFieldType ft, SearchExecutionContext ctx) {
         return ft.termsQuery(List.of(randomLong()), ctx);
@@ -279,6 +347,10 @@ protected DoubleScriptFieldType simpleMappedFieldType() {
         return build("read_foo", Map.of(), OnScriptError.FAIL);
     }
 
+    private DoubleScriptFieldType simpleSourceOnlyMappedFieldType() {
+        return build("read_test", Map.of(), OnScriptError.FAIL);
+    }
+
     @Override
     protected MappedFieldType loopFieldType() {
         return build("loop", Map.of(), OnScriptError.FAIL);
@@ -296,6 +368,31 @@ protected DoubleScriptFieldType build(String code, Map<String, Object> params, O
 
     private static DoubleFieldScript.Factory factory(Script script) {
         return switch (script.getIdOrCode()) {
+            case "read_test" -> new DoubleFieldScript.Factory() {
+                @Override
+                public DoubleFieldScript.LeafFactory newFactory(
+                    String fieldName,
+                    Map<String, Object> params,
+                    SearchLookup lookup,
+                    OnScriptError onScriptError
+                ) {
+                    return (ctx) -> new DoubleFieldScript(fieldName, params, lookup, onScriptError, ctx) {
+                        @Override
+                        @SuppressWarnings("unchecked")
+                        public void execute() {
+                            Map<String, Object> source = (Map<String, Object>) this.getParams().get("_source");
+                            for (Object foo : (List<?>) source.get("test")) {
+                                emit(((Number) foo).doubleValue());
+                            }
+                        };
+                    };
+                }
+
+                @Override
+                public boolean isParsedFromSource() {
+                    return true;
+                }
+            };
             case "read_foo" -> (fieldName, params, lookup, onScriptError) -> (ctx) -> new DoubleFieldScript(
                 fieldName,
                 params,
diff --git a/server/src/test/java/org/elasticsearch/index/mapper/LongScriptFieldTypeTests.java b/server/src/test/java/org/elasticsearch/index/mapper/LongScriptFieldTypeTests.java
@@ -374,7 +374,7 @@ public void testBlockLoaderSourceOnlyRuntimeFieldWithSyntheticSource() throws IO
         }
     }
 
-    private static LuceneDocument createDocumentWithIgnoredSource(String bytes) throws IOException {
+    static LuceneDocument createDocumentWithIgnoredSource(String bytes) throws IOException {
         var doc = new LuceneDocument();
         var parser = XContentHelper.createParser(
             XContentParserConfiguration.EMPTY,

Original file line number	Diff line number	Diff line change
`@@ -374,7 +374,7 @@ public void testBlockLoaderSourceOnlyRuntimeFieldWithSyntheticSource() throws IO`
`374`	`374`	`}`
`375`	`375`	`}`
`376`	`376`
`377`		`- private static LuceneDocument createDocumentWithIgnoredSource(String bytes) throws IOException {`
	`377`	`+ static LuceneDocument createDocumentWithIgnoredSource(String bytes) throws IOException {`
`378`	`378`	`var doc = new LuceneDocument();`
`379`	`379`	`var parser = XContentHelper.createParser(`
`380`	`380`	`XContentParserConfiguration.EMPTY,`