Skip to content

[8.x] Include ignored source as part of loading field values in ValueSourceReaderOperator via BlockSourceReader. (#114903) #115064

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 18, 2024
Merged

[8.x] Include ignored source as part of loading field values in ValueSourceReaderOperator via BlockSourceReader. (#114903) #115064

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion ...-extras/src/main/java/org/elasticsearch/index/mapper/extras/MatchOnlyTextFieldMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -364,7 +364,8 @@ public BlockLoader blockLoader(BlockLoaderContext blContext) {
SourceValueFetcher fetcher = SourceValueFetcher.toString(blContext.sourcePaths(name()));
// MatchOnlyText never has norms, so we have to use the field names field
BlockSourceReader.LeafIteratorLookup lookup = BlockSourceReader.lookupFromFieldNames(blContext.fieldNames(), name());
return new BlockSourceReader.BytesRefsBlockLoader(fetcher, lookup);
var sourceMode = blContext.indexSettings().getIndexMappingSourceMode();
return new BlockSourceReader.BytesRefsBlockLoader(fetcher, lookup, sourceMode);
}

@Override
Expand Down
3 changes: 2 additions & 1 deletion ...er-extras/src/main/java/org/elasticsearch/index/mapper/extras/ScaledFloatFieldMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,8 @@ public BlockLoader blockLoader(BlockLoaderContext blContext) {
BlockSourceReader.LeafIteratorLookup lookup = isStored() || isIndexed()
? BlockSourceReader.lookupFromFieldNames(blContext.fieldNames(), name())
: BlockSourceReader.lookupMatchingAll();
return new BlockSourceReader.DoublesBlockLoader(valueFetcher, lookup);
var sourceMode = blContext.indexSettings().getIndexMappingSourceMode();
return new BlockSourceReader.DoublesBlockLoader(valueFetcher, lookup, sourceMode);
}

@Override
Expand Down
3 changes: 2 additions & 1 deletion server/src/main/java/org/elasticsearch/index/mapper/AbstractGeometryFieldMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,8 @@ public BlockLoader blockLoader(BlockLoaderContext blContext) {
protected BlockLoader blockLoaderFromSource(BlockLoaderContext blContext) {
ValueFetcher fetcher = valueFetcher(blContext.sourcePaths(name()), nullValue, GeometryFormatterFactory.WKB);
// TODO consider optimization using BlockSourceReader.lookupFromFieldNames(blContext.fieldNames(), name())
return new BlockSourceReader.GeometriesBlockLoader(fetcher, BlockSourceReader.lookupMatchingAll());
var sourceMode = blContext.indexSettings().getIndexMappingSourceMode();
return new BlockSourceReader.GeometriesBlockLoader(fetcher, BlockSourceReader.lookupMatchingAll(), sourceMode);
}

protected abstract Object nullValueAsSource(T nullValue);
Expand Down
47 changes: 29 additions & 18 deletions server/src/main/java/org/elasticsearch/index/mapper/BlockSourceReader.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,22 @@
import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
import java.util.Set;

/**
* Loads values from {@code _source}. This whole process is very slow and cast-tastic,
* so it doesn't really try to avoid megamorphic invocations. It's just going to be
* slow.
*/
public abstract class BlockSourceReader implements BlockLoader.RowStrideReader {

// _ignored_source is needed when source mode is synthetic.
static final StoredFieldsSpec NEEDS_SOURCE_AND_IGNORED_SOURCE = new StoredFieldsSpec(
true,
false,
Set.of(IgnoredSourceFieldMapper.NAME)
);

private final ValueFetcher fetcher;
private final List<Object> ignoredValues = new ArrayList<>();
private final DocIdSetIterator iter;
Expand Down Expand Up @@ -91,10 +100,12 @@ public interface LeafIteratorLookup {
private abstract static class SourceBlockLoader implements BlockLoader {
protected final ValueFetcher fetcher;
private final LeafIteratorLookup lookup;
private final SourceFieldMapper.Mode sourceMode;

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

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

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

@Override
Expand Down Expand Up @@ -140,8 +151,8 @@ public final String toString() {
* Load {@code boolean}s from {@code _source}.
*/
public static class BooleansBlockLoader extends SourceBlockLoader {
public BooleansBlockLoader(ValueFetcher fetcher, LeafIteratorLookup lookup) {
super(fetcher, lookup);
public BooleansBlockLoader(ValueFetcher fetcher, LeafIteratorLookup lookup, SourceFieldMapper.Mode sourceMode) {
super(fetcher, lookup, sourceMode);
}

@Override
Expand Down Expand Up @@ -180,8 +191,8 @@ public String toString() {
* Load {@link BytesRef}s from {@code _source}.
*/
public static class BytesRefsBlockLoader extends SourceBlockLoader {
public BytesRefsBlockLoader(ValueFetcher fetcher, LeafIteratorLookup lookup) {
super(fetcher, lookup);
public BytesRefsBlockLoader(ValueFetcher fetcher, LeafIteratorLookup lookup, SourceFieldMapper.Mode sourceMode) {
super(fetcher, lookup, sourceMode);
}

@Override
Expand All @@ -191,7 +202,7 @@ public final Builder builder(BlockFactory factory, int expectedCount) {

@Override
protected RowStrideReader rowStrideReader(LeafReaderContext context, DocIdSetIterator iter) throws IOException {
return new BytesRefs(fetcher, iter);
return new BytesRefs(fetcher, iter, null);
}

@Override
Expand All @@ -201,8 +212,8 @@ protected String name() {
}

public static class GeometriesBlockLoader extends SourceBlockLoader {
public GeometriesBlockLoader(ValueFetcher fetcher, LeafIteratorLookup lookup) {
super(fetcher, lookup);
public GeometriesBlockLoader(ValueFetcher fetcher, LeafIteratorLookup lookup, SourceFieldMapper.Mode sourceMode) {
super(fetcher, lookup, sourceMode);
}

@Override
Expand All @@ -212,7 +223,7 @@ public final Builder builder(BlockFactory factory, int expectedCount) {

@Override
protected RowStrideReader rowStrideReader(LeafReaderContext context, DocIdSetIterator iter) {
return new Geometries(fetcher, iter);
return new Geometries(fetcher, iter, null);
}

@Override
Expand All @@ -224,7 +235,7 @@ protected String name() {
private static class BytesRefs extends BlockSourceReader {
private final BytesRef scratch = new BytesRef();

BytesRefs(ValueFetcher fetcher, DocIdSetIterator iter) {
BytesRefs(ValueFetcher fetcher, DocIdSetIterator iter, SourceFieldMapper.Mode sourceMode) {
super(fetcher, iter);
}

Expand All @@ -241,7 +252,7 @@ public String toString() {

private static class Geometries extends BlockSourceReader {

Geometries(ValueFetcher fetcher, DocIdSetIterator iter) {
Geometries(ValueFetcher fetcher, DocIdSetIterator iter, SourceFieldMapper.Mode sourceMode) {
super(fetcher, iter);
}

Expand All @@ -264,8 +275,8 @@ public String toString() {
* Load {@code double}s from {@code _source}.
*/
public static class DoublesBlockLoader extends SourceBlockLoader {
public DoublesBlockLoader(ValueFetcher fetcher, LeafIteratorLookup lookup) {
super(fetcher, lookup);
public DoublesBlockLoader(ValueFetcher fetcher, LeafIteratorLookup lookup, SourceFieldMapper.Mode sourceMode) {
super(fetcher, lookup, sourceMode);
}

@Override
Expand Down Expand Up @@ -304,8 +315,8 @@ public String toString() {
* Load {@code int}s from {@code _source}.
*/
public static class IntsBlockLoader extends SourceBlockLoader {
public IntsBlockLoader(ValueFetcher fetcher, LeafIteratorLookup lookup) {
super(fetcher, lookup);
public IntsBlockLoader(ValueFetcher fetcher, LeafIteratorLookup lookup, SourceFieldMapper.Mode sourceMode) {
super(fetcher, lookup, sourceMode);
}

@Override
Expand Down Expand Up @@ -344,8 +355,8 @@ public String toString() {
* Load {@code long}s from {@code _source}.
*/
public static class LongsBlockLoader extends SourceBlockLoader {
public LongsBlockLoader(ValueFetcher fetcher, LeafIteratorLookup lookup) {
super(fetcher, lookup);
public LongsBlockLoader(ValueFetcher fetcher, LeafIteratorLookup lookup, SourceFieldMapper.Mode sourceMode) {
super(fetcher, lookup, sourceMode);
}

@Override
Expand Down
2 changes: 1 addition & 1 deletion server/src/main/java/org/elasticsearch/index/mapper/BooleanFieldMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,7 @@ public BlockLoader blockLoader(BlockLoaderContext blContext) {
BlockSourceReader.LeafIteratorLookup lookup = isIndexed() || isStored()
? BlockSourceReader.lookupFromFieldNames(blContext.fieldNames(), name())
: BlockSourceReader.lookupMatchingAll();
return new BlockSourceReader.BooleansBlockLoader(fetcher, lookup);
return new BlockSourceReader.BooleansBlockLoader(fetcher, lookup, blContext.indexSettings().getIndexMappingSourceMode());
}

@Override
Expand Down
3 changes: 2 additions & 1 deletion server/src/main/java/org/elasticsearch/index/mapper/DateFieldMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -809,7 +809,8 @@ public BlockLoader blockLoader(BlockLoaderContext blContext) {
BlockSourceReader.LeafIteratorLookup lookup = isStored() || isIndexed()
? BlockSourceReader.lookupFromFieldNames(blContext.fieldNames(), name())
: BlockSourceReader.lookupMatchingAll();
return new BlockSourceReader.LongsBlockLoader(sourceValueFetcher(blContext.sourcePaths(name())), lookup);
var sourceMode = blContext.indexSettings().getIndexMappingSourceMode();
return new BlockSourceReader.LongsBlockLoader(sourceValueFetcher(blContext.sourcePaths(name())), lookup, sourceMode);
}

@Override
Expand Down
12 changes: 3 additions & 9 deletions server/src/main/java/org/elasticsearch/index/mapper/KeywordFieldMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -632,18 +632,12 @@ public BlockLoader blockLoader(BlockLoaderContext blContext) {
if (hasDocValues()) {
return new BlockDocValuesReader.BytesRefsFromOrdsBlockLoader(name());
}
if (isSyntheticSource) {
if (false == isStored()) {
throw new IllegalStateException(
"keyword field ["
+ name()
+ "] is only supported in synthetic _source index if it creates doc values or stored fields"
);
}
if (isStored()) {
return new BlockStoredFieldsReader.BytesFromBytesRefsBlockLoader(name());
}
SourceValueFetcher fetcher = sourceValueFetcher(blContext.sourcePaths(name()));
return new BlockSourceReader.BytesRefsBlockLoader(fetcher, sourceBlockLoaderLookup(blContext));
var sourceMode = blContext.indexSettings().getIndexMappingSourceMode();
return new BlockSourceReader.BytesRefsBlockLoader(fetcher, sourceBlockLoaderLookup(blContext), sourceMode);
}

private BlockSourceReader.LeafIteratorLookup sourceBlockLoaderLookup(BlockLoaderContext blContext) {
Expand Down
65 changes: 49 additions & 16 deletions server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -461,8 +461,12 @@ BlockLoader blockLoaderFromDocValues(String fieldName) {
}

@Override
BlockLoader blockLoaderFromSource(SourceValueFetcher sourceValueFetcher, BlockSourceReader.LeafIteratorLookup lookup) {
return new BlockSourceReader.DoublesBlockLoader(sourceValueFetcher, lookup);
BlockLoader blockLoaderFromSource(
SourceValueFetcher sourceValueFetcher,
BlockSourceReader.LeafIteratorLookup lookup,
SourceFieldMapper.Mode sourceMode
) {
return new BlockSourceReader.DoublesBlockLoader(sourceValueFetcher, lookup, sourceMode);
}
},
FLOAT("float", NumericType.FLOAT) {
Expand Down Expand Up @@ -645,8 +649,12 @@ BlockLoader blockLoaderFromDocValues(String fieldName) {
}

@Override
BlockLoader blockLoaderFromSource(SourceValueFetcher sourceValueFetcher, BlockSourceReader.LeafIteratorLookup lookup) {
return new BlockSourceReader.DoublesBlockLoader(sourceValueFetcher, lookup);
BlockLoader blockLoaderFromSource(
SourceValueFetcher sourceValueFetcher,
BlockSourceReader.LeafIteratorLookup lookup,
SourceFieldMapper.Mode sourceMode
) {
return new BlockSourceReader.DoublesBlockLoader(sourceValueFetcher, lookup, sourceMode);
}
},
DOUBLE("double", NumericType.DOUBLE) {
Expand Down Expand Up @@ -795,8 +803,12 @@ BlockLoader blockLoaderFromDocValues(String fieldName) {
}

@Override
BlockLoader blockLoaderFromSource(SourceValueFetcher sourceValueFetcher, BlockSourceReader.LeafIteratorLookup lookup) {
return new BlockSourceReader.DoublesBlockLoader(sourceValueFetcher, lookup);
BlockLoader blockLoaderFromSource(
SourceValueFetcher sourceValueFetcher,
BlockSourceReader.LeafIteratorLookup lookup,
SourceFieldMapper.Mode sourceMode
) {
return new BlockSourceReader.DoublesBlockLoader(sourceValueFetcher, lookup, sourceMode);
}
},
BYTE("byte", NumericType.BYTE) {
Expand Down Expand Up @@ -908,8 +920,12 @@ BlockLoader blockLoaderFromDocValues(String fieldName) {
}

@Override
BlockLoader blockLoaderFromSource(SourceValueFetcher sourceValueFetcher, BlockSourceReader.LeafIteratorLookup lookup) {
return new BlockSourceReader.IntsBlockLoader(sourceValueFetcher, lookup);
BlockLoader blockLoaderFromSource(
SourceValueFetcher sourceValueFetcher,
BlockSourceReader.LeafIteratorLookup lookup,
SourceFieldMapper.Mode sourceMode
) {
return new BlockSourceReader.IntsBlockLoader(sourceValueFetcher, lookup, sourceMode);
}

private boolean isOutOfRange(Object value) {
Expand Down Expand Up @@ -1021,8 +1037,12 @@ BlockLoader blockLoaderFromDocValues(String fieldName) {
}

@Override
BlockLoader blockLoaderFromSource(SourceValueFetcher sourceValueFetcher, BlockSourceReader.LeafIteratorLookup lookup) {
return new BlockSourceReader.IntsBlockLoader(sourceValueFetcher, lookup);
BlockLoader blockLoaderFromSource(
SourceValueFetcher sourceValueFetcher,
BlockSourceReader.LeafIteratorLookup lookup,
SourceFieldMapper.Mode sourceMode
) {
return new BlockSourceReader.IntsBlockLoader(sourceValueFetcher, lookup, sourceMode);
}

private boolean isOutOfRange(Object value) {
Expand Down Expand Up @@ -1208,8 +1228,12 @@ BlockLoader blockLoaderFromDocValues(String fieldName) {
}

@Override
BlockLoader blockLoaderFromSource(SourceValueFetcher sourceValueFetcher, BlockSourceReader.LeafIteratorLookup lookup) {
return new BlockSourceReader.IntsBlockLoader(sourceValueFetcher, lookup);
BlockLoader blockLoaderFromSource(
SourceValueFetcher sourceValueFetcher,
BlockSourceReader.LeafIteratorLookup lookup,
SourceFieldMapper.Mode sourceMode
) {
return new BlockSourceReader.IntsBlockLoader(sourceValueFetcher, lookup, sourceMode);
}
},
LONG("long", NumericType.LONG) {
Expand Down Expand Up @@ -1355,8 +1379,12 @@ BlockLoader blockLoaderFromDocValues(String fieldName) {
}

@Override
BlockLoader blockLoaderFromSource(SourceValueFetcher sourceValueFetcher, BlockSourceReader.LeafIteratorLookup lookup) {
return new BlockSourceReader.LongsBlockLoader(sourceValueFetcher, lookup);
BlockLoader blockLoaderFromSource(
SourceValueFetcher sourceValueFetcher,
BlockSourceReader.LeafIteratorLookup lookup,
SourceFieldMapper.Mode sourceMode
) {
return new BlockSourceReader.LongsBlockLoader(sourceValueFetcher, lookup, sourceMode);
}

private boolean isOutOfRange(Object value) {
Expand Down Expand Up @@ -1634,7 +1662,11 @@ protected void writeValue(XContentBuilder b, long value) throws IOException {

abstract BlockLoader blockLoaderFromDocValues(String fieldName);

abstract BlockLoader blockLoaderFromSource(SourceValueFetcher sourceValueFetcher, BlockSourceReader.LeafIteratorLookup lookup);
abstract BlockLoader blockLoaderFromSource(
SourceValueFetcher sourceValueFetcher,
BlockSourceReader.LeafIteratorLookup lookup,
SourceFieldMapper.Mode sourceMode
);
}

public static class NumberFieldType extends SimpleMappedFieldType {
Expand Down Expand Up @@ -1773,7 +1805,8 @@ public BlockLoader blockLoader(BlockLoaderContext blContext) {
BlockSourceReader.LeafIteratorLookup lookup = isStored() || isIndexed()
? BlockSourceReader.lookupFromFieldNames(blContext.fieldNames(), name())
: BlockSourceReader.lookupMatchingAll();
return type.blockLoaderFromSource(sourceValueFetcher(blContext.sourcePaths(name())), lookup);
var sourceMode = blContext.indexSettings().getIndexMappingSourceMode();
return type.blockLoaderFromSource(sourceValueFetcher(blContext.sourcePaths(name())), lookup, sourceMode);
}

@Override
Expand Down
9 changes: 6 additions & 3 deletions server/src/main/java/org/elasticsearch/index/mapper/TextFieldMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1012,17 +1012,20 @@ protected String delegatingTo() {
if (isStored()) {
return new BlockStoredFieldsReader.BytesFromStringsBlockLoader(name());
}
if (isSyntheticSource) {
if (isSyntheticSource && syntheticSourceDelegate == null) {
/*
* When we're in synthetic source mode we don't currently
* support text fields that are not stored and are not children
* of perfect keyword fields. We'd have to load from the parent
* field and then convert the result to a string.
* field and then convert the result to a string. In this case,
* even if we would synthesize the source, the current field
* would be missing.
*/
return null;
}
SourceValueFetcher fetcher = SourceValueFetcher.toString(blContext.sourcePaths(name()));
return new BlockSourceReader.BytesRefsBlockLoader(fetcher, blockReaderDisiLookup(blContext));
var sourceMode = blContext.indexSettings().getIndexMappingSourceMode();
return new BlockSourceReader.BytesRefsBlockLoader(fetcher, blockReaderDisiLookup(blContext), sourceMode);
}

/**
Expand Down
2 changes: 1 addition & 1 deletion server/src/test/java/org/elasticsearch/index/mapper/BlockSourceReaderTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public void testEmptyArray() throws IOException {
private void loadBlock(LeafReaderContext ctx, Consumer<TestBlock> test) throws IOException {
ValueFetcher valueFetcher = SourceValueFetcher.toString(Set.of("field"));
BlockSourceReader.LeafIteratorLookup lookup = BlockSourceReader.lookupFromNorms("field");
BlockLoader loader = new BlockSourceReader.BytesRefsBlockLoader(valueFetcher, lookup);
BlockLoader loader = new BlockSourceReader.BytesRefsBlockLoader(valueFetcher, lookup, null);
assertThat(loader.columnAtATimeReader(ctx), nullValue());
BlockLoader.RowStrideReader reader = loader.rowStrideReader(ctx);
assertThat(loader.rowStrideStoredFieldSpec(), equalTo(StoredFieldsSpec.NEEDS_SOURCE));
Expand Down
7 changes: 5 additions & 2 deletions test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1337,12 +1337,15 @@ private BlockLoader getBlockLoader(boolean columnReader) {
return mapper.fieldType(loaderFieldName).blockLoader(new MappedFieldType.BlockLoaderContext() {
@Override
public String indexName() {
throw new UnsupportedOperationException();
return "test_index";
}

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

@Override
Expand Down
Loading