Skip to content

Commit

Permalink
Use FallbackSyntheticSourceBlockLoader for number fields (elastic#122280
Browse files Browse the repository at this point in the history 
)
  • Loading branch information
@lkts
lkts authored Feb 13, 2025
1 parent 6c6e8d8 commit b8d7e99
Show file tree
Hide file tree
Showing 23 changed files with 447 additions and 39 deletions.
3 changes: 2 additions & 1 deletion ...c/main/java/org/elasticsearch/benchmark/compute/operator/ValuesSourceReaderBenchmark.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,8 @@ private static BlockLoader numericBlockLoader(WhereAndBaseName w, NumberFieldMap
null,
false,
null,
null
null,
false
).blockLoader(null);
}

Expand Down
2 changes: 1 addition & 1 deletion benchmarks/src/main/java/org/elasticsearch/benchmark/script/ScriptScoreBenchmark.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
5 changes: 5 additions & 0 deletions docs/changelog/122280.yaml
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
pr: 122280
summary: Use `FallbackSyntheticSourceBlockLoader` for number fields
area: Mapping
type: enhancement
issues: []
9 changes: 6 additions & 3 deletions ...per-extras/src/main/java/org/elasticsearch/index/mapper/extras/TokenCountFieldMapper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand All @@ -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,
Expand All @@ -114,7 +116,8 @@ static class TokenCountFieldType extends NumberFieldMapper.NumberFieldType {
null,
false,
null,
null
null,
isSyntheticSource
);
}

Expand Down
2 changes: 1 addition & 1 deletion plugins/mapper-size/src/main/java/org/elasticsearch/index/mapper/size/SizeFieldMapper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
177 changes: 168 additions & 9 deletions server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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
}
}
};
}

public static class NumberFieldType extends SimpleMappedFieldType {
Expand All @@ -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,
Expand All @@ -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);
Expand All @@ -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,
Expand All @@ -1675,7 +1828,8 @@ public NumberFieldType(
builder.scriptValues(),
builder.dimension.getValue(),
builder.metric.getValue(),
builder.indexMode
builder.indexMode,
isSyntheticSource
);
}

Expand All @@ -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
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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);
Expand All @@ -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;
}

Expand Down Expand Up @@ -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()));
}
Expand Down
6 changes: 4 additions & 2 deletions server/src/test/java/org/elasticsearch/index/fielddata/IndexFieldDataServiceTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,8 @@ public void testRequireDocValuesOnLongs() {
null,
false,
null,
null
null,
false
)
);
}
Expand All @@ -353,7 +354,8 @@ public void testRequireDocValuesOnDoubles() {
null,
false,
null,
null
null,
false
)
);
}
Expand Down
Loading

0 comments on commit b8d7e99

Please sign in to comment.