Skip to content

Commit 0022605

Browse files
jordan-powersjordan-powers
authored
Improve block loader for source only runtime fields of type keyword (elastic#135026)
This patch updates source-only keyword scripts to use the FallbackSyntheticSourceBlockLoader instead of generic KeywordScriptBlockLoader to retrieve the values out of ignored_source when synthetic source is enabled. Relates to elastic#134121
1 parent 2041de3 commit 0022605

File tree

9 files changed

+187
-31
lines changed

9 files changed

+187
-31
lines changed

‎docs/changelog/135026.yaml‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
pr: 135026
2+
summary: Improve block loader for source only runtime fields of type keyword
3+
area: Mapping
4+
type: enhancement
5+
issues: []

‎server/src/main/java/org/elasticsearch/index/mapper/AbstractScriptFieldType.java‎

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
import java.util.function.BiConsumer;
4141
import java.util.function.BiFunction;
4242
import java.util.function.Function;
43+
import java.util.function.Supplier;
4344

4445
import static org.elasticsearch.search.SearchService.ALLOW_EXPENSIVE_QUERIES;
4546

@@ -205,15 +206,32 @@ protected final LeafFactory leafFactory(SearchLookup searchLookup) {
205206
}
206207
}
207208

209+
protected final FallbackSyntheticSourceBlockLoader numericFallbackSyntheticSourceBlockLoader(
210+
BlockLoaderContext blContext,
211+
NumberFieldMapper.NumberType numberType,
212+
BiFunction<BlockLoader.BlockFactory, Integer, BlockLoader.Builder> builderSupplier,
213+
BiConsumer<List<Number>, BlockLoader.Builder> writeToBlock
214+
) {
215+
return fallbackSyntheticSourceBlockLoader(
216+
blContext,
217+
builderSupplier,
218+
() -> new NumberFieldMapper.NumberType.NumberFallbackSyntheticSourceReader(numberType, null, true) {
219+
@Override
220+
public void writeToBlock(List<Number> values, BlockLoader.Builder blockBuilder) {
221+
writeToBlock.accept(values, blockBuilder);
222+
}
223+
}
224+
);
225+
}
226+
208227
/**
209228
* Returns synthetic source fallback block loader if source mode is synthetic, runtime field is source only and field is only mapped
210229
* as a runtime field.
211230
*/
212231
protected final FallbackSyntheticSourceBlockLoader fallbackSyntheticSourceBlockLoader(
213232
BlockLoaderContext blContext,
214-
NumberFieldMapper.NumberType numberType,
215233
BiFunction<BlockLoader.BlockFactory, Integer, BlockLoader.Builder> builderSupplier,
216-
BiConsumer<List<Number>, BlockLoader.Builder> writeToBlock
234+
Supplier<FallbackSyntheticSourceBlockLoader.Reader<?>> readerSupplier
217235
) {
218236
var indexSettings = blContext.indexSettings();
219237
// A runtime and normal field can share the same name.
@@ -222,12 +240,7 @@ protected final FallbackSyntheticSourceBlockLoader fallbackSyntheticSourceBlockL
222240
if (isParsedFromSource
223241
&& indexSettings.getIndexMappingSourceMode() == SourceFieldMapper.Mode.SYNTHETIC
224242
&& blContext.lookup().onlyMappedAsRuntimeField(name())) {
225-
var reader = new NumberFieldMapper.NumberType.NumberFallbackSyntheticSourceReader(numberType, null, true) {
226-
@Override
227-
public void writeToBlock(List<Number> values, BlockLoader.Builder blockBuilder) {
228-
writeToBlock.accept(values, blockBuilder);
229-
}
230-
};
243+
var reader = readerSupplier.get();
231244

232245
return new FallbackSyntheticSourceBlockLoader(
233246
reader,

‎server/src/main/java/org/elasticsearch/index/mapper/DoubleScriptFieldType.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ public DocValueFormat docValueFormat(String format, ZoneId timeZone) {
109109

110110
@Override
111111
public BlockLoader blockLoader(BlockLoaderContext blContext) {
112-
var fallbackSyntheticSourceBlockLoader = fallbackSyntheticSourceBlockLoader(
112+
var fallbackSyntheticSourceBlockLoader = numericFallbackSyntheticSourceBlockLoader(
113113
blContext,
114114
NumberType.DOUBLE,
115115
BlockLoader.BlockFactory::doubles,

‎server/src/main/java/org/elasticsearch/index/mapper/KeywordScriptFieldType.java‎

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,12 @@
3333
import org.elasticsearch.search.runtime.StringScriptFieldTermQuery;
3434
import org.elasticsearch.search.runtime.StringScriptFieldTermsQuery;
3535
import org.elasticsearch.search.runtime.StringScriptFieldWildcardQuery;
36+
import org.elasticsearch.xcontent.XContentParser;
3637

38+
import java.io.IOException;
3739
import java.time.ZoneId;
3840
import java.util.Collection;
41+
import java.util.List;
3942
import java.util.Map;
4043
import java.util.Objects;
4144
import java.util.Set;
@@ -112,9 +115,44 @@ public Object valueForDisplay(Object value) {
112115
return binaryValue.utf8ToString();
113116
}
114117

118+
private FallbackSyntheticSourceBlockLoader.Reader<?> fallbackSyntheticSourceBlockLoaderReader() {
119+
return new FallbackSyntheticSourceBlockLoader.SingleValueReader<BytesRef>(null) {
120+
@Override
121+
public void convertValue(Object value, List<BytesRef> accumulator) {
122+
accumulator.add((BytesRef) value);
123+
}
124+
125+
@Override
126+
public void parseNonNullValue(XContentParser parser, List<BytesRef> accumulator) throws IOException {
127+
assert parser.currentToken() == XContentParser.Token.VALUE_STRING : "Unexpected token " + parser.currentToken();
128+
129+
var utfBytes = parser.optimizedText().bytes();
130+
accumulator.add(new BytesRef(utfBytes.bytes(), utfBytes.offset(), utfBytes.length()));
131+
}
132+
133+
@Override
134+
public void writeToBlock(List<BytesRef> values, BlockLoader.Builder blockBuilder) {
135+
var bytesRefBuilder = (BlockLoader.BytesRefBuilder) blockBuilder;
136+
137+
for (var value : values) {
138+
bytesRefBuilder.appendBytesRef(value);
139+
}
140+
}
141+
};
142+
}
143+
115144
@Override
116145
public BlockLoader blockLoader(BlockLoaderContext blContext) {
117-
return new KeywordScriptBlockDocValuesReader.KeywordScriptBlockLoader(leafFactory(blContext.lookup()));
146+
var fallbackSyntheticSourceBlockLoader = fallbackSyntheticSourceBlockLoader(
147+
blContext,
148+
BlockLoader.BlockFactory::bytesRefs,
149+
this::fallbackSyntheticSourceBlockLoaderReader
150+
);
151+
if (fallbackSyntheticSourceBlockLoader != null) {
152+
return fallbackSyntheticSourceBlockLoader;
153+
} else {
154+
return new KeywordScriptBlockDocValuesReader.KeywordScriptBlockLoader(leafFactory(blContext.lookup()));
155+
}
118156
}
119157

120158
@Override

‎server/src/main/java/org/elasticsearch/index/mapper/LongScriptFieldType.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ public DocValueFormat docValueFormat(String format, ZoneId timeZone) {
109109

110110
@Override
111111
public BlockLoader blockLoader(BlockLoaderContext blContext) {
112-
var fallbackSyntheticSourceBlockLoader = fallbackSyntheticSourceBlockLoader(
112+
var fallbackSyntheticSourceBlockLoader = numericFallbackSyntheticSourceBlockLoader(
113113
blContext,
114114
NumberType.LONG,
115115
BlockLoader.BlockFactory::longs,

‎server/src/test/java/org/elasticsearch/index/mapper/DoubleScriptFieldTypeTests.java‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,6 @@
4949
import java.util.Map;
5050

5151
import static java.util.Collections.emptyMap;
52-
import static org.elasticsearch.index.mapper.LongScriptFieldTypeTests.createDocumentWithIgnoredSource;
5352
import static org.hamcrest.Matchers.containsInAnyOrder;
5453
import static org.hamcrest.Matchers.equalTo;
5554
import static org.hamcrest.Matchers.instanceOf;

‎server/src/test/java/org/elasticsearch/index/mapper/KeywordScriptFieldTypeTests.java‎

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import org.apache.lucene.util.BytesRef;
3030
import org.apache.lucene.util.automaton.Operations;
3131
import org.elasticsearch.common.lucene.search.function.ScriptScoreQuery;
32+
import org.elasticsearch.common.settings.Settings;
3233
import org.elasticsearch.common.unit.Fuzziness;
3334
import org.elasticsearch.index.IndexVersion;
3435
import org.elasticsearch.index.fielddata.BinaryScriptFieldData;
@@ -44,6 +45,7 @@
4445
import org.elasticsearch.script.ScriptType;
4546
import org.elasticsearch.script.StringFieldScript;
4647
import org.elasticsearch.search.MultiValueMode;
48+
import org.elasticsearch.search.lookup.SearchLookup;
4749

4850
import java.io.IOException;
4951
import java.util.ArrayList;
@@ -53,6 +55,8 @@
5355
import static java.util.Collections.emptyMap;
5456
import static org.hamcrest.Matchers.containsInAnyOrder;
5557
import static org.hamcrest.Matchers.equalTo;
58+
import static org.hamcrest.Matchers.instanceOf;
59+
import static org.hamcrest.Matchers.nullValue;
5660

5761
public class KeywordScriptFieldTypeTests extends AbstractScriptFieldTypeTestCase {
5862

@@ -421,6 +425,105 @@ public void testBlockLoader() throws IOException {
421425
}
422426
}
423427

428+
public void testBlockLoaderSourceOnlyRuntimeField() throws IOException {
429+
try (
430+
Directory directory = newDirectory();
431+
RandomIndexWriter iw = new RandomIndexWriter(random(), directory, newIndexWriterConfig().setMergePolicy(NoMergePolicy.INSTANCE))
432+
) {
433+
iw.addDocuments(
434+
List.of(
435+
List.of(new StoredField("_source", new BytesRef("{\"test\": [\"cat\"]}"))),
436+
List.of(new StoredField("_source", new BytesRef("{\"test\": [\"dog\"]}")))
437+
)
438+
);
439+
try (DirectoryReader reader = iw.getReader()) {
440+
KeywordScriptFieldType fieldType = simpleSourceOnlyMappedFieldType();
441+
442+
// Assert implementations:
443+
BlockLoader loader = fieldType.blockLoader(blContext(Settings.EMPTY, true));
444+
assertThat(loader, instanceOf(KeywordScriptBlockDocValuesReader.KeywordScriptBlockLoader.class));
445+
// ignored source doesn't support column at a time loading:
446+
var columnAtATimeLoader = loader.columnAtATimeReader(reader.leaves().getFirst());
447+
assertThat(columnAtATimeLoader, instanceOf(KeywordScriptBlockDocValuesReader.class));
448+
var rowStrideReader = loader.rowStrideReader(reader.leaves().getFirst());
449+
assertThat(rowStrideReader, instanceOf(KeywordScriptBlockDocValuesReader.class));
450+
451+
var catBytes = new BytesRef("cat");
452+
var dogBytes = new BytesRef("dog");
453+
454+
// Assert values:
455+
assertThat(blockLoaderReadValuesFromColumnAtATimeReader(reader, fieldType, 0), equalTo(List.of(catBytes, dogBytes)));
456+
assertThat(blockLoaderReadValuesFromColumnAtATimeReader(reader, fieldType, 1), equalTo(List.of(dogBytes)));
457+
assertThat(blockLoaderReadValuesFromRowStrideReader(reader, fieldType), equalTo(List.of(catBytes, dogBytes)));
458+
}
459+
}
460+
}
461+
462+
public void testBlockLoaderSourceOnlyRuntimeFieldWithSyntheticSource() throws IOException {
463+
var settings = Settings.builder().put("index.mapping.source.mode", "synthetic").build();
464+
try (
465+
Directory directory = newDirectory();
466+
RandomIndexWriter iw = new RandomIndexWriter(random(), directory, newIndexWriterConfig().setMergePolicy(NoMergePolicy.INSTANCE))
467+
) {
468+
469+
var document1 = createDocumentWithIgnoredSource("[\"cat\"]");
470+
var document2 = createDocumentWithIgnoredSource("[\"dog\"]");
471+
472+
iw.addDocuments(List.of(document1, document2));
473+
try (DirectoryReader reader = iw.getReader()) {
474+
KeywordScriptFieldType fieldType = simpleSourceOnlyMappedFieldType();
475+
476+
// Assert implementations:
477+
BlockLoader loader = fieldType.blockLoader(blContext(settings, true));
478+
assertThat(loader, instanceOf(FallbackSyntheticSourceBlockLoader.class));
479+
// ignored source doesn't support column at a time loading:
480+
var columnAtATimeLoader = loader.columnAtATimeReader(reader.leaves().getFirst());
481+
assertThat(columnAtATimeLoader, nullValue());
482+
var rowStrideReader = loader.rowStrideReader(reader.leaves().getFirst());
483+
assertThat(
484+
rowStrideReader.getClass().getName(),
485+
equalTo("org.elasticsearch.index.mapper.FallbackSyntheticSourceBlockLoader$IgnoredSourceRowStrideReader")
486+
);
487+
488+
// Assert values:
489+
assertThat(
490+
blockLoaderReadValuesFromRowStrideReader(settings, reader, fieldType, true),
491+
equalTo(List.of(new BytesRef("cat"), new BytesRef("dog")))
492+
);
493+
}
494+
}
495+
}
496+
497+
private KeywordScriptFieldType simpleSourceOnlyMappedFieldType() {
498+
Script script = new Script(ScriptType.INLINE, "test", "", emptyMap());
499+
StringFieldScript.Factory factory = new StringFieldScript.Factory() {
500+
@Override
501+
public StringFieldScript.LeafFactory newFactory(
502+
String fieldName,
503+
Map<String, Object> params,
504+
SearchLookup searchLookup,
505+
OnScriptError onScriptError
506+
) {
507+
return ctx -> new StringFieldScript(fieldName, params, searchLookup, onScriptError, ctx) {
508+
@Override
509+
@SuppressWarnings("unchecked")
510+
public void execute() {
511+
Map<String, Object> source = (Map<String, Object>) this.getParams().get("_source");
512+
for (Object foo : (List<?>) source.get("test")) {
513+
emit((String) foo);
514+
}
515+
}
516+
};
517+
}
518+
519+
@Override
520+
public boolean isParsedFromSource() {
521+
return true;
522+
}
523+
};
524+
return new KeywordScriptFieldType("test", factory, script, emptyMap(), OnScriptError.FAIL);
525+
}
526+
424527
@Override
425528
protected KeywordScriptFieldType simpleMappedFieldType() {
426529
return build("read_foo", Map.of(), OnScriptError.FAIL);

‎server/src/test/java/org/elasticsearch/index/mapper/LongScriptFieldTypeTests.java‎

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -29,11 +29,9 @@
2929
import org.apache.lucene.store.Directory;
3030
import org.apache.lucene.tests.index.RandomIndexWriter;
3131
import org.apache.lucene.util.BytesRef;
32-
import org.elasticsearch.common.bytes.BytesArray;
3332
import org.elasticsearch.common.geo.ShapeRelation;
3433
import org.elasticsearch.common.lucene.search.function.ScriptScoreQuery;
3534
import org.elasticsearch.common.settings.Settings;
36-
import org.elasticsearch.common.xcontent.XContentHelper;
3735
import org.elasticsearch.index.IndexVersion;
3836
import org.elasticsearch.index.fielddata.LongScriptFieldData;
3937
import org.elasticsearch.index.fielddata.ScriptDocValues;
@@ -46,9 +44,6 @@
4644
import org.elasticsearch.script.ScriptType;
4745
import org.elasticsearch.search.MultiValueMode;
4846
import org.elasticsearch.search.lookup.SearchLookup;
49-
import org.elasticsearch.xcontent.XContentFactory;
50-
import org.elasticsearch.xcontent.XContentParserConfiguration;
51-
import org.elasticsearch.xcontent.XContentType;
5247

5348
import java.io.IOException;
5449
import java.util.ArrayList;
@@ -374,20 +369,6 @@ public void testBlockLoaderSourceOnlyRuntimeFieldWithSyntheticSource() throws IO
374369
}
375370
}
376371

377-
static LuceneDocument createDocumentWithIgnoredSource(String bytes) throws IOException {
378-
var doc = new LuceneDocument();
379-
var parser = XContentHelper.createParser(
380-
XContentParserConfiguration.EMPTY,
381-
new BytesArray(bytes),
382-
XContentFactory.xContent(XContentType.JSON).type()
383-
);
384-
parser.nextToken();
385-
var nameValue = new IgnoredSourceFieldMapper.NameValue("test", 0, XContentDataHelper.encodeToken(parser), doc);
386-
var ignoredSourceFormat = IgnoredSourceFieldMapper.ignoredSourceFormat(IndexVersion.current());
387-
ignoredSourceFormat.writeIgnoredFields(List.of(nameValue));
388-
return doc;
389-
}
390-
391372
@Override
392373
protected Query randomTermsQuery(MappedFieldType ft, SearchExecutionContext ctx) {
393374
return ft.termsQuery(List.of(randomLong()), ctx);

‎test/framework/src/main/java/org/elasticsearch/index/mapper/AbstractScriptFieldTypeTestCase.java‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import org.elasticsearch.ElasticsearchException;
2525
import org.elasticsearch.cluster.metadata.IndexMetadata;
2626
import org.elasticsearch.common.Strings;
27+
import org.elasticsearch.common.bytes.BytesArray;
2728
import org.elasticsearch.common.bytes.BytesReference;
2829
import org.elasticsearch.common.geo.ShapeRelation;
2930
import org.elasticsearch.common.settings.Settings;
@@ -46,6 +47,8 @@
4647
import org.elasticsearch.xcontent.ToXContent;
4748
import org.elasticsearch.xcontent.XContentBuilder;
4849
import org.elasticsearch.xcontent.XContentFactory;
50+
import org.elasticsearch.xcontent.XContentParserConfiguration;
51+
import org.elasticsearch.xcontent.XContentType;
4952
import org.elasticsearch.xcontent.json.JsonXContent;
5053

5154
import java.io.IOException;
@@ -621,4 +624,18 @@ protected <T> T compileScript(Script script, ScriptContext<T> context) {
621624
boolean deterministicSource = "deterministic_source".equals(script.getIdOrCode());
622625
return deterministicSource ? (T) parseFromSource() : (T) dummyScript();
623626
}
627+
628+
protected static LuceneDocument createDocumentWithIgnoredSource(String bytes) throws IOException {
629+
var doc = new LuceneDocument();
630+
var parser = XContentHelper.createParser(
631+
XContentParserConfiguration.EMPTY,
632+
new BytesArray(bytes),
633+
XContentFactory.xContent(XContentType.JSON).type()
634+
);
635+
parser.nextToken();
636+
var nameValue = new IgnoredSourceFieldMapper.NameValue("test", 0, XContentDataHelper.encodeToken(parser), doc);
637+
var ignoredSourceFormat = IgnoredSourceFieldMapper.ignoredSourceFormat(IndexVersion.current());
638+
ignoredSourceFormat.writeIgnoredFields(List.of(nameValue));
639+
return doc;
640+
}
624641
}

0 commit comments

Comments
 (0)