diff --git a/docs/changelog/122637.yaml b/docs/changelog/122637.yaml new file mode 100644 index 0000000000000..e9c108311d208 --- /dev/null +++ b/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: [] diff --git a/modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/extras/ScaledFloatFieldMapper.java b/modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/extras/ScaledFloatFieldMapper.java index a91ca66faa405..8f269ad4066c0 100644 --- a/modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/extras/ScaledFloatFieldMapper.java +++ b/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,13 +224,17 @@ 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) { @@ -233,7 +242,7 @@ public ScaledFloatFieldType(String name, double scalingFactor) { } 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,6 +324,15 @@ 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()) @@ -322,6 +340,57 @@ public BlockLoader blockLoader(BlockLoaderContext blContext) { 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 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 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 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) { diff --git a/modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/extras/ScaledFloatFieldBlockLoaderTests.java b/modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/extras/ScaledFloatFieldBlockLoaderTests.java new file mode 100644 index 0000000000000..5541b5677f00e --- /dev/null +++ b/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 { + public ScaledFloatFieldBlockLoaderTests() { + super(FieldType.SCALED_FLOAT); + } + + @Override + protected Double convert(Number value, Map 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 getPlugins() { + return List.of(new MapperExtrasPlugin()); + } +} diff --git a/modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/extras/ScaledFloatFieldTypeTests.java b/modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/extras/ScaledFloatFieldTypeTests.java index 0c69ec6258589..aaf1abc735c3d 100644 --- a/modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/extras/ScaledFloatFieldTypeTests.java +++ b/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)); diff --git a/server/src/main/java/org/elasticsearch/index/mapper/FallbackSyntheticSourceBlockLoader.java b/server/src/main/java/org/elasticsearch/index/mapper/FallbackSyntheticSourceBlockLoader.java index 64f4d1bec04c7..68b2e31a4b011 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/FallbackSyntheticSourceBlockLoader.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/FallbackSyntheticSourceBlockLoader.java @@ -293,6 +293,6 @@ public void parse(XContentParser parser, List accumulator) throws IOException parseNonNullValue(parser, accumulator); } - abstract void parseNonNullValue(XContentParser parser, List accumulator) throws IOException; + protected abstract void parseNonNullValue(XContentParser parser, List accumulator) throws IOException; } } diff --git a/server/src/test/java/org/elasticsearch/index/mapper/blockloader/ByteFieldBlockLoaderTests.java b/server/src/test/java/org/elasticsearch/index/mapper/blockloader/ByteFieldBlockLoaderTests.java index 28d7cbcfb42db..0f8283c4fd426 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/blockloader/ByteFieldBlockLoaderTests.java +++ b/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 { public ByteFieldBlockLoaderTests() { super(FieldType.BYTE); } @Override - protected Integer convert(Number value) { + protected Integer convert(Number value, Map fieldMapping) { // All values that fit into int are represented as ints return value.intValue(); } diff --git a/server/src/test/java/org/elasticsearch/index/mapper/blockloader/DoubleFieldBlockLoaderTests.java b/server/src/test/java/org/elasticsearch/index/mapper/blockloader/DoubleFieldBlockLoaderTests.java index e0b62b21ad87a..5a504487680c5 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/blockloader/DoubleFieldBlockLoaderTests.java +++ b/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 { public DoubleFieldBlockLoaderTests() { super(FieldType.DOUBLE); } @Override - protected Double convert(Number value) { + protected Double convert(Number value, Map fieldMapping) { return value.doubleValue(); } } diff --git a/server/src/test/java/org/elasticsearch/index/mapper/blockloader/FloatFieldBlockLoaderTests.java b/server/src/test/java/org/elasticsearch/index/mapper/blockloader/FloatFieldBlockLoaderTests.java index 63439a97d7c9d..9b82293d33918 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/blockloader/FloatFieldBlockLoaderTests.java +++ b/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 { public FloatFieldBlockLoaderTests() { super(FieldType.FLOAT); } @Override - protected Double convert(Number value) { + protected Double convert(Number value, Map fieldMapping) { // All float values are represented as double return value.doubleValue(); } diff --git a/server/src/test/java/org/elasticsearch/index/mapper/blockloader/HalfFloatFieldBlockLoaderTests.java b/server/src/test/java/org/elasticsearch/index/mapper/blockloader/HalfFloatFieldBlockLoaderTests.java index 1e8cedb734af3..23e6c86747f15 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/blockloader/HalfFloatFieldBlockLoaderTests.java +++ b/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 { public HalfFloatFieldBlockLoaderTests() { super(FieldType.HALF_FLOAT); } @Override - protected Double convert(Number value) { + protected Double convert(Number value, Map fieldMapping) { // All float values are represented as double return (double) HalfFloatPoint.sortableShortToHalfFloat(HalfFloatPoint.halfFloatToSortableShort(value.floatValue())); } diff --git a/server/src/test/java/org/elasticsearch/index/mapper/blockloader/IntegerFieldBlockLoaderTests.java b/server/src/test/java/org/elasticsearch/index/mapper/blockloader/IntegerFieldBlockLoaderTests.java index 5d7b9d78442cb..6a9c9ca4ba7d5 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/blockloader/IntegerFieldBlockLoaderTests.java +++ b/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 { public IntegerFieldBlockLoaderTests() { super(FieldType.INTEGER); } @Override - protected Integer convert(Number value) { + protected Integer convert(Number value, Map fieldMapping) { return value.intValue(); } } diff --git a/server/src/test/java/org/elasticsearch/index/mapper/blockloader/LongFieldBlockLoaderTests.java b/server/src/test/java/org/elasticsearch/index/mapper/blockloader/LongFieldBlockLoaderTests.java index ff953294fb618..b6425029175e2 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/blockloader/LongFieldBlockLoaderTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/blockloader/LongFieldBlockLoaderTests.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 LongFieldBlockLoaderTests extends NumberFieldBlockLoaderTestCase { public LongFieldBlockLoaderTests() { super(FieldType.LONG); } @Override - protected Long convert(Number value) { + protected Long convert(Number value, Map fieldMapping) { return value.longValue(); } } diff --git a/server/src/test/java/org/elasticsearch/index/mapper/blockloader/ShortFieldBlockLoaderTests.java b/server/src/test/java/org/elasticsearch/index/mapper/blockloader/ShortFieldBlockLoaderTests.java index a40bc1c404f45..1f3286c3398c4 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/blockloader/ShortFieldBlockLoaderTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/blockloader/ShortFieldBlockLoaderTests.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 ShortFieldBlockLoaderTests extends NumberFieldBlockLoaderTestCase { public ShortFieldBlockLoaderTests() { super(FieldType.SHORT); } @Override - protected Integer convert(Number value) { + protected Integer convert(Number value, Map fieldMapping) { // All values that fit into int are represented as ints return value.intValue(); } diff --git a/server/src/test/java/org/elasticsearch/index/mapper/blockloader/NumberFieldBlockLoaderTestCase.java b/test/framework/src/main/java/org/elasticsearch/index/mapper/NumberFieldBlockLoaderTestCase.java similarity index 72% rename from server/src/test/java/org/elasticsearch/index/mapper/blockloader/NumberFieldBlockLoaderTestCase.java rename to test/framework/src/main/java/org/elasticsearch/index/mapper/NumberFieldBlockLoaderTestCase.java index e523d011c3ab1..e5f68362005d5 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/blockloader/NumberFieldBlockLoaderTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/index/mapper/NumberFieldBlockLoaderTestCase.java @@ -7,9 +7,8 @@ * License v3.0 only", or the "Server Side Public License, v 1". */ -package org.elasticsearch.index.mapper.blockloader; +package org.elasticsearch.index.mapper; -import org.elasticsearch.index.mapper.BlockLoaderTestCase; import org.elasticsearch.logsdb.datageneration.FieldType; import java.util.List; @@ -24,25 +23,29 @@ public NumberFieldBlockLoaderTestCase(FieldType fieldType) { @Override @SuppressWarnings("unchecked") protected Object expected(Map fieldMapping, Object value, boolean syntheticSource) { - var nullValue = fieldMapping.get("null_value") != null ? convert((Number) fieldMapping.get("null_value")) : null; + var nullValue = fieldMapping.get("null_value") != null ? convert((Number) fieldMapping.get("null_value"), fieldMapping) : null; if (value instanceof List == false) { - return convert(value, nullValue); + return convert(value, nullValue, fieldMapping); } if ((boolean) fieldMapping.getOrDefault("doc_values", false)) { // Sorted and no duplicates - var resultList = ((List) value).stream().map(v -> convert(v, nullValue)).filter(Objects::nonNull).sorted().toList(); + var resultList = ((List) value).stream() + .map(v -> convert(v, nullValue, fieldMapping)) + .filter(Objects::nonNull) + .sorted() + .toList(); return maybeFoldList(resultList); } // parsing from source - var resultList = ((List) value).stream().map(v -> convert(v, nullValue)).filter(Objects::nonNull).toList(); + var resultList = ((List) value).stream().map(v -> convert(v, nullValue, fieldMapping)).filter(Objects::nonNull).toList(); return maybeFoldList(resultList); } @SuppressWarnings("unchecked") - private T convert(Object value, T nullValue) { + private T convert(Object value, T nullValue, Map fieldMapping) { if (value == null) { return nullValue; } @@ -51,12 +54,12 @@ private T convert(Object value, T nullValue) { return nullValue; } if (value instanceof Number n) { - return convert(n); + return convert(n, fieldMapping); } // Malformed values are excluded return null; } - protected abstract T convert(Number value); + protected abstract T convert(Number value, Map fieldMapping); } diff --git a/test/framework/src/main/java/org/elasticsearch/logsdb/datageneration/datasource/DefaultMappingParametersHandler.java b/test/framework/src/main/java/org/elasticsearch/logsdb/datageneration/datasource/DefaultMappingParametersHandler.java index bf99ab71d0149..e7b9140b07daf 100644 --- a/test/framework/src/main/java/org/elasticsearch/logsdb/datageneration/datasource/DefaultMappingParametersHandler.java +++ b/test/framework/src/main/java/org/elasticsearch/logsdb/datageneration/datasource/DefaultMappingParametersHandler.java @@ -102,7 +102,7 @@ private Supplier> scaledFloatMapping(Map inj injected.put("scaling_factor", ESTestCase.randomFrom(10, 1000, 100000, 100.5)); if (ESTestCase.randomDouble() <= 0.2) { - injected.put("null_value", ESTestCase.randomFloat()); + injected.put("null_value", ESTestCase.randomDouble()); } if (ESTestCase.randomBoolean()) { diff --git a/x-pack/plugin/mapper-unsigned-long/src/main/java/org/elasticsearch/xpack/unsignedlong/UnsignedLongFieldMapper.java b/x-pack/plugin/mapper-unsigned-long/src/main/java/org/elasticsearch/xpack/unsignedlong/UnsignedLongFieldMapper.java index 7af69a1a2b453..a99f8066d9598 100644 --- a/x-pack/plugin/mapper-unsigned-long/src/main/java/org/elasticsearch/xpack/unsignedlong/UnsignedLongFieldMapper.java +++ b/x-pack/plugin/mapper-unsigned-long/src/main/java/org/elasticsearch/xpack/unsignedlong/UnsignedLongFieldMapper.java @@ -28,6 +28,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.MappedFieldType; @@ -207,7 +208,8 @@ public UnsignedLongFieldMapper build(MapperBuilderContext context) { meta.getValue(), dimension.getValue(), metric.getValue(), - indexMode + indexMode, + context.isSourceSynthetic() ); return new UnsignedLongFieldMapper(leafName(), fieldType, builderParams(this, context), context.isSourceSynthetic(), this); } @@ -221,6 +223,7 @@ public static final class UnsignedLongFieldType extends SimpleMappedFieldType { private final boolean isDimension; private final MetricType metricType; private final IndexMode indexMode; + private final boolean isSyntheticSource; public UnsignedLongFieldType( String name, @@ -231,17 +234,19 @@ public UnsignedLongFieldType( Map meta, boolean isDimension, MetricType metricType, - IndexMode indexMode + IndexMode indexMode, + boolean isSyntheticSource ) { super(name, indexed, isStored, hasDocValues, TextSearchInfo.SIMPLE_MATCH_WITHOUT_TERMS, meta); this.nullValueFormatted = nullValueFormatted; this.isDimension = isDimension; this.metricType = metricType; this.indexMode = indexMode; + this.isSyntheticSource = isSyntheticSource; } public UnsignedLongFieldType(String name) { - this(name, true, false, true, null, Collections.emptyMap(), false, null, null); + this(name, true, false, true, null, Collections.emptyMap(), false, null, null, false); } @Override @@ -327,11 +332,24 @@ public BlockLoader blockLoader(BlockLoaderContext blContext) { if (hasDocValues()) { return new BlockDocValuesReader.LongsBlockLoader(name()); } + if (isSyntheticSource) { + return new FallbackSyntheticSourceBlockLoader(fallbackSyntheticSourceBlockLoaderReader(), name()) { + @Override + public Builder builder(BlockFactory factory, int expectedCount) { + return factory.longs(expectedCount); + } + }; + } + ValueFetcher valueFetcher = new SourceValueFetcher(blContext.sourcePaths(name()), nullValueFormatted) { @Override protected Object parseSourceValue(Object value) { if (value.equals("")) { - return nullValueFormatted; + // `nullValueFormatted` is an unsigned value formatted for human use + // but block loaders operate with encoded signed 64 bit values + // because this is the format doc_values use. + // So we need to perform this conversion. + return unsignedToSortableSignedLong(parseUnsignedLong(nullValueFormatted)); } return unsignedToSortableSignedLong(parseUnsignedLong(value)); } @@ -342,6 +360,64 @@ protected Object parseSourceValue(Object value) { return new BlockSourceReader.LongsBlockLoader(valueFetcher, lookup); } + private FallbackSyntheticSourceBlockLoader.Reader fallbackSyntheticSourceBlockLoaderReader() { + var nullValueEncoded = nullValueFormatted != null + ? (Number) unsignedToSortableSignedLong(parseUnsignedLong(nullValueFormatted)) + : null; + return new FallbackSyntheticSourceBlockLoader.ReaderWithNullValueSupport<>(nullValueFormatted) { + @Override + public void convertValue(Object value, List accumulator) { + if (value.equals("")) { + if (nullValueEncoded != null) { + accumulator.add(nullValueEncoded); + } + } + + try { + // Convert to doc_values format + var converted = unsignedToSortableSignedLong(parseUnsignedLong(value)); + accumulator.add(converted); + } catch (Exception e) { + // Malformed value, skip it + } + } + + @Override + protected void parseNonNullValue(XContentParser parser, List accumulator) throws IOException { + // Aligned with implementation of `parseCreateField(XContentParser)` + if (parser.currentToken() == XContentParser.Token.VALUE_STRING && parser.textLength() == 0) { + if (nullValueEncoded != null) { + accumulator.add(nullValueEncoded); + } + } + + try { + Long rawValue; + if (parser.currentToken() == XContentParser.Token.VALUE_NUMBER) { + rawValue = parseUnsignedLong(parser.numberValue()); + } else { + rawValue = parseUnsignedLong(parser.text()); + } + + // Convert to doc_values format + var converted = unsignedToSortableSignedLong(rawValue); + accumulator.add(converted); + } catch (Exception e) { + // Malformed value, skip it + } + } + + @Override + public void writeToBlock(List values, BlockLoader.Builder blockBuilder) { + var longBuilder = (BlockLoader.LongBuilder) blockBuilder; + + for (var value : values) { + longBuilder.appendLong(value.longValue()); + } + } + }; + } + @Override public IndexFieldData.Builder fielddataBuilder(FieldDataContext fieldDataContext) { FielddataOperation operation = fieldDataContext.fielddataOperation(); diff --git a/x-pack/plugin/mapper-unsigned-long/src/test/java/org/elasticsearch/xpack/unsignedlong/UnsignedLongFieldBlockLoaderTests.java b/x-pack/plugin/mapper-unsigned-long/src/test/java/org/elasticsearch/xpack/unsignedlong/UnsignedLongFieldBlockLoaderTests.java new file mode 100644 index 0000000000000..562b1da7f07e3 --- /dev/null +++ b/x-pack/plugin/mapper-unsigned-long/src/test/java/org/elasticsearch/xpack/unsignedlong/UnsignedLongFieldBlockLoaderTests.java @@ -0,0 +1,37 @@ +/* + * 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; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +package org.elasticsearch.xpack.unsignedlong; + +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 UnsignedLongFieldBlockLoaderTests extends NumberFieldBlockLoaderTestCase { + private static final long MASK_2_63 = 0x8000000000000000L; + + public UnsignedLongFieldBlockLoaderTests() { + super(FieldType.UNSIGNED_LONG); + } + + @Override + protected Long convert(Number value, Map fieldMapping) { + // Adjust values coming from source to the way they are stored in doc_values. + // See mapper implementation. + var unsigned = value.longValue(); + return unsigned ^ MASK_2_63; + } + + @Override + protected Collection getPlugins() { + return List.of(new UnsignedLongMapperPlugin()); + } +} diff --git a/x-pack/plugin/mapper-unsigned-long/src/test/java/org/elasticsearch/xpack/unsignedlong/UnsignedLongFieldTypeTests.java b/x-pack/plugin/mapper-unsigned-long/src/test/java/org/elasticsearch/xpack/unsignedlong/UnsignedLongFieldTypeTests.java index e5f85f8b87b12..72cd64516617f 100644 --- a/x-pack/plugin/mapper-unsigned-long/src/test/java/org/elasticsearch/xpack/unsignedlong/UnsignedLongFieldTypeTests.java +++ b/x-pack/plugin/mapper-unsigned-long/src/test/java/org/elasticsearch/xpack/unsignedlong/UnsignedLongFieldTypeTests.java @@ -63,7 +63,8 @@ public void testRangeQuery() { Collections.emptyMap(), false, null, - null + null, + false ); assertEquals(