Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
6652523
BlockSourceReader should apply source filtering
martijnvg Oct 10, 2025
2a849b9
iter
martijnvg Oct 12, 2025
0dbadd4
[CI] Auto commit changes from spotless
Oct 12, 2025
b65a397
iter2
martijnvg Oct 12, 2025
07ddeae
[CI] Auto commit changes from spotless
Oct 12, 2025
d68d873
iter4
martijnvg Oct 12, 2025
f055a87
cleanup
martijnvg Oct 12, 2025
fa845b6
[CI] Auto commit changes from spotless
Oct 12, 2025
f435afd
iter5
martijnvg Oct 12, 2025
d745536
fixed unit test
martijnvg Oct 12, 2025
70c7396
Merge remote-tracking branch 'es/main' into BlockSourceReader_source_…
martijnvg Oct 12, 2025
f33dabd
fix unit tests
martijnvg Oct 12, 2025
a177f8a
fix mocking in unit tests
martijnvg Oct 13, 2025
7741566
fix mocking in more unit tests
martijnvg Oct 13, 2025
026651b
Merge remote-tracking branch 'es/main' into BlockSourceReader_source_…
martijnvg Oct 13, 2025
ae0b28d
Remove SourceBasedValueFetcher
martijnvg Oct 13, 2025
6b556d0
Merge remote-tracking branch 'es/main' into BlockSourceReader_source_…
martijnvg Oct 13, 2025
c16ef04
added unit tests
martijnvg Oct 14, 2025
71c4af5
Merge remote-tracking branch 'es/main' into BlockSourceReader_source_…
martijnvg Oct 14, 2025
faf9b86
Update docs/changelog/136438.yaml
martijnvg Oct 14, 2025
6ebdd64
update changelog entry
martijnvg Oct 14, 2025
22868fc
update changelog entry (2)
martijnvg Oct 14, 2025
db13c3c
remove line length annotation
martijnvg Oct 14, 2025
c32be56
fix compile error
martijnvg Oct 14, 2025
d7de31a
Merge remote-tracking branch 'es/main' into BlockSourceReader_source_…
martijnvg Oct 15, 2025
43ad340
Avoid using null for StoredFieldsSpec#sourcePaths
martijnvg Oct 15, 2025
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
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,7 @@ public void benchmark() {
blockFactory,
ByteSizeValue.ofMb(1).getBytes(),
fields(name),
new IndexedByShardIdFromSingleton<>(new ValuesSourceReaderOperator.ShardContext(reader, () -> {
new IndexedByShardIdFromSingleton<>(new ValuesSourceReaderOperator.ShardContext(reader, (sourcePaths) -> {
throw new UnsupportedOperationException("can't load _source here");
}, EsqlPlugin.STORED_FIELDS_SEQUENTIAL_PROPORTION.getDefault(Settings.EMPTY))),
0
Expand Down
5 changes: 5 additions & 0 deletions docs/changelog/136438.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
pr: 136438
summary: '`BlockSourceReader` should always apply source filtering'
area: "ES|QL"
type: enhancement
issues: []
Original file line number Diff line number Diff line change
Expand Up @@ -605,7 +605,7 @@ protected String delegatingTo() {
}

// fallback to _source (synthetic or not)
SourceValueFetcher fetcher = SourceValueFetcher.toString(blContext.sourcePaths(name()));
SourceValueFetcher fetcher = SourceValueFetcher.toString(blContext.sourcePaths(name()), blContext.indexSettings());
// 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);
Expand Down Expand Up @@ -636,7 +636,7 @@ protected BytesRef storedToBytesRef(Object stored) {
return new SourceValueFetcherSortedBinaryIndexFieldData.Builder(
name(),
CoreValuesSourceType.KEYWORD,
SourceValueFetcher.toString(fieldDataContext.sourcePathsLookup().apply(name())),
SourceValueFetcher.toString(fieldDataContext.sourcePathsLookup().apply(name()), fieldDataContext.indexSettings()),
fieldDataContext.lookupSupplier().get(),
TextDocValuesField::new
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,11 +148,12 @@ public ValueFetcher valueFetcher(SearchExecutionContext context, String format)
if (format != null) {
throw new IllegalArgumentException("Field [" + name() + "] of type [" + typeName() + "] doesn't support formats.");
}
return sourceValueFetcher(context.isSourceEnabled() ? context.sourcePath(name()) : Collections.emptySet());
return sourceValueFetcher(context);
}

private SourceValueFetcher sourceValueFetcher(Set<String> sourcePaths) {
return new SourceValueFetcher(sourcePaths, nullValue) {
private SourceValueFetcher sourceValueFetcher(SearchExecutionContext context) {
Set<String> sourcePaths = context.isSourceEnabled() ? context.sourcePath(name()) : Collections.emptySet();
return new SourceValueFetcher(sourcePaths, nullValue, context.getIndexSettings().getIgnoredSourceFormat()) {
@Override
protected Object parseSourceValue(Object value) {
return objectToFloat(value);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.xcontent.support.XContentMapValues;
import org.elasticsearch.index.IndexMode;
import org.elasticsearch.index.IndexSettings;
import org.elasticsearch.index.IndexVersion;
import org.elasticsearch.index.IndexVersions;
import org.elasticsearch.index.fielddata.FieldData;
Expand Down Expand Up @@ -396,8 +397,7 @@ public Builder builder(BlockFactory factory, int expectedCount) {
}
};
}

ValueFetcher valueFetcher = sourceValueFetcher(blContext.sourcePaths(name()));
var valueFetcher = sourceValueFetcher(blContext.sourcePaths(name()), blContext.indexSettings());
BlockSourceReader.LeafIteratorLookup lookup = hasDocValues() == false && isStored()
// We only write the field names field if there aren't doc values
? BlockSourceReader.lookupFromFieldNames(blContext.fieldNames(), name())
Expand Down Expand Up @@ -489,7 +489,7 @@ public IndexFieldData.Builder fielddataBuilder(FieldDataContext fieldDataContext
return new SourceValueFetcherSortedDoubleIndexFieldData.Builder(
name(),
valuesSourceType,
sourceValueFetcher(sourcePaths),
sourceValueFetcher(sourcePaths, fieldDataContext.indexSettings()),
searchLookup,
ScaledFloatDocValuesField::new
);
Expand All @@ -503,11 +503,14 @@ public ValueFetcher valueFetcher(SearchExecutionContext context, String format)
if (format != null) {
throw new IllegalArgumentException("Field [" + name() + "] of type [" + typeName() + "] doesn't support formats.");
}
return sourceValueFetcher(context.isSourceEnabled() ? context.sourcePath(name()) : Collections.emptySet());
return sourceValueFetcher(
context.isSourceEnabled() ? context.sourcePath(name()) : Collections.emptySet(),
context.getIndexSettings()
);
}

private SourceValueFetcher sourceValueFetcher(Set<String> sourcePaths) {
return new SourceValueFetcher(sourcePaths, nullValue) {
private SourceValueFetcher sourceValueFetcher(Set<String> sourcePaths, IndexSettings indexSettings) {
return new SourceValueFetcher(sourcePaths, nullValue, indexSettings.getIgnoredSourceFormat()) {
@Override
protected Double parseSourceValue(Object value) {
double doubleValue;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@

import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

public class MatchOnlyTextFieldTypeTests extends FieldTypeTestCase {

Expand Down Expand Up @@ -315,6 +316,7 @@ public void testBlockLoaderDoesNotUseSyntheticSourceDelegateWhenIgnoreAboveIsSet
// when
MappedFieldType.BlockLoaderContext blContext = mock(MappedFieldType.BlockLoaderContext.class);
doReturn(FieldNamesFieldMapper.FieldNamesFieldType.get(false)).when(blContext).fieldNames();
when(blContext.indexSettings()).thenReturn(indexSettings);
BlockLoader blockLoader = ft.blockLoader(blContext);

// then
Expand Down Expand Up @@ -362,6 +364,7 @@ public void testBlockLoaderDoesNotUseSyntheticSourceDelegateWhenIgnoreAboveIsSet

// when
MappedFieldType.BlockLoaderContext blContext = mock(MappedFieldType.BlockLoaderContext.class);
when(blContext.indexSettings()).thenReturn(indexSettings);
doReturn(FieldNamesFieldMapper.FieldNamesFieldType.get(false)).when(blContext).fieldNames();
BlockLoader blockLoader = ft.blockLoader(blContext);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ protected void assertFetch(MapperService mapperService, String field, Object val
.build(new IndexFieldDataCache.None(), new NoneCircuitBreakerService())
);
SearchExecutionContext searchExecutionContext = mock(SearchExecutionContext.class);
when(searchExecutionContext.getIndexSettings()).thenReturn(mapperService.getIndexSettings());
when(searchExecutionContext.isSourceEnabled()).thenReturn(true);
when(searchExecutionContext.sourcePath(field)).thenReturn(Set.of(field));
when(searchExecutionContext.getForField(ft, fdt)).thenAnswer(inv -> fieldDataLookup(mapperService).apply(ft, () -> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1828,6 +1828,14 @@ public SourceFieldMapper.Mode getIndexMappingSourceMode() {
return indexMappingSourceMode;
}

public IgnoredSourceFieldMapper.IgnoredSourceFormat getIgnoredSourceFormat() {
if (getIndexMappingSourceMode() == SourceFieldMapper.Mode.SYNTHETIC) {
return IgnoredSourceFieldMapper.ignoredSourceFormat(getIndexVersionCreated());
} else {
return IgnoredSourceFieldMapper.IgnoredSourceFormat.NO_IGNORED_SOURCE;
}
}

/**
* @return Whether recovery source should be enabled if needed.
* Note that this is a node setting, and this setting is not sourced from index settings.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import org.elasticsearch.common.geo.GeometryFormatterFactory;
import org.elasticsearch.common.logging.DeprecationLogger;
import org.elasticsearch.core.CheckedConsumer;
import org.elasticsearch.index.IndexSettings;
import org.elasticsearch.index.query.SearchExecutionContext;
import org.elasticsearch.xcontent.DeprecationHandler;
import org.elasticsearch.xcontent.NamedXContentRegistry;
Expand Down Expand Up @@ -172,9 +173,9 @@ protected Object parseSourceValue(Object value) {
};
}

public ValueFetcher valueFetcher(Set<String> sourcePaths, T nullValue, String format) {
public ValueFetcher valueFetcher(Set<String> sourcePaths, T nullValue, String format, IndexSettings indexSettings) {
Function<List<T>, List<Object>> formatter = getFormatter(format != null ? format : GeometryFormatterFactory.GEOJSON);
return new ArraySourceValueFetcher(sourcePaths, nullValueAsSource(nullValue)) {
return new ArraySourceValueFetcher(sourcePaths, nullValueAsSource(nullValue), indexSettings.getIgnoredSourceFormat()) {
@Override
protected Object parseSourceValue(Object value) {
final List<T> values = new ArrayList<>();
Expand All @@ -185,7 +186,7 @@ protected Object parseSourceValue(Object value) {
}

protected BlockLoader blockLoaderFromSource(BlockLoaderContext blContext) {
ValueFetcher fetcher = valueFetcher(blContext.sourcePaths(name()), nullValue, GeometryFormatterFactory.WKB);
var fetcher = valueFetcher(blContext.sourcePaths(name()), nullValue, GeometryFormatterFactory.WKB, blContext.indexSettings());
// TODO consider optimization using BlockSourceReader.lookupFromFieldNames(blContext.fieldNames(), name())
return new BlockSourceReader.GeometriesBlockLoader(fetcher, BlockSourceReader.lookupMatchingAll());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
package org.elasticsearch.index.mapper;

import org.elasticsearch.core.Nullable;
import org.elasticsearch.index.mapper.IgnoredSourceFieldMapper.IgnoredSourceFormat;
import org.elasticsearch.index.query.SearchExecutionContext;
import org.elasticsearch.search.fetch.StoredFieldsSpec;
import org.elasticsearch.search.lookup.Source;
Expand All @@ -30,6 +31,7 @@
public abstract class ArraySourceValueFetcher implements ValueFetcher {
private final Set<String> sourcePaths;
private final @Nullable Object nullValue;
private final IgnoredSourceFormat ignoredSourceFormat;

public ArraySourceValueFetcher(String fieldName, SearchExecutionContext context) {
this(fieldName, context, null);
Expand All @@ -43,15 +45,17 @@ public ArraySourceValueFetcher(String fieldName, SearchExecutionContext context)
public ArraySourceValueFetcher(String fieldName, SearchExecutionContext context, Object nullValue) {
this.sourcePaths = context.isSourceEnabled() ? context.sourcePath(fieldName) : Collections.emptySet();
this.nullValue = nullValue;
this.ignoredSourceFormat = context.getIndexSettings().getIgnoredSourceFormat();
}

/**
* @param sourcePaths The paths to pull source values from
* @param nullValue An optional substitute value if the _source value is `null`
*/
public ArraySourceValueFetcher(Set<String> sourcePaths, Object nullValue) {
public ArraySourceValueFetcher(Set<String> sourcePaths, Object nullValue, IgnoredSourceFormat ignoredSourceFormat) {
this.sourcePaths = sourcePaths;
this.nullValue = nullValue;
this.ignoredSourceFormat = ignoredSourceFormat;
}

@Override
Expand All @@ -76,7 +80,7 @@ public List<Object> fetchValues(Source source, int doc, List<Object> ignoredValu

@Override
public StoredFieldsSpec storedFieldsSpec() {
return StoredFieldsSpec.NEEDS_SOURCE;
return StoredFieldsSpec.withSourcePaths(ignoredSourceFormat, sourcePaths);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ public final ColumnAtATimeReader columnAtATimeReader(LeafReaderContext context)

@Override
public final StoredFieldsSpec rowStrideStoredFieldSpec() {
return StoredFieldsSpec.NEEDS_SOURCE;
return fetcher.storedFieldsSpec();
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import org.elasticsearch.common.xcontent.support.XContentMapValues;
import org.elasticsearch.core.Booleans;
import org.elasticsearch.core.Nullable;
import org.elasticsearch.index.IndexSettings;
import org.elasticsearch.index.IndexVersion;
import org.elasticsearch.index.IndexVersions;
import org.elasticsearch.index.analysis.NamedAnalyzer;
Expand Down Expand Up @@ -281,11 +282,14 @@ public ValueFetcher valueFetcher(SearchExecutionContext context, String format)
if (this.scriptValues != null) {
return FieldValues.valueFetcher(this.scriptValues, context);
}
return sourceValueFetcher(context.isSourceEnabled() ? context.sourcePath(name()) : Collections.emptySet());
return sourceValueFetcher(
context.isSourceEnabled() ? context.sourcePath(name()) : Collections.emptySet(),
context.getIndexSettings()
);
}

private SourceValueFetcher sourceValueFetcher(Set<String> sourcePaths) {
return new SourceValueFetcher(sourcePaths, nullValue) {
private SourceValueFetcher sourceValueFetcher(Set<String> sourcePaths, IndexSettings indexSettings) {
return new SourceValueFetcher(sourcePaths, nullValue, indexSettings.getIgnoredSourceFormat()) {
@Override
protected Boolean parseSourceValue(Object value) {
if (value instanceof Boolean) {
Expand Down Expand Up @@ -364,7 +368,7 @@ public Builder builder(BlockFactory factory, int expectedCount) {
};
}

ValueFetcher fetcher = sourceValueFetcher(blContext.sourcePaths(name()));
var fetcher = sourceValueFetcher(blContext.sourcePaths(name()), blContext.indexSettings());
BlockSourceReader.LeafIteratorLookup lookup = indexType.hasTerms() || isStored()
? BlockSourceReader.lookupFromFieldNames(blContext.fieldNames(), name())
: BlockSourceReader.lookupMatchingAll();
Expand Down Expand Up @@ -431,7 +435,7 @@ public IndexFieldData.Builder fielddataBuilder(FieldDataContext fieldDataContext
return new SourceValueFetcherSortedBooleanIndexFieldData.Builder(
name(),
CoreValuesSourceType.BOOLEAN,
sourceValueFetcher(sourcePaths),
sourceValueFetcher(sourcePaths, fieldDataContext.indexSettings()),
searchLookup,
BooleanDocValuesField::new
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -662,8 +662,8 @@ public String parseSourceValue(Object value) {
}

// returns a Long to support source fallback which emulates numeric doc values for dates
private SourceValueFetcher sourceValueFetcher(Set<String> sourcePaths) {
return new SourceValueFetcher(sourcePaths, nullValue) {
private SourceValueFetcher sourceValueFetcher(Set<String> sourcePaths, IndexSettings indexSettings) {
return new SourceValueFetcher(sourcePaths, nullValue, indexSettings.getIgnoredSourceFormat()) {
@Override
public Long parseSourceValue(Object value) {
String date = value instanceof Number ? NUMBER_FORMAT.format(value) : value.toString();
Expand Down Expand Up @@ -995,11 +995,13 @@ public Builder builder(BlockFactory factory, int expectedCount) {
}
};
}

BlockSourceReader.LeafIteratorLookup lookup = isStored() || indexType.hasPoints()
? BlockSourceReader.lookupFromFieldNames(blContext.fieldNames(), name())
: BlockSourceReader.lookupMatchingAll();
return new BlockSourceReader.LongsBlockLoader(sourceValueFetcher(blContext.sourcePaths(name())), lookup);
return new BlockSourceReader.LongsBlockLoader(
sourceValueFetcher(blContext.sourcePaths(name()), blContext.indexSettings()),
lookup
);
}

private FallbackSyntheticSourceBlockLoader.Reader<?> fallbackSyntheticSourceBlockLoaderReader() {
Expand Down Expand Up @@ -1067,7 +1069,7 @@ public IndexFieldData.Builder fielddataBuilder(FieldDataContext fieldDataContext
return new SourceValueFetcherSortedNumericIndexFieldData.Builder(
name(),
resolution.numericType().getValuesSourceType(),
sourceValueFetcher(sourcePaths),
sourceValueFetcher(sourcePaths, fieldDataContext.indexSettings()),
searchLookup,
resolution.getDefaultToScriptFieldFactory()
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ public RowStrideReader rowStrideReader(LeafReaderContext context) throws IOExcep

@Override
public StoredFieldsSpec rowStrideStoredFieldSpec() {
return new StoredFieldsSpec(false, false, Set.of(), new IgnoredFieldsSpec(Set.of(fieldName), ignoredSourceFormat));
return new StoredFieldsSpec(false, false, Set.of(), new IgnoredFieldsSpec(Set.of(fieldName), ignoredSourceFormat), Set.of());
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -494,7 +494,7 @@ public IndexFieldData.Builder fielddataBuilder(FieldDataContext fieldDataContext
return new SourceValueFetcherMultiGeoPointIndexFieldData.Builder(
name(),
valuesSourceType,
valueFetcher(sourcePaths, null, null),
valueFetcher(sourcePaths, null, null, fieldDataContext.indexSettings()),
searchLookup,
GeoPointDocValuesField::new
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@
import java.util.Collections;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.function.BiFunction;

import static org.elasticsearch.index.mapper.FieldArrayContext.getOffsetsFieldName;
Expand Down Expand Up @@ -481,12 +480,11 @@ public BlockLoader blockLoader(BlockLoaderContext blContext) {
if (isSyntheticSource && blContext.parentField(name()) == null) {
return blockLoaderFromFallbackSyntheticSource(blContext);
}

// see #indexValue
BlockSourceReader.LeafIteratorLookup lookup = hasDocValues() == false && hasPoints
? BlockSourceReader.lookupFromFieldNames(blContext.fieldNames(), name())
: BlockSourceReader.lookupMatchingAll();
return new BlockSourceReader.IpsBlockLoader(sourceValueFetcher(blContext.sourcePaths(name())), lookup);
return new BlockSourceReader.IpsBlockLoader(sourceValueFetcher(blContext), lookup);
}

private BlockLoader blockLoaderFromFallbackSyntheticSource(BlockLoaderContext blContext) {
Expand All @@ -503,8 +501,8 @@ public Builder builder(BlockFactory factory, int expectedCount) {
};
}

private SourceValueFetcher sourceValueFetcher(Set<String> sourcePaths) {
return new SourceValueFetcher(sourcePaths, nullValue) {
private SourceValueFetcher sourceValueFetcher(BlockLoaderContext blContext) {
return new SourceValueFetcher(blContext.sourcePaths(name()), nullValue, blContext.indexSettings().getIgnoredSourceFormat()) {
@Override
public InetAddress parseSourceValue(Object value) {
return parse(value);
Expand Down
Loading