Use FallbackSyntheticSourceBlockLoader for number fields (elastic#122280) (elastic#122452)

Author: lkts

benchmarks/src/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);
     }
 

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();

docs/changelog/122280.yaml

@@ -0,0 +1,5 @@
+pr: 122280
+summary: Use `FallbackSyntheticSourceBlockLoader` for number fields
+area: Mapping
+type: enhancement
+issues: []

modules/mapper-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
             );
         }
 

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

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

@@ -270,7 +270,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);
@@ -464,6 +464,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
@@ -648,6 +653,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
@@ -798,6 +808,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
@@ -912,6 +927,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;
@@ -1025,6 +1045,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;
@@ -1211,6 +1236,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
@@ -1359,6 +1389,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;
@@ -1635,6 +1685,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
+                }
+            }
+        };
     }
 
     public static class NumberFieldType extends SimpleMappedFieldType {
@@ -1646,6 +1796,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,
@@ -1659,7 +1810,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);
@@ -1669,9 +1821,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,
@@ -1684,7 +1837,8 @@ public NumberFieldType(
                 builder.scriptValues(),
                 builder.dimension.getValue(),
                 builder.metric.getValue(),
-                builder.indexMode
+                builder.indexMode,
+                isSyntheticSource
             );
         }
 
@@ -1693,7 +1847,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
@@ -1770,6 +1924,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();
@@ -1885,15 +2044,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);
@@ -1913,7 +2072,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;
     }
 
@@ -1948,7 +2107,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()));
                 }

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
             )
         );
     }