elastic · lkts · Feb 13, 2025 · Feb 11, 2025 · Feb 11, 2025 · Feb 11, 2025
diff --git a/...c/main/java/org/elasticsearch/benchmark/compute/operator/ValuesSourceReaderBenchmark.java b/...c/main/java/org/elasticsearch/benchmark/compute/operator/ValuesSourceReaderBenchmark.java
@@ -261,7 +261,8 @@ private static BlockLoader numericBlockLoader(WhereAndBaseName w, NumberFieldMap
             null,
             false,
             null,
-            null
+            null,
+            false
         ).blockLoader(null);
     }
 

diff --git a/...org/elasticsearch/benchmark/index/mapper/FallbackSyntheticSourceBlockLoaderBenchmark.java b/...org/elasticsearch/benchmark/index/mapper/FallbackSyntheticSourceBlockLoaderBenchmark.java
@@ -0,0 +1,3 @@
+package org.elasticsearch.benchmark.index.mapper;
+
+public class FallbackSyntheticSourceBlockLoaderBenchmark {}
diff --git a/benchmarks/src/main/java/org/elasticsearch/benchmark/script/ScriptScoreBenchmark.java b/benchmarks/src/main/java/org/elasticsearch/benchmark/script/ScriptScoreBenchmark.java
@@ -83,7 +83,7 @@ public class ScriptScoreBenchmark {
     private final ScriptModule scriptModule = new ScriptModule(Settings.EMPTY, pluginsService.filterPlugins(ScriptPlugin.class).toList());
 
     private final Map<String, MappedFieldType> fieldTypes = Map.ofEntries(
-        Map.entry("n", new NumberFieldType("n", NumberType.LONG, false, false, true, true, null, Map.of(), null, false, null, null))
+        Map.entry("n", new NumberFieldType("n", NumberType.LONG, false, false, true, true, null, Map.of(), null, false, null, null, false))
     );
     private final IndexFieldDataCache fieldDataCache = new IndexFieldDataCache.None();
     private final CircuitBreakerService breakerService = new NoneCircuitBreakerService();

diff --git a/docs/changelog/122280.yaml b/docs/changelog/122280.yaml
@@ -0,0 +1,5 @@
+pr: 122280
+summary: Use `FallbackSyntheticSourceBlockLoader` for number fields
+area: Mapping
+type: enhancement
+issues: []
diff --git a/...per-extras/src/main/java/org/elasticsearch/index/mapper/extras/TokenCountFieldMapper.java b/...per-extras/src/main/java/org/elasticsearch/index/mapper/extras/TokenCountFieldMapper.java
@@ -86,7 +86,8 @@ public TokenCountFieldMapper build(MapperBuilderContext context) {
                 store.getValue(),
                 hasDocValues.getValue(),
                 nullValue.getValue(),
-                meta.getValue()
+                meta.getValue(),
+                context.isSourceSynthetic()
             );
             return new TokenCountFieldMapper(leafName(), ft, builderParams(this, context), this);
         }
@@ -100,7 +101,8 @@ static class TokenCountFieldType extends NumberFieldMapper.NumberFieldType {
             boolean isStored,
             boolean hasDocValues,
             Number nullValue,
-            Map<String, String> meta
+            Map<String, String> meta,
+            boolean isSyntheticSource
         ) {
             super(
                 name,
@@ -114,7 +116,8 @@ static class TokenCountFieldType extends NumberFieldMapper.NumberFieldType {
                 null,
                 false,
                 null,
-                null
+                null,
+                isSyntheticSource
             );
         }
 

diff --git a/plugins/mapper-size/src/main/java/org/elasticsearch/index/mapper/size/SizeFieldMapper.java b/plugins/mapper-size/src/main/java/org/elasticsearch/index/mapper/size/SizeFieldMapper.java
@@ -50,7 +50,7 @@ public SizeFieldMapper build() {
 
     private static class SizeFieldType extends NumberFieldType {
         SizeFieldType() {
-            super(NAME, NumberType.INTEGER, true, true, true, false, null, Collections.emptyMap(), null, false, null, null);
+            super(NAME, NumberType.INTEGER, true, true, true, false, null, Collections.emptyMap(), null, false, null, null, false);
         }
 
         @Override

diff --git a/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java
@@ -269,7 +269,7 @@ public NumberFieldMapper build(MapperBuilderContext context) {
                 dimension.setValue(true);
             }
 
-            MappedFieldType ft = new NumberFieldType(context.buildFullName(leafName()), this);
+            MappedFieldType ft = new NumberFieldType(context.buildFullName(leafName()), this, context.isSourceSynthetic());
             hasScript = script.get() != null;
             onScriptError = onScriptErrorParam.getValue();
             return new NumberFieldMapper(leafName(), ft, builderParams(this, context), context.isSourceSynthetic(), this);
@@ -463,6 +463,11 @@ BlockLoader blockLoaderFromDocValues(String fieldName) {
             BlockLoader blockLoaderFromSource(SourceValueFetcher sourceValueFetcher, BlockSourceReader.LeafIteratorLookup lookup) {
                 return new BlockSourceReader.DoublesBlockLoader(sourceValueFetcher, lookup);
             }
+
+            @Override
+            BlockLoader blockLoaderFromFallbackSyntheticSource(String fieldName, Number nullValue, boolean coerce) {
+                return floatingPointBlockLoaderFromFallbackSyntheticSource(this, fieldName, nullValue, coerce);
+            }
         },
         FLOAT("float", NumericType.FLOAT) {
             @Override
@@ -647,6 +652,11 @@ BlockLoader blockLoaderFromDocValues(String fieldName) {
             BlockLoader blockLoaderFromSource(SourceValueFetcher sourceValueFetcher, BlockSourceReader.LeafIteratorLookup lookup) {
                 return new BlockSourceReader.DoublesBlockLoader(sourceValueFetcher, lookup);
             }
+
+            @Override
+            BlockLoader blockLoaderFromFallbackSyntheticSource(String fieldName, Number nullValue, boolean coerce) {
+                return floatingPointBlockLoaderFromFallbackSyntheticSource(this, fieldName, nullValue, coerce);
+            }
         },
         DOUBLE("double", NumericType.DOUBLE) {
             @Override
@@ -797,6 +807,11 @@ BlockLoader blockLoaderFromDocValues(String fieldName) {
             BlockLoader blockLoaderFromSource(SourceValueFetcher sourceValueFetcher, BlockSourceReader.LeafIteratorLookup lookup) {
                 return new BlockSourceReader.DoublesBlockLoader(sourceValueFetcher, lookup);
             }
+
+            @Override
+            BlockLoader blockLoaderFromFallbackSyntheticSource(String fieldName, Number nullValue, boolean coerce) {
+                return floatingPointBlockLoaderFromFallbackSyntheticSource(this, fieldName, nullValue, coerce);
+            }
         },
         BYTE("byte", NumericType.BYTE) {
             @Override
@@ -911,6 +926,11 @@ BlockLoader blockLoaderFromSource(SourceValueFetcher sourceValueFetcher, BlockSo
                 return new BlockSourceReader.IntsBlockLoader(sourceValueFetcher, lookup);
             }
 
+            @Override
+            BlockLoader blockLoaderFromFallbackSyntheticSource(String fieldName, Number nullValue, boolean coerce) {
+                return integerBlockLoaderFromFallbackSyntheticSource(this, fieldName, nullValue, coerce);
+            }
+
             private boolean isOutOfRange(Object value) {
                 double doubleValue = objectToDouble(value);
                 return doubleValue < Byte.MIN_VALUE || doubleValue > Byte.MAX_VALUE;
@@ -1024,6 +1044,11 @@ BlockLoader blockLoaderFromSource(SourceValueFetcher sourceValueFetcher, BlockSo
                 return new BlockSourceReader.IntsBlockLoader(sourceValueFetcher, lookup);
             }
 
+            @Override
+            BlockLoader blockLoaderFromFallbackSyntheticSource(String fieldName, Number nullValue, boolean coerce) {
+                return integerBlockLoaderFromFallbackSyntheticSource(this, fieldName, nullValue, coerce);
+            }
+
             private boolean isOutOfRange(Object value) {
                 double doubleValue = objectToDouble(value);
                 return doubleValue < Short.MIN_VALUE || doubleValue > Short.MAX_VALUE;
@@ -1210,6 +1235,11 @@ BlockLoader blockLoaderFromDocValues(String fieldName) {
             BlockLoader blockLoaderFromSource(SourceValueFetcher sourceValueFetcher, BlockSourceReader.LeafIteratorLookup lookup) {
                 return new BlockSourceReader.IntsBlockLoader(sourceValueFetcher, lookup);
             }
+
+            @Override
+            BlockLoader blockLoaderFromFallbackSyntheticSource(String fieldName, Number nullValue, boolean coerce) {
+                return integerBlockLoaderFromFallbackSyntheticSource(this, fieldName, nullValue, coerce);
+            }
         },
         LONG("long", NumericType.LONG) {
             @Override
@@ -1358,6 +1388,26 @@ BlockLoader blockLoaderFromSource(SourceValueFetcher sourceValueFetcher, BlockSo
                 return new BlockSourceReader.LongsBlockLoader(sourceValueFetcher, lookup);
             }
 
+            @Override
+            BlockLoader blockLoaderFromFallbackSyntheticSource(String fieldName, Number nullValue, boolean coerce) {
+                var reader = new NumberFallbackSyntheticSourceReader(this, nullValue, coerce) {
+                    @Override
+                    public void writeToBlock(List<Number> values, BlockLoader.Builder blockBuilder) {
+                        var builder = (BlockLoader.LongBuilder) blockBuilder;
+                        for (var value : values) {
+                            builder.appendLong(value.longValue());
+                        }
+                    }
+                };
+
+                return new FallbackSyntheticSourceBlockLoader(reader, fieldName) {
+                    @Override
+                    public Builder builder(BlockFactory factory, int expectedCount) {
+                        return factory.longs(expectedCount);
+                    }
+                };
+            }
+
             private boolean isOutOfRange(Object value) {
                 if (value instanceof Long) {
                     return false;
@@ -1626,6 +1676,106 @@ protected void writeValue(XContentBuilder b, long value) throws IOException {
         abstract BlockLoader blockLoaderFromDocValues(String fieldName);
 
         abstract BlockLoader blockLoaderFromSource(SourceValueFetcher sourceValueFetcher, BlockSourceReader.LeafIteratorLookup lookup);
+
+        abstract BlockLoader blockLoaderFromFallbackSyntheticSource(String fieldName, Number nullValue, boolean coerce);
+
+        // All values that fit into integer are returned as integers
+        private static BlockLoader integerBlockLoaderFromFallbackSyntheticSource(
+            NumberType type,
+            String fieldName,
+            Number nullValue,
+            boolean coerce
+        ) {
+            var reader = new NumberFallbackSyntheticSourceReader(type, nullValue, coerce) {
+                @Override
+                public void writeToBlock(List<Number> values, BlockLoader.Builder blockBuilder) {
+                    var builder = (BlockLoader.IntBuilder) blockBuilder;
+                    for (var value : values) {
+                        builder.appendInt(value.intValue());
+                    }
+                }
+            };
+
+            return new FallbackSyntheticSourceBlockLoader(reader, fieldName) {
+                @Override
+                public Builder builder(BlockFactory factory, int expectedCount) {
+                    return factory.ints(expectedCount);
+                }
+            };
+        }
+
+        // All floating point values are returned as doubles
+        private static BlockLoader floatingPointBlockLoaderFromFallbackSyntheticSource(
+            NumberType type,
+            String fieldName,
+            Number nullValue,
+            boolean coerce
+        ) {
+            var reader = new NumberFallbackSyntheticSourceReader(type, nullValue, coerce) {
+                @Override
+                public void writeToBlock(List<Number> values, BlockLoader.Builder blockBuilder) {
+                    var builder = (BlockLoader.DoubleBuilder) blockBuilder;
+                    for (var value : values) {
+                        builder.appendDouble(value.doubleValue());
+                    }
+                }
+            };
+
+            return new FallbackSyntheticSourceBlockLoader(reader, fieldName) {
+                @Override
+                public Builder builder(BlockFactory factory, int expectedCount) {
+                    return factory.doubles(expectedCount);
+                }
+            };
+        }
+
+        abstract static class NumberFallbackSyntheticSourceReader extends FallbackSyntheticSourceBlockLoader.ReaderWithNullValueSupport<
+            Number> {
+            private final NumberType type;
+            private final Number nullValue;
+            private final boolean coerce;
+
+            NumberFallbackSyntheticSourceReader(NumberType type, Number nullValue, boolean coerce) {
+                super(nullValue);
+                this.type = type;
+                this.nullValue = nullValue;
+                this.coerce = coerce;
+            }
+
+            @Override
+            public void convertValue(Object value, List<Number> accumulator) {
+                if (coerce && value.equals("")) {
+                    if (nullValue != null) {
+                        accumulator.add(nullValue);
+                    }
+                }
+
+                try {
+                    var converted = type.parse(value, coerce);
+                    accumulator.add(converted);
+                } catch (Exception e) {
+                    // Malformed value, skip it
+                }
+            }
+
+            @Override
+            public void parseNonNullValue(XContentParser parser, List<Number> accumulator) throws IOException {
+                // Aligned with implementation of `value(XContentParser)`
+                if (coerce && parser.currentToken() == Token.VALUE_STRING && parser.textLength() == 0) {
+                    if (nullValue != null) {
+                        accumulator.add(nullValue);
+                    }
+                }
+
+                try {
+                    Number rawValue = type.parse(parser, coerce);
+                    // Transform number to correct type (e.g. reduce precision)
+                    accumulator.add(type.parse(rawValue, coerce));
+                } catch (Exception e) {
+                    // Malformed value, skip it./gradlew ":server:test" --tests
+                }
+            }
+        };
     }
 
     public static class NumberFieldType extends SimpleMappedFieldType {
@@ -1637,6 +1787,7 @@ public static class NumberFieldType extends SimpleMappedFieldType {
         private final boolean isDimension;
         private final MetricType metricType;
         private final IndexMode indexMode;
+        private final boolean isSyntheticSource;
 
         public NumberFieldType(
             String name,
@@ -1650,7 +1801,8 @@ public NumberFieldType(
             FieldValues<Number> script,
             boolean isDimension,
             MetricType metricType,
-            IndexMode indexMode
+            IndexMode indexMode,
+            boolean isSyntheticSource
         ) {
             super(name, isIndexed, isStored, hasDocValues, TextSearchInfo.SIMPLE_MATCH_WITHOUT_TERMS, meta);
             this.type = Objects.requireNonNull(type);
@@ -1660,9 +1812,10 @@ public NumberFieldType(
             this.isDimension = isDimension;
             this.metricType = metricType;
             this.indexMode = indexMode;
+            this.isSyntheticSource = isSyntheticSource;
         }
 
-        NumberFieldType(String name, Builder builder) {
+        NumberFieldType(String name, Builder builder, boolean isSyntheticSource) {
             this(
                 name,
                 builder.type,
@@ -1675,7 +1828,8 @@ public NumberFieldType(
                 builder.scriptValues(),
                 builder.dimension.getValue(),
                 builder.metric.getValue(),
-                builder.indexMode
+                builder.indexMode,
+                isSyntheticSource
             );
         }
 
@@ -1684,7 +1838,7 @@ public NumberFieldType(String name, NumberType type) {
         }
 
         public NumberFieldType(String name, NumberType type, boolean isIndexed) {
-            this(name, type, isIndexed, false, true, true, null, Collections.emptyMap(), null, false, null, null);
+            this(name, type, isIndexed, false, true, true, null, Collections.emptyMap(), null, false, null, null, false);
         }
 
         @Override
@@ -1761,6 +1915,11 @@ public BlockLoader blockLoader(BlockLoaderContext blContext) {
             if (hasDocValues()) {
                 return type.blockLoaderFromDocValues(name());
             }
+
+            if (isSyntheticSource) {
+                return type.blockLoaderFromFallbackSyntheticSource(name(), nullValue, coerce);
+            }
+
             BlockSourceReader.LeafIteratorLookup lookup = isStored() || isIndexed()
                 ? BlockSourceReader.lookupFromFieldNames(blContext.fieldNames(), name())
                 : BlockSourceReader.lookupMatchingAll();
@@ -1876,15 +2035,15 @@ public MetricType getMetricType() {
     private final MetricType metricType;
     private boolean allowMultipleValues;
     private final IndexVersion indexCreatedVersion;
-    private final boolean storeMalformedFields;
+    private final boolean isSyntheticSource;
 
     private final IndexMode indexMode;
 
     private NumberFieldMapper(
         String simpleName,
         MappedFieldType mappedFieldType,
         BuilderParams builderParams,
-        boolean storeMalformedFields,
+        boolean isSyntheticSource,
         Builder builder
     ) {
         super(simpleName, mappedFieldType, builderParams);
@@ -1904,7 +2063,7 @@ private NumberFieldMapper(
         this.metricType = builder.metric.getValue();
         this.allowMultipleValues = builder.allowMultipleValues;
         this.indexCreatedVersion = builder.indexCreatedVersion;
-        this.storeMalformedFields = storeMalformedFields;
+        this.isSyntheticSource = isSyntheticSource;
         this.indexMode = builder.indexMode;
     }
 
@@ -1939,7 +2098,7 @@ protected void parseCreateField(DocumentParserContext context) throws IOExceptio
         } catch (IllegalArgumentException e) {
             if (ignoreMalformed.value() && context.parser().currentToken().isValue()) {
                 context.addIgnoredField(mappedFieldType.name());
-                if (storeMalformedFields) {
+                if (isSyntheticSource) {
                     // Save a copy of the field so synthetic source can load it
                     context.doc().add(IgnoreMalformedStoredValues.storedField(fullPath(), context.parser()));
                 }

diff --git a/server/src/test/java/org/elasticsearch/index/fielddata/IndexFieldDataServiceTests.java b/server/src/test/java/org/elasticsearch/index/fielddata/IndexFieldDataServiceTests.java
@@ -333,7 +333,8 @@ public void testRequireDocValuesOnLongs() {
                 null,
                 false,
                 null,
-                null
+                null,
+                false
             )
         );
     }
@@ -353,7 +354,8 @@ public void testRequireDocValuesOnDoubles() {
                 null,
                 false,
                 null,
-                null
+                null,
+                false
             )
         );
     }
-Original file line number
+Diff line change
@@ Expand Up @@
                 null,
                 false,
                 null,
-                null
+                null,
+                false
             ).blockLoader(null);
         }
@@ Expand Down @@
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,3 @@
		package org.elasticsearch.benchmark.index.mapper;

		public class FallbackSyntheticSourceBlockLoaderBenchmark {}
Copy link Member martijnvg Feb 12, 2025 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others. Learn more. Is the intend to include the benchmark? Copy link Contributor Author lkts Feb 12, 2025 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others. Learn more. I didn't intend to do that, i am not sure how this ended up being here :)