diff --git a/docs/changelog/134629.yaml b/docs/changelog/134629.yaml new file mode 100644 index 0000000000000..85905f00519c7 --- /dev/null +++ b/docs/changelog/134629.yaml @@ -0,0 +1,5 @@ +pr: 134629 +summary: Improve block loader for source only runtime fields of type double +area: Mapping +type: enhancement +issues: [] diff --git a/server/src/main/java/org/elasticsearch/index/mapper/AbstractScriptFieldType.java b/server/src/main/java/org/elasticsearch/index/mapper/AbstractScriptFieldType.java index 7f4b287018bed..2bbe78c8fc1dd 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/AbstractScriptFieldType.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/AbstractScriptFieldType.java @@ -37,6 +37,8 @@ import java.util.Locale; import java.util.Map; import java.util.Objects; +import java.util.function.BiConsumer; +import java.util.function.BiFunction; import java.util.function.Function; import static org.elasticsearch.search.SearchService.ALLOW_EXPENSIVE_QUERIES; @@ -203,6 +205,45 @@ protected final LeafFactory leafFactory(SearchLookup searchLookup) { } } + /** + * Returns synthetic source fallback block loader if source mode is synthetic, runtime field is source only and field is only mapped + * as a runtime field. + */ + protected final FallbackSyntheticSourceBlockLoader fallbackSyntheticSourceBlockLoader( + BlockLoaderContext blContext, + NumberFieldMapper.NumberType numberType, + BiFunction builderSupplier, + BiConsumer, BlockLoader.Builder> writeToBlock + ) { + var indexSettings = blContext.indexSettings(); + // A runtime and normal field can share the same name. + // In that case there is no ignored source entry, and so we need to fail back to LongScriptBlockLoader. + // We could optimize this, but at this stage feels like a rare scenario. + if (isParsedFromSource + && indexSettings.getIndexMappingSourceMode() == SourceFieldMapper.Mode.SYNTHETIC + && blContext.lookup().onlyMappedAsRuntimeField(name())) { + var reader = new NumberFieldMapper.NumberType.NumberFallbackSyntheticSourceReader(numberType, null, true) { + @Override + public void writeToBlock(List values, BlockLoader.Builder blockBuilder) { + writeToBlock.accept(values, blockBuilder); + } + }; + + return new FallbackSyntheticSourceBlockLoader( + reader, + name(), + IgnoredSourceFieldMapper.ignoredSourceFormat(indexSettings.getIndexVersionCreated()) + ) { + @Override + public Builder builder(BlockFactory factory, int expectedCount) { + return builderSupplier.apply(factory, expectedCount); + } + }; + } else { + return null; + } + } + /** * Create a script leaf factory for queries. */ diff --git a/server/src/main/java/org/elasticsearch/index/mapper/DoubleScriptFieldType.java b/server/src/main/java/org/elasticsearch/index/mapper/DoubleScriptFieldType.java index 7c7cd2edca6d4..e321216102569 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/DoubleScriptFieldType.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/DoubleScriptFieldType.java @@ -109,7 +109,22 @@ public DocValueFormat docValueFormat(String format, ZoneId timeZone) { @Override public BlockLoader blockLoader(BlockLoaderContext blContext) { - return new DoubleScriptBlockDocValuesReader.DoubleScriptBlockLoader(leafFactory(blContext.lookup())); + var fallbackSyntheticSourceBlockLoader = fallbackSyntheticSourceBlockLoader( + blContext, + NumberType.DOUBLE, + BlockLoader.BlockFactory::doubles, + (values, blockBuilder) -> { + var builder = (BlockLoader.DoubleBuilder) blockBuilder; + for (var value : values) { + builder.appendDouble(value.doubleValue()); + } + } + ); + if (fallbackSyntheticSourceBlockLoader != null) { + return fallbackSyntheticSourceBlockLoader; + } else { + return new DoubleScriptBlockDocValuesReader.DoubleScriptBlockLoader(leafFactory(blContext.lookup())); + } } @Override diff --git a/server/src/main/java/org/elasticsearch/index/mapper/LongScriptFieldType.java b/server/src/main/java/org/elasticsearch/index/mapper/LongScriptFieldType.java index cbfd9ad2c3570..03fa5ac4e24b7 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/LongScriptFieldType.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/LongScriptFieldType.java @@ -30,7 +30,6 @@ import java.time.ZoneId; import java.util.Collection; -import java.util.List; import java.util.Map; import java.util.Set; import java.util.function.Function; @@ -110,32 +109,19 @@ public DocValueFormat docValueFormat(String format, ZoneId timeZone) { @Override public BlockLoader blockLoader(BlockLoaderContext blContext) { - var indexSettings = blContext.indexSettings(); - if (isParsedFromSource && indexSettings.getIndexMappingSourceMode() == SourceFieldMapper.Mode.SYNTHETIC - // A runtime and normal field can share the same name. - // In that case there is no ignored source entry, and so we need to fail back to LongScriptBlockLoader. - // We could optimize this, but at this stage feels like a rare scenario. - && blContext.lookup().onlyMappedAsRuntimeField(name())) { - var reader = new NumberType.NumberFallbackSyntheticSourceReader(NumberType.LONG, null, true) { - @Override - public void writeToBlock(List values, BlockLoader.Builder blockBuilder) { - var builder = (BlockLoader.LongBuilder) blockBuilder; - for (var value : values) { - builder.appendLong(value.longValue()); - } + var fallbackSyntheticSourceBlockLoader = fallbackSyntheticSourceBlockLoader( + blContext, + NumberType.LONG, + BlockLoader.BlockFactory::longs, + (values, blockBuilder) -> { + var builder = (BlockLoader.LongBuilder) blockBuilder; + for (var value : values) { + builder.appendLong(value.longValue()); } - }; - - return new FallbackSyntheticSourceBlockLoader( - reader, - name(), - IgnoredSourceFieldMapper.ignoredSourceFormat(indexSettings.getIndexVersionCreated()) - ) { - @Override - public Builder builder(BlockFactory factory, int expectedCount) { - return factory.longs(expectedCount); - } - }; + } + ); + if (fallbackSyntheticSourceBlockLoader != null) { + return fallbackSyntheticSourceBlockLoader; } else { return new LongScriptBlockDocValuesReader.LongScriptBlockLoader(leafFactory(blContext.lookup())); } diff --git a/server/src/test/java/org/elasticsearch/index/mapper/DoubleScriptFieldTypeTests.java b/server/src/test/java/org/elasticsearch/index/mapper/DoubleScriptFieldTypeTests.java index b1cda53876993..709e90bdb2ecf 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/DoubleScriptFieldTypeTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/DoubleScriptFieldTypeTests.java @@ -28,6 +28,7 @@ import org.apache.lucene.tests.index.RandomIndexWriter; import org.apache.lucene.util.BytesRef; import org.elasticsearch.common.lucene.search.function.ScriptScoreQuery; +import org.elasticsearch.common.settings.Settings; import org.elasticsearch.index.IndexVersion; import org.elasticsearch.index.fielddata.DoubleScriptFieldData; import org.elasticsearch.index.fielddata.ScriptDocValues; @@ -40,6 +41,7 @@ import org.elasticsearch.script.ScriptFactory; import org.elasticsearch.script.ScriptType; import org.elasticsearch.search.MultiValueMode; +import org.elasticsearch.search.lookup.SearchLookup; import java.io.IOException; import java.util.ArrayList; @@ -47,8 +49,11 @@ import java.util.Map; import static java.util.Collections.emptyMap; +import static org.elasticsearch.index.mapper.LongScriptFieldTypeTests.createDocumentWithIgnoredSource; import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.instanceOf; +import static org.hamcrest.Matchers.nullValue; public class DoubleScriptFieldTypeTests extends AbstractNonTextScriptFieldTypeTestCase { @@ -269,6 +274,69 @@ public void testBlockLoader() throws IOException { } } + public void testBlockLoaderSourceOnlyRuntimeField() throws IOException { + try ( + Directory directory = newDirectory(); + RandomIndexWriter iw = new RandomIndexWriter(random(), directory, newIndexWriterConfig().setMergePolicy(NoMergePolicy.INSTANCE)) + ) { + iw.addDocuments( + List.of( + List.of(new StoredField("_source", new BytesRef("{\"test\": [1.1]}"))), + List.of(new StoredField("_source", new BytesRef("{\"test\": [2.1]}"))) + ) + ); + try (DirectoryReader reader = iw.getReader()) { + DoubleScriptFieldType fieldType = simpleSourceOnlyMappedFieldType(); + + // Assert implementations: + BlockLoader loader = fieldType.blockLoader(blContext(Settings.EMPTY, true)); + assertThat(loader, instanceOf(DoubleScriptBlockDocValuesReader.DoubleScriptBlockLoader.class)); + // ignored source doesn't support column at a time loading: + var columnAtATimeLoader = loader.columnAtATimeReader(reader.leaves().getFirst()); + assertThat(columnAtATimeLoader, instanceOf(DoubleScriptBlockDocValuesReader.class)); + var rowStrideReader = loader.rowStrideReader(reader.leaves().getFirst()); + assertThat(rowStrideReader, instanceOf(DoubleScriptBlockDocValuesReader.class)); + + // Assert values: + assertThat(blockLoaderReadValuesFromColumnAtATimeReader(reader, fieldType, 0), equalTo(List.of(1.1, 2.1))); + assertThat(blockLoaderReadValuesFromColumnAtATimeReader(reader, fieldType, 1), equalTo(List.of(2.1))); + assertThat(blockLoaderReadValuesFromRowStrideReader(reader, fieldType), equalTo(List.of(1.1, 2.1))); + } + } + } + + public void testBlockLoaderSourceOnlyRuntimeFieldWithSyntheticSource() throws IOException { + var settings = Settings.builder().put("index.mapping.source.mode", "synthetic").build(); + try ( + Directory directory = newDirectory(); + RandomIndexWriter iw = new RandomIndexWriter(random(), directory, newIndexWriterConfig().setMergePolicy(NoMergePolicy.INSTANCE)) + ) { + + var document1 = createDocumentWithIgnoredSource("[1.1]"); + var document2 = createDocumentWithIgnoredSource("[2.1]"); + + iw.addDocuments(List.of(document1, document2)); + try (DirectoryReader reader = iw.getReader()) { + DoubleScriptFieldType fieldType = simpleSourceOnlyMappedFieldType(); + + // Assert implementations: + BlockLoader loader = fieldType.blockLoader(blContext(settings, true)); + assertThat(loader, instanceOf(FallbackSyntheticSourceBlockLoader.class)); + // ignored source doesn't support column at a time loading: + var columnAtATimeLoader = loader.columnAtATimeReader(reader.leaves().getFirst()); + assertThat(columnAtATimeLoader, nullValue()); + var rowStrideReader = loader.rowStrideReader(reader.leaves().getFirst()); + assertThat( + rowStrideReader.getClass().getName(), + equalTo("org.elasticsearch.index.mapper.FallbackSyntheticSourceBlockLoader$IgnoredSourceRowStrideReader") + ); + + // Assert values: + assertThat(blockLoaderReadValuesFromRowStrideReader(settings, reader, fieldType, true), equalTo(List.of(1.1, 2.1))); + } + } + } + @Override protected Query randomTermsQuery(MappedFieldType ft, SearchExecutionContext ctx) { return ft.termsQuery(List.of(randomLong()), ctx); @@ -279,6 +347,10 @@ protected DoubleScriptFieldType simpleMappedFieldType() { return build("read_foo", Map.of(), OnScriptError.FAIL); } + private DoubleScriptFieldType simpleSourceOnlyMappedFieldType() { + return build("read_test", Map.of(), OnScriptError.FAIL); + } + @Override protected MappedFieldType loopFieldType() { return build("loop", Map.of(), OnScriptError.FAIL); @@ -296,6 +368,31 @@ protected DoubleScriptFieldType build(String code, Map params, O private static DoubleFieldScript.Factory factory(Script script) { return switch (script.getIdOrCode()) { + case "read_test" -> new DoubleFieldScript.Factory() { + @Override + public DoubleFieldScript.LeafFactory newFactory( + String fieldName, + Map params, + SearchLookup lookup, + OnScriptError onScriptError + ) { + return (ctx) -> new DoubleFieldScript(fieldName, params, lookup, onScriptError, ctx) { + @Override + @SuppressWarnings("unchecked") + public void execute() { + Map source = (Map) this.getParams().get("_source"); + for (Object foo : (List) source.get("test")) { + emit(((Number) foo).doubleValue()); + } + }; + }; + } + + @Override + public boolean isParsedFromSource() { + return true; + } + }; case "read_foo" -> (fieldName, params, lookup, onScriptError) -> (ctx) -> new DoubleFieldScript( fieldName, params, diff --git a/server/src/test/java/org/elasticsearch/index/mapper/LongScriptFieldTypeTests.java b/server/src/test/java/org/elasticsearch/index/mapper/LongScriptFieldTypeTests.java index 67b0b47adf4bd..bbb36876aa065 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/LongScriptFieldTypeTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/LongScriptFieldTypeTests.java @@ -374,7 +374,7 @@ public void testBlockLoaderSourceOnlyRuntimeFieldWithSyntheticSource() throws IO } } - private static LuceneDocument createDocumentWithIgnoredSource(String bytes) throws IOException { + static LuceneDocument createDocumentWithIgnoredSource(String bytes) throws IOException { var doc = new LuceneDocument(); var parser = XContentHelper.createParser( XContentParserConfiguration.EMPTY,