Skip to content

Commit

Permalink
Synthetic source and ignore above in source block loaders.
Browse files Browse the repository at this point in the history
Handle ignore_above in source BlockLoader correctly when synthetic source is enabled.
This done by loading that *.original stored field that keyword fields introduce if a value is longer than specified ignore_above threshold.

Closes elastic#115076
  • Loading branch information
martijnvg committed Oct 18, 2024
1 parent 906bf46 commit 9173205
Show file tree
Hide file tree
Showing 7 changed files with 48 additions and 29 deletions.
12 changes: 0 additions & 12 deletions muted-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -350,18 +350,6 @@ tests:
- class: org.elasticsearch.upgrades.MultiVersionRepositoryAccessIT
method: testCreateAndRestoreSnapshot
issue: https://github.com/elastic/elasticsearch/issues/114998
- class: org.elasticsearch.index.mapper.TextFieldMapperTests
method: testBlockLoaderFromRowStrideReaderWithSyntheticSource
issue: https://github.com/elastic/elasticsearch/issues/115066
- class: org.elasticsearch.index.mapper.TextFieldMapperTests
method: testBlockLoaderFromColumnReaderWithSyntheticSource
issue: https://github.com/elastic/elasticsearch/issues/115073
- class: org.elasticsearch.index.mapper.annotatedtext.AnnotatedTextFieldMapperTests
method: testBlockLoaderFromColumnReaderWithSyntheticSource
issue: https://github.com/elastic/elasticsearch/issues/115074
- class: org.elasticsearch.index.mapper.annotatedtext.AnnotatedTextFieldMapperTests
method: testBlockLoaderFromRowStrideReaderWithSyntheticSource
issue: https://github.com/elastic/elasticsearch/issues/115076

# Examples:
#
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,12 +100,25 @@ public interface LeafIteratorLookup {
private abstract static class SourceBlockLoader implements BlockLoader {
protected final ValueFetcher fetcher;
private final LeafIteratorLookup lookup;
private final SourceFieldMapper.Mode sourceMode;
private final StoredFieldsSpec storedFieldsSpec;

private SourceBlockLoader(ValueFetcher fetcher, LeafIteratorLookup lookup, SourceFieldMapper.Mode sourceMode) {
private SourceBlockLoader(ValueFetcher fetcher, LeafIteratorLookup lookup, StoredFieldsSpec storedFieldsSpec) {
this.fetcher = fetcher;
this.lookup = lookup;
this.sourceMode = sourceMode;
this.storedFieldsSpec = storedFieldsSpec;
}

private SourceBlockLoader(ValueFetcher fetcher, LeafIteratorLookup lookup, SourceFieldMapper.Mode sourceMode) {
this(
fetcher,
lookup,
sourceMode == SourceFieldMapper.Mode.SYNTHETIC ? NEEDS_SOURCE_AND_IGNORED_SOURCE : StoredFieldsSpec.NEEDS_SOURCE
);
}

// Assumes synthetic source only
private SourceBlockLoader(ValueFetcher fetcher, LeafIteratorLookup lookup, String additionalField) {
this(fetcher, lookup, NEEDS_SOURCE_AND_IGNORED_SOURCE.merge(new StoredFieldsSpec(true, false, Set.of(additionalField))));
}

@Override
Expand All @@ -115,7 +128,7 @@ public final ColumnAtATimeReader columnAtATimeReader(LeafReaderContext context)

@Override
public final StoredFieldsSpec rowStrideStoredFieldSpec() {
return sourceMode == SourceFieldMapper.Mode.SYNTHETIC ? NEEDS_SOURCE_AND_IGNORED_SOURCE : StoredFieldsSpec.NEEDS_SOURCE;
return storedFieldsSpec;
}

@Override
Expand Down Expand Up @@ -195,6 +208,10 @@ public BytesRefsBlockLoader(ValueFetcher fetcher, LeafIteratorLookup lookup, Sou
super(fetcher, lookup, sourceMode);
}

public BytesRefsBlockLoader(SourceValueFetcher fetcher, LeafIteratorLookup lookup, String originalFieldName) {
super(fetcher, lookup, originalFieldName);
}

@Override
public final Builder builder(BlockFactory factory, int expectedCount) {
return factory.bytesRefs(expectedCount);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1025,7 +1025,12 @@ protected String delegatingTo() {
}
SourceValueFetcher fetcher = SourceValueFetcher.toString(blContext.sourcePaths(name()));
var sourceMode = blContext.indexSettings().getIndexMappingSourceMode();
return new BlockSourceReader.BytesRefsBlockLoader(fetcher, blockReaderDisiLookup(blContext), sourceMode);
if (sourceMode == SourceFieldMapper.Mode.SYNTHETIC && syntheticSourceDelegate().ignoreAbove() != Integer.MAX_VALUE) {
String originalName = syntheticSourceDelegate().name() + "._original";
return new BlockSourceReader.BytesRefsBlockLoader(fetcher, blockReaderDisiLookup(blContext), originalName);
} else {
return new BlockSourceReader.BytesRefsBlockLoader(fetcher, blockReaderDisiLookup(blContext), sourceMode);
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1353,6 +1353,7 @@ private void testBlockLoaderFromParent(boolean columnReader, boolean syntheticSo
};
MapperService mapper = createMapperService(syntheticSource ? syntheticSourceMapping(buildFields) : mapping(buildFields));
BlockReaderSupport blockReaderSupport = getSupportedReaders(mapper, "field.sub");
testBlockLoader(columnReader, example, blockReaderSupport);
var sourceLoader = mapper.mappingLookup().newSourceLoader(SourceFieldMetrics.NOOP);
testBlockLoader(columnReader, example, blockReaderSupport, sourceLoader);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,11 @@ public final MapperService createMapperService(XContentBuilder mappings) throws
return createMapperService(getVersion(), mappings);
}

public final MapperService createSytheticSourceMapperService(XContentBuilder mappings) throws IOException {
var settings = Settings.builder().put("index.mapping.source.mode", "synthetic").build();
return createMapperService(getVersion(), settings, () -> true, mappings);
}

protected IndexVersion getVersion() {
return IndexVersion.current();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1321,15 +1321,12 @@ private BlockLoader getBlockLoader(boolean columnReader) {
return mapper.fieldType(loaderFieldName).blockLoader(new MappedFieldType.BlockLoaderContext() {
@Override
public String indexName() {
return "test_index";
return mapper.getIndexSettings().getIndex().getName();
}

@Override
public IndexSettings indexSettings() {
var imd = IndexMetadata.builder(indexName())
.settings(MapperTestCase.indexSettings(IndexVersion.current(), 1, 1).put(Settings.EMPTY))
.build();
return new IndexSettings(imd, Settings.EMPTY);
return mapper.getIndexSettings();
}

@Override
Expand Down Expand Up @@ -1363,8 +1360,9 @@ public FieldNamesFieldMapper.FieldNamesFieldType fieldNames() {
private void testBlockLoader(boolean syntheticSource, boolean columnReader) throws IOException {
// TODO if we're not using synthetic source use a different sort of example. Or something.
SyntheticSourceExample example = syntheticSourceSupport(false, columnReader).example(5);
// TODO: only rely index.mapping.source.mode setting
XContentBuilder mapping = syntheticSource ? syntheticSourceFieldMapping(example.mapping) : fieldMapping(example.mapping);
MapperService mapper = createMapperService(mapping);
MapperService mapper = syntheticSource ? createSytheticSourceMapperService(mapping) : createMapperService(mapping);
BlockReaderSupport blockReaderSupport = getSupportedReaders(mapper, "field");
if (syntheticSource) {
// geo_point and point do not yet support synthetic source
Expand All @@ -1373,11 +1371,16 @@ private void testBlockLoader(boolean syntheticSource, boolean columnReader) thro
blockReaderSupport.syntheticSource
);
}
testBlockLoader(columnReader, example, blockReaderSupport);
var sourceLoader = mapper.mappingLookup().newSourceLoader(SourceFieldMetrics.NOOP);
testBlockLoader(columnReader, example, blockReaderSupport, sourceLoader);
}

protected final void testBlockLoader(boolean columnReader, SyntheticSourceExample example, BlockReaderSupport blockReaderSupport)
throws IOException {
protected final void testBlockLoader(
boolean columnReader,
SyntheticSourceExample example,
BlockReaderSupport blockReaderSupport,
SourceLoader sourceLoader
) throws IOException {
BlockLoader loader = blockReaderSupport.getBlockLoader(columnReader);
Function<Object, Object> valuesConvert = loadBlockExpected(blockReaderSupport, columnReader);
if (valuesConvert == null) {
Expand Down Expand Up @@ -1406,7 +1409,7 @@ protected final void testBlockLoader(boolean columnReader, SyntheticSourceExampl
} else {
BlockLoaderStoredFieldsFromLeafLoader storedFieldsLoader = new BlockLoaderStoredFieldsFromLeafLoader(
StoredFieldLoader.fromSpec(loader.rowStrideStoredFieldSpec()).getLoader(ctx, null),
loader.rowStrideStoredFieldSpec().requiresSource() ? SourceLoader.FROM_STORED_SOURCE.leaf(ctx.reader(), null) : null
loader.rowStrideStoredFieldSpec().requiresSource() ? sourceLoader.leaf(ctx.reader(), null) : null
);
storedFieldsLoader.advanceTo(0);
BlockLoader.Builder builder = loader.builder(TestBlock.factory(ctx.reader().numDocs()), 1);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ public static Function<Object, Object> loadBlockExpected(MapperTestCase.BlockRea
private static boolean nullLoaderExpected(MapperService mapper, String fieldName) {
MappedFieldType type = mapper.fieldType(fieldName);
if (type instanceof TextFieldMapper.TextFieldType t) {
if (t.isSyntheticSource() == false || t.canUseSyntheticSourceDelegateForQuerying() || t.isStored()) {
if (t.isSyntheticSource() == false || t.syntheticSourceDelegate() != null || t.isStored()) {
return false;
}
String parentField = mapper.mappingLookup().parentField(fieldName);
Expand Down

0 comments on commit 9173205

Please sign in to comment.