Use FallbackSyntheticSourceBlockLoader for unsigned_long and scaled_float fields (#122637) (#122877)

Author: lkts

docs/changelog/122637.yaml

@@ -0,0 +1,6 @@
+pr: 122637
+summary: Use `FallbackSyntheticSourceBlockLoader` for `unsigned_long` and `scaled_float`
+ fields
+area: Mapping
+type: enhancement
+issues: []

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

@@ -33,6 +33,7 @@
 import org.elasticsearch.index.mapper.BlockLoader;
 import org.elasticsearch.index.mapper.BlockSourceReader;
 import org.elasticsearch.index.mapper.DocumentParserContext;
+import org.elasticsearch.index.mapper.FallbackSyntheticSourceBlockLoader;
 import org.elasticsearch.index.mapper.FieldMapper;
 import org.elasticsearch.index.mapper.IgnoreMalformedStoredValues;
 import org.elasticsearch.index.mapper.MapperBuilderContext;
@@ -195,7 +196,9 @@ public ScaledFloatFieldMapper build(MapperBuilderContext context) {
  scalingFactor.getValue(),
  nullValue.getValue(),
  metric.getValue(),
- indexMode
+ indexMode,
+ coerce.getValue().value(),
+ context.isSourceSynthetic()
  );
  return new ScaledFloatFieldMapper(leafName(), type, builderParams(this, context), context.isSourceSynthetic(), this);
  }
@@ -209,6 +212,8 @@ public static final class ScaledFloatFieldType extends SimpleMappedFieldType {
  private final Double nullValue;
  private final TimeSeriesParams.MetricType metricType;
  private final IndexMode indexMode;
+ private final boolean coerce;
+ private final boolean isSyntheticSource;
 
  public ScaledFloatFieldType(
  String name,
@@ -219,21 +224,25 @@ public ScaledFloatFieldType(
  double scalingFactor,
  Double nullValue,
  TimeSeriesParams.MetricType metricType,
- IndexMode indexMode
+ IndexMode indexMode,
+ boolean coerce,
+ boolean isSyntheticSource
  ) {
  super(name, indexed, stored, hasDocValues, TextSearchInfo.SIMPLE_MATCH_WITHOUT_TERMS, meta);
  this.scalingFactor = scalingFactor;
  this.nullValue = nullValue;
  this.metricType = metricType;
  this.indexMode = indexMode;
+ this.coerce = coerce;
+ this.isSyntheticSource = isSyntheticSource;
  }
 
  public ScaledFloatFieldType(String name, double scalingFactor) {
  this(name, scalingFactor, true);
  }
 
  public ScaledFloatFieldType(String name, double scalingFactor, boolean indexed) {
- this(name, indexed, false, true, Collections.emptyMap(), scalingFactor, null, null, null);
+ this(name, indexed, false, true, Collections.emptyMap(), scalingFactor, null, null, null, false, false);
  }
 
  public double getScalingFactor() {
@@ -315,13 +324,73 @@ public BlockLoader blockLoader(BlockLoaderContext blContext) {
  double scalingFactorInverse = 1d / scalingFactor;
  return new BlockDocValuesReader.DoublesBlockLoader(name(), l -> l * scalingFactorInverse);
  }
+ if (isSyntheticSource) {
+ return new FallbackSyntheticSourceBlockLoader(fallbackSyntheticSourceBlockLoaderReader(), name()) {
+ @Override
+ public Builder builder(BlockFactory factory, int expectedCount) {
+ return factory.doubles(expectedCount);
+ }
+ };
+ }
+
  ValueFetcher valueFetcher = sourceValueFetcher(blContext.sourcePaths(name()));
  BlockSourceReader.LeafIteratorLookup lookup = isStored() || isIndexed()
  ? BlockSourceReader.lookupFromFieldNames(blContext.fieldNames(), name())
  : BlockSourceReader.lookupMatchingAll();
  return new BlockSourceReader.DoublesBlockLoader(valueFetcher, lookup);
  }
 
+ private FallbackSyntheticSourceBlockLoader.Reader<?> fallbackSyntheticSourceBlockLoaderReader() {
+ var nullValueAdjusted = nullValue != null ? adjustSourceValue(nullValue, scalingFactor) : null;
+
+ return new FallbackSyntheticSourceBlockLoader.ReaderWithNullValueSupport<>(nullValue) {
+ @Override
+ public void convertValue(Object value, List<Double> accumulator) {
+ if (coerce && value.equals("")) {
+ if (nullValueAdjusted != null) {
+ accumulator.add(nullValueAdjusted);
+ }
+ }
+
+ try {
+ // Convert to doc_values format
+ var converted = adjustSourceValue(NumberFieldMapper.NumberType.objectToDouble(value), scalingFactor);
+ accumulator.add(converted);
+ } catch (Exception e) {
+ // Malformed value, skip it
+ }
+ }
+
+ @Override
+ protected void parseNonNullValue(XContentParser parser, List<Double> accumulator) throws IOException {
+ // Aligned with implementation of `parseCreateField(XContentParser)`
+ if (coerce && parser.currentToken() == XContentParser.Token.VALUE_STRING && parser.textLength() == 0) {
+ if (nullValueAdjusted != null) {
+ accumulator.add(nullValueAdjusted);
+ }
+ }
+
+ try {
+ double value = parser.doubleValue(coerce);
+ // Convert to doc_values format
+ var converted = adjustSourceValue(value, scalingFactor);
+ accumulator.add(converted);
+ } catch (Exception e) {
+ // Malformed value, skip it
+ }
+ }
+
+ @Override
+ public void writeToBlock(List<Double> values, BlockLoader.Builder blockBuilder) {
+ var longBuilder = (BlockLoader.DoubleBuilder) blockBuilder;
+
+ for (var value : values) {
+ longBuilder.appendDouble(value);
+ }
+ }
+ };
+ }
+
  @Override
  public IndexFieldData.Builder fielddataBuilder(FieldDataContext fieldDataContext) {
  FielddataOperation operation = fieldDataContext.fielddataOperation();
@@ -386,12 +455,16 @@ protected Double parseSourceValue(Object value) {
  doubleValue = NumberFieldMapper.NumberType.objectToDouble(value);
  }
 
- double factor = getScalingFactor();
- return Math.round(doubleValue * factor) / factor;
+ return adjustSourceValue(doubleValue, getScalingFactor());
  }
  };
  }
 
+ // Adjusts precision of a double value so that it looks like it came from doc_values.
+ private static Double adjustSourceValue(double value, double scalingFactor) {
+ return Math.round(value * scalingFactor) / scalingFactor;
+ }
+
  @Override
  public Object valueForDisplay(Object value) {
  if (value == null) {

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

@@ -0,0 +1,48 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the "Elastic License
+ * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
+ * Public License v 1"; you may not use this file except in compliance with, at
+ * your election, the "Elastic License 2.0", the "GNU Affero General Public
+ * License v3.0 only", or the "Server Side Public License, v 1".
+ */
+
+package org.elasticsearch.index.mapper.extras;
+
+import org.elasticsearch.index.mapper.NumberFieldBlockLoaderTestCase;
+import org.elasticsearch.logsdb.datageneration.FieldType;
+import org.elasticsearch.plugins.Plugin;
+
+import java.util.Collection;
+import java.util.List;
+import java.util.Map;
+
+public class ScaledFloatFieldBlockLoaderTests extends NumberFieldBlockLoaderTestCase<Double> {
+ public ScaledFloatFieldBlockLoaderTests() {
+ super(FieldType.SCALED_FLOAT);
+ }
+
+ @Override
+ protected Double convert(Number value, Map<String, Object> fieldMapping) {
+ var scalingFactor = ((Number) fieldMapping.get("scaling_factor")).doubleValue();
+
+ var docValues = (boolean) fieldMapping.getOrDefault("doc_values", false);
+
+ // There is a slight inconsistency between values that are read from doc_values and from source.
+ // Due to how precision reduction is applied to source values so that they are consistent with doc_values.
+ // See #122547.
+ if (docValues) {
+ var reverseScalingFactor = 1d / scalingFactor;
+ return Math.round(value.doubleValue() * scalingFactor) * reverseScalingFactor;
+ }
+
+ // Adjust values coming from source to the way they are stored in doc_values.
+ // See mapper implementation.
+ return Math.round(value.doubleValue() * scalingFactor) / scalingFactor;
+ }
+
+ @Override
+ protected Collection<? extends Plugin> getPlugins() {
+ return List.of(new MapperExtrasPlugin());
+ }
+}

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

@@ -95,7 +95,9 @@ public void testRangeQuery() throws IOException {
  0.1 + randomDouble() * 100,
  null,
  null,
- null
+ null,
+ false,
+ false
  );
  Directory dir = newDirectory();
  IndexWriter w = new IndexWriter(dir, new IndexWriterConfig(null));

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

@@ -293,6 +293,6 @@ public void parse(XContentParser parser, List<T> accumulator) throws IOException
  parseNonNullValue(parser, accumulator);
  }
 
- abstract void parseNonNullValue(XContentParser parser, List<T> accumulator) throws IOException;
+ protected abstract void parseNonNullValue(XContentParser parser, List<T> accumulator) throws IOException;
  }
 }

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

@@ -9,15 +9,18 @@
 
 package org.elasticsearch.index.mapper.blockloader;
 
+import org.elasticsearch.index.mapper.NumberFieldBlockLoaderTestCase;
 import org.elasticsearch.logsdb.datageneration.FieldType;
 
+import java.util.Map;
+
 public class ByteFieldBlockLoaderTests extends NumberFieldBlockLoaderTestCase<Integer> {
  public ByteFieldBlockLoaderTests() {
  super(FieldType.BYTE);
  }
 
  @Override
- protected Integer convert(Number value) {
+ protected Integer convert(Number value, Map<String, Object> fieldMapping) {
  // All values that fit into int are represented as ints
  return value.intValue();
  }

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

@@ -9,15 +9,18 @@
 
 package org.elasticsearch.index.mapper.blockloader;
 
+import org.elasticsearch.index.mapper.NumberFieldBlockLoaderTestCase;
 import org.elasticsearch.logsdb.datageneration.FieldType;
 
+import java.util.Map;
+
 public class DoubleFieldBlockLoaderTests extends NumberFieldBlockLoaderTestCase<Double> {
  public DoubleFieldBlockLoaderTests() {
  super(FieldType.DOUBLE);
  }
 
  @Override
- protected Double convert(Number value) {
+ protected Double convert(Number value, Map<String, Object> fieldMapping) {
  return value.doubleValue();
  }
 }

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

@@ -9,15 +9,18 @@
 
 package org.elasticsearch.index.mapper.blockloader;
 
+import org.elasticsearch.index.mapper.NumberFieldBlockLoaderTestCase;
 import org.elasticsearch.logsdb.datageneration.FieldType;
 
+import java.util.Map;
+
 public class FloatFieldBlockLoaderTests extends NumberFieldBlockLoaderTestCase<Double> {
  public FloatFieldBlockLoaderTests() {
  super(FieldType.FLOAT);
  }
 
  @Override
- protected Double convert(Number value) {
+ protected Double convert(Number value, Map<String, Object> fieldMapping) {
  // All float values are represented as double
  return value.doubleValue();
  }

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

@@ -10,15 +10,18 @@
 package org.elasticsearch.index.mapper.blockloader;
 
 import org.apache.lucene.sandbox.document.HalfFloatPoint;
+import org.elasticsearch.index.mapper.NumberFieldBlockLoaderTestCase;
 import org.elasticsearch.logsdb.datageneration.FieldType;
 
+import java.util.Map;
+
 public class HalfFloatFieldBlockLoaderTests extends NumberFieldBlockLoaderTestCase<Double> {
  public HalfFloatFieldBlockLoaderTests() {
  super(FieldType.HALF_FLOAT);
  }
 
  @Override
- protected Double convert(Number value) {
+ protected Double convert(Number value, Map<String, Object> fieldMapping) {
  // All float values are represented as double
  return (double) HalfFloatPoint.sortableShortToHalfFloat(HalfFloatPoint.halfFloatToSortableShort(value.floatValue()));
  }

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

@@ -9,15 +9,18 @@
 
 package org.elasticsearch.index.mapper.blockloader;
 
+import org.elasticsearch.index.mapper.NumberFieldBlockLoaderTestCase;
 import org.elasticsearch.logsdb.datageneration.FieldType;
 
+import java.util.Map;
+
 public class IntegerFieldBlockLoaderTests extends NumberFieldBlockLoaderTestCase<Integer> {
  public IntegerFieldBlockLoaderTests() {
  super(FieldType.INTEGER);
  }
 
  @Override
- protected Integer convert(Number value) {
+ protected Integer convert(Number value, Map<String, Object> fieldMapping) {
  return value.intValue();
  }
 }