Skip to content

Commit 3814cf4

Browse files
martijnvgmartijnvg
authored
Sometimes delegate to SourceLoader in ValueSourceReaderOperator for required stored fields (#115114) (#115390)
If source is required by a block loader then the StoredFieldsSpec that gets populated should be enhanced by SourceLoader#requiredStoredFields(...) in ValuesSourceReaderOperator. Otherwise in case of synthetic source many stored fields aren't loaded, which causes only a subset of _source to be synthesized. For example when unmapped fields exist or field values that exceed configured ignore above will not appear is _source. This happens when field types fallback to a block loader implementation that uses _source. The required field values are then extracted from the source once loaded. This change also reverts the production code changes introduced via #114903. That change only ensured that _ignored_source field was added to the required list of stored fields. In reality more fields could be required. This change is better fix, since it handles also other cases and the SourceLoader implementation indicates which stored fields are needed. Closes #115076
1 parent a8cef2d commit 3814cf4

File tree

21 files changed

+276
-163
lines changed

21 files changed

+276
-163
lines changed

‎modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/extras/MatchOnlyTextFieldMapper.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -364,8 +364,7 @@ public BlockLoader blockLoader(BlockLoaderContext blContext) {
364364
SourceValueFetcher fetcher = SourceValueFetcher.toString(blContext.sourcePaths(name()));
365365
// MatchOnlyText never has norms, so we have to use the field names field
366366
BlockSourceReader.LeafIteratorLookup lookup = BlockSourceReader.lookupFromFieldNames(blContext.fieldNames(), name());
367-
var sourceMode = blContext.indexSettings().getIndexMappingSourceMode();
368-
return new BlockSourceReader.BytesRefsBlockLoader(fetcher, lookup, sourceMode);
367+
return new BlockSourceReader.BytesRefsBlockLoader(fetcher, lookup);
369368
}
370369

371370
@Override

‎modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/extras/ScaledFloatFieldMapper.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -319,8 +319,7 @@ public BlockLoader blockLoader(BlockLoaderContext blContext) {
319319
BlockSourceReader.LeafIteratorLookup lookup = isStored() || isIndexed()
320320
? BlockSourceReader.lookupFromFieldNames(blContext.fieldNames(), name())
321321
: BlockSourceReader.lookupMatchingAll();
322-
var sourceMode = blContext.indexSettings().getIndexMappingSourceMode();
323-
return new BlockSourceReader.DoublesBlockLoader(valueFetcher, lookup, sourceMode);
322+
return new BlockSourceReader.DoublesBlockLoader(valueFetcher, lookup);
324323
}
325324

326325
@Override

‎muted-tests.yml‎

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -395,18 +395,6 @@ tests:
395395
- class: org.elasticsearch.xpack.enrich.EnrichIT
396396
method: testDeleteExistingPipeline
397397
issue: https://github.com/elastic/elasticsearch/issues/114775
398-
- class: org.elasticsearch.index.mapper.TextFieldMapperTests
399-
method: testBlockLoaderFromRowStrideReaderWithSyntheticSource
400-
issue: https://github.com/elastic/elasticsearch/issues/115066
401-
- class: org.elasticsearch.index.mapper.TextFieldMapperTests
402-
method: testBlockLoaderFromColumnReaderWithSyntheticSource
403-
issue: https://github.com/elastic/elasticsearch/issues/115073
404-
- class: org.elasticsearch.index.mapper.annotatedtext.AnnotatedTextFieldMapperTests
405-
method: testBlockLoaderFromColumnReaderWithSyntheticSource
406-
issue: https://github.com/elastic/elasticsearch/issues/115074
407-
- class: org.elasticsearch.index.mapper.annotatedtext.AnnotatedTextFieldMapperTests
408-
method: testBlockLoaderFromRowStrideReaderWithSyntheticSource
409-
issue: https://github.com/elastic/elasticsearch/issues/115076
410398
- class: org.elasticsearch.xpack.enrich.EnrichIT
411399
method: testEnrichSpecialTypes
412400
issue: https://github.com/elastic/elasticsearch/issues/114773

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -189,8 +189,7 @@ public BlockLoader blockLoader(BlockLoaderContext blContext) {
189189
protected BlockLoader blockLoaderFromSource(BlockLoaderContext blContext) {
190190
ValueFetcher fetcher = valueFetcher(blContext.sourcePaths(name()), nullValue, GeometryFormatterFactory.WKB);
191191
// TODO consider optimization using BlockSourceReader.lookupFromFieldNames(blContext.fieldNames(), name())
192-
var sourceMode = blContext.indexSettings().getIndexMappingSourceMode();
193-
return new BlockSourceReader.GeometriesBlockLoader(fetcher, BlockSourceReader.lookupMatchingAll(), sourceMode);
192+
return new BlockSourceReader.GeometriesBlockLoader(fetcher, BlockSourceReader.lookupMatchingAll());
194193
}
195194

196195
protected abstract Object nullValueAsSource(T nullValue);

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

Lines changed: 18 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -22,22 +22,13 @@
2222
import java.io.IOException;
2323
import java.util.ArrayList;
2424
import java.util.List;
25-
import java.util.Set;
2625

2726
/**
2827
* Loads values from {@code _source}. This whole process is very slow and cast-tastic,
2928
* so it doesn't really try to avoid megamorphic invocations. It's just going to be
3029
* slow.
3130
*/
3231
public abstract class BlockSourceReader implements BlockLoader.RowStrideReader {
33-
34-
// _ignored_source is needed when source mode is synthetic.
35-
static final StoredFieldsSpec NEEDS_SOURCE_AND_IGNORED_SOURCE = new StoredFieldsSpec(
36-
true,
37-
false,
38-
Set.of(IgnoredSourceFieldMapper.NAME)
39-
);
40-
4132
private final ValueFetcher fetcher;
4233
private final List<Object> ignoredValues = new ArrayList<>();
4334
private final DocIdSetIterator iter;
@@ -100,12 +91,10 @@ public interface LeafIteratorLookup {
10091
private abstract static class SourceBlockLoader implements BlockLoader {
10192
protected final ValueFetcher fetcher;
10293
private final LeafIteratorLookup lookup;
103-
private final SourceFieldMapper.Mode sourceMode;
10494

105-
private SourceBlockLoader(ValueFetcher fetcher, LeafIteratorLookup lookup, SourceFieldMapper.Mode sourceMode) {
95+
private SourceBlockLoader(ValueFetcher fetcher, LeafIteratorLookup lookup) {
10696
this.fetcher = fetcher;
10797
this.lookup = lookup;
108-
this.sourceMode = sourceMode;
10998
}
11099

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

116105
@Override
117106
public final StoredFieldsSpec rowStrideStoredFieldSpec() {
118-
return sourceMode == SourceFieldMapper.Mode.SYNTHETIC ? NEEDS_SOURCE_AND_IGNORED_SOURCE : StoredFieldsSpec.NEEDS_SOURCE;
107+
return StoredFieldsSpec.NEEDS_SOURCE;
119108
}
120109

121110
@Override
@@ -151,8 +140,8 @@ public final String toString() {
151140
* Load {@code boolean}s from {@code _source}.
152141
*/
153142
public static class BooleansBlockLoader extends SourceBlockLoader {
154-
public BooleansBlockLoader(ValueFetcher fetcher, LeafIteratorLookup lookup, SourceFieldMapper.Mode sourceMode) {
155-
super(fetcher, lookup, sourceMode);
143+
public BooleansBlockLoader(ValueFetcher fetcher, LeafIteratorLookup lookup) {
144+
super(fetcher, lookup);
156145
}
157146

158147
@Override
@@ -191,8 +180,8 @@ public String toString() {
191180
* Load {@link BytesRef}s from {@code _source}.
192181
*/
193182
public static class BytesRefsBlockLoader extends SourceBlockLoader {
194-
public BytesRefsBlockLoader(ValueFetcher fetcher, LeafIteratorLookup lookup, SourceFieldMapper.Mode sourceMode) {
195-
super(fetcher, lookup, sourceMode);
183+
public BytesRefsBlockLoader(ValueFetcher fetcher, LeafIteratorLookup lookup) {
184+
super(fetcher, lookup);
196185
}
197186

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

203192
@Override
204193
protected RowStrideReader rowStrideReader(LeafReaderContext context, DocIdSetIterator iter) throws IOException {
205-
return new BytesRefs(fetcher, iter, null);
194+
return new BytesRefs(fetcher, iter);
206195
}
207196

208197
@Override
@@ -212,8 +201,8 @@ protected String name() {
212201
}
213202

214203
public static class GeometriesBlockLoader extends SourceBlockLoader {
215-
public GeometriesBlockLoader(ValueFetcher fetcher, LeafIteratorLookup lookup, SourceFieldMapper.Mode sourceMode) {
216-
super(fetcher, lookup, sourceMode);
204+
public GeometriesBlockLoader(ValueFetcher fetcher, LeafIteratorLookup lookup) {
205+
super(fetcher, lookup);
217206
}
218207

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

224213
@Override
225214
protected RowStrideReader rowStrideReader(LeafReaderContext context, DocIdSetIterator iter) {
226-
return new Geometries(fetcher, iter, null);
215+
return new Geometries(fetcher, iter);
227216
}
228217

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

238-
BytesRefs(ValueFetcher fetcher, DocIdSetIterator iter, SourceFieldMapper.Mode sourceMode) {
227+
BytesRefs(ValueFetcher fetcher, DocIdSetIterator iter) {
239228
super(fetcher, iter);
240229
}
241230

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

253242
private static class Geometries extends BlockSourceReader {
254243

255-
Geometries(ValueFetcher fetcher, DocIdSetIterator iter, SourceFieldMapper.Mode sourceMode) {
244+
Geometries(ValueFetcher fetcher, DocIdSetIterator iter) {
256245
super(fetcher, iter);
257246
}
258247

@@ -275,8 +264,8 @@ public String toString() {
275264
* Load {@code double}s from {@code _source}.
276265
*/
277266
public static class DoublesBlockLoader extends SourceBlockLoader {
278-
public DoublesBlockLoader(ValueFetcher fetcher, LeafIteratorLookup lookup, SourceFieldMapper.Mode sourceMode) {
279-
super(fetcher, lookup, sourceMode);
267+
public DoublesBlockLoader(ValueFetcher fetcher, LeafIteratorLookup lookup) {
268+
super(fetcher, lookup);
280269
}
281270

282271
@Override
@@ -315,8 +304,8 @@ public String toString() {
315304
* Load {@code int}s from {@code _source}.
316305
*/
317306
public static class IntsBlockLoader extends SourceBlockLoader {
318-
public IntsBlockLoader(ValueFetcher fetcher, LeafIteratorLookup lookup, SourceFieldMapper.Mode sourceMode) {
319-
super(fetcher, lookup, sourceMode);
307+
public IntsBlockLoader(ValueFetcher fetcher, LeafIteratorLookup lookup) {
308+
super(fetcher, lookup);
320309
}
321310

322311
@Override
@@ -355,8 +344,8 @@ public String toString() {
355344
* Load {@code long}s from {@code _source}.
356345
*/
357346
public static class LongsBlockLoader extends SourceBlockLoader {
358-
public LongsBlockLoader(ValueFetcher fetcher, LeafIteratorLookup lookup, SourceFieldMapper.Mode sourceMode) {
359-
super(fetcher, lookup, sourceMode);
347+
public LongsBlockLoader(ValueFetcher fetcher, LeafIteratorLookup lookup) {
348+
super(fetcher, lookup);
360349
}
361350

362351
@Override

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -314,7 +314,7 @@ public BlockLoader blockLoader(BlockLoaderContext blContext) {
314314
BlockSourceReader.LeafIteratorLookup lookup = isIndexed() || isStored()
315315
? BlockSourceReader.lookupFromFieldNames(blContext.fieldNames(), name())
316316
: BlockSourceReader.lookupMatchingAll();
317-
return new BlockSourceReader.BooleansBlockLoader(fetcher, lookup, blContext.indexSettings().getIndexMappingSourceMode());
317+
return new BlockSourceReader.BooleansBlockLoader(fetcher, lookup);
318318
}
319319

320320
@Override

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -809,8 +809,7 @@ public BlockLoader blockLoader(BlockLoaderContext blContext) {
809809
BlockSourceReader.LeafIteratorLookup lookup = isStored() || isIndexed()
810810
? BlockSourceReader.lookupFromFieldNames(blContext.fieldNames(), name())
811811
: BlockSourceReader.lookupMatchingAll();
812-
var sourceMode = blContext.indexSettings().getIndexMappingSourceMode();
813-
return new BlockSourceReader.LongsBlockLoader(sourceValueFetcher(blContext.sourcePaths(name())), lookup, sourceMode);
812+
return new BlockSourceReader.LongsBlockLoader(sourceValueFetcher(blContext.sourcePaths(name())), lookup);
814813
}
815814

816815
@Override

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -636,8 +636,7 @@ public BlockLoader blockLoader(BlockLoaderContext blContext) {
636636
return new BlockStoredFieldsReader.BytesFromBytesRefsBlockLoader(name());
637637
}
638638
SourceValueFetcher fetcher = sourceValueFetcher(blContext.sourcePaths(name()));
639-
var sourceMode = blContext.indexSettings().getIndexMappingSourceMode();
640-
return new BlockSourceReader.BytesRefsBlockLoader(fetcher, sourceBlockLoaderLookup(blContext), sourceMode);
639+
return new BlockSourceReader.BytesRefsBlockLoader(fetcher, sourceBlockLoaderLookup(blContext));
641640
}
642641

643642
private BlockSourceReader.LeafIteratorLookup sourceBlockLoaderLookup(BlockLoaderContext blContext) {

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

Lines changed: 16 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -461,12 +461,8 @@ BlockLoader blockLoaderFromDocValues(String fieldName) {
461461
}
462462

463463
@Override
464-
BlockLoader blockLoaderFromSource(
465-
SourceValueFetcher sourceValueFetcher,
466-
BlockSourceReader.LeafIteratorLookup lookup,
467-
SourceFieldMapper.Mode sourceMode
468-
) {
469-
return new BlockSourceReader.DoublesBlockLoader(sourceValueFetcher, lookup, sourceMode);
464+
BlockLoader blockLoaderFromSource(SourceValueFetcher sourceValueFetcher, BlockSourceReader.LeafIteratorLookup lookup) {
465+
return new BlockSourceReader.DoublesBlockLoader(sourceValueFetcher, lookup);
470466
}
471467
},
472468
FLOAT("float", NumericType.FLOAT) {
@@ -649,12 +645,8 @@ BlockLoader blockLoaderFromDocValues(String fieldName) {
649645
}
650646

651647
@Override
652-
BlockLoader blockLoaderFromSource(
653-
SourceValueFetcher sourceValueFetcher,
654-
BlockSourceReader.LeafIteratorLookup lookup,
655-
SourceFieldMapper.Mode sourceMode
656-
) {
657-
return new BlockSourceReader.DoublesBlockLoader(sourceValueFetcher, lookup, sourceMode);
648+
BlockLoader blockLoaderFromSource(SourceValueFetcher sourceValueFetcher, BlockSourceReader.LeafIteratorLookup lookup) {
649+
return new BlockSourceReader.DoublesBlockLoader(sourceValueFetcher, lookup);
658650
}
659651
},
660652
DOUBLE("double", NumericType.DOUBLE) {
@@ -803,12 +795,8 @@ BlockLoader blockLoaderFromDocValues(String fieldName) {
803795
}
804796

805797
@Override
806-
BlockLoader blockLoaderFromSource(
807-
SourceValueFetcher sourceValueFetcher,
808-
BlockSourceReader.LeafIteratorLookup lookup,
809-
SourceFieldMapper.Mode sourceMode
810-
) {
811-
return new BlockSourceReader.DoublesBlockLoader(sourceValueFetcher, lookup, sourceMode);
798+
BlockLoader blockLoaderFromSource(SourceValueFetcher sourceValueFetcher, BlockSourceReader.LeafIteratorLookup lookup) {
799+
return new BlockSourceReader.DoublesBlockLoader(sourceValueFetcher, lookup);
812800
}
813801
},
814802
BYTE("byte", NumericType.BYTE) {
@@ -920,12 +908,8 @@ BlockLoader blockLoaderFromDocValues(String fieldName) {
920908
}
921909

922910
@Override
923-
BlockLoader blockLoaderFromSource(
924-
SourceValueFetcher sourceValueFetcher,
925-
BlockSourceReader.LeafIteratorLookup lookup,
926-
SourceFieldMapper.Mode sourceMode
927-
) {
928-
return new BlockSourceReader.IntsBlockLoader(sourceValueFetcher, lookup, sourceMode);
911+
BlockLoader blockLoaderFromSource(SourceValueFetcher sourceValueFetcher, BlockSourceReader.LeafIteratorLookup lookup) {
912+
return new BlockSourceReader.IntsBlockLoader(sourceValueFetcher, lookup);
929913
}
930914

931915
private boolean isOutOfRange(Object value) {
@@ -1037,12 +1021,8 @@ BlockLoader blockLoaderFromDocValues(String fieldName) {
10371021
}
10381022

10391023
@Override
1040-
BlockLoader blockLoaderFromSource(
1041-
SourceValueFetcher sourceValueFetcher,
1042-
BlockSourceReader.LeafIteratorLookup lookup,
1043-
SourceFieldMapper.Mode sourceMode
1044-
) {
1045-
return new BlockSourceReader.IntsBlockLoader(sourceValueFetcher, lookup, sourceMode);
1024+
BlockLoader blockLoaderFromSource(SourceValueFetcher sourceValueFetcher, BlockSourceReader.LeafIteratorLookup lookup) {
1025+
return new BlockSourceReader.IntsBlockLoader(sourceValueFetcher, lookup);
10461026
}
10471027

10481028
private boolean isOutOfRange(Object value) {
@@ -1228,12 +1208,8 @@ BlockLoader blockLoaderFromDocValues(String fieldName) {
12281208
}
12291209

12301210
@Override
1231-
BlockLoader blockLoaderFromSource(
1232-
SourceValueFetcher sourceValueFetcher,
1233-
BlockSourceReader.LeafIteratorLookup lookup,
1234-
SourceFieldMapper.Mode sourceMode
1235-
) {
1236-
return new BlockSourceReader.IntsBlockLoader(sourceValueFetcher, lookup, sourceMode);
1211+
BlockLoader blockLoaderFromSource(SourceValueFetcher sourceValueFetcher, BlockSourceReader.LeafIteratorLookup lookup) {
1212+
return new BlockSourceReader.IntsBlockLoader(sourceValueFetcher, lookup);
12371213
}
12381214
},
12391215
LONG("long", NumericType.LONG) {
@@ -1379,12 +1355,8 @@ BlockLoader blockLoaderFromDocValues(String fieldName) {
13791355
}
13801356

13811357
@Override
1382-
BlockLoader blockLoaderFromSource(
1383-
SourceValueFetcher sourceValueFetcher,
1384-
BlockSourceReader.LeafIteratorLookup lookup,
1385-
SourceFieldMapper.Mode sourceMode
1386-
) {
1387-
return new BlockSourceReader.LongsBlockLoader(sourceValueFetcher, lookup, sourceMode);
1358+
BlockLoader blockLoaderFromSource(SourceValueFetcher sourceValueFetcher, BlockSourceReader.LeafIteratorLookup lookup) {
1359+
return new BlockSourceReader.LongsBlockLoader(sourceValueFetcher, lookup);
13881360
}
13891361

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

16631635
abstract BlockLoader blockLoaderFromDocValues(String fieldName);
16641636

1665-
abstract BlockLoader blockLoaderFromSource(
1666-
SourceValueFetcher sourceValueFetcher,
1667-
BlockSourceReader.LeafIteratorLookup lookup,
1668-
SourceFieldMapper.Mode sourceMode
1669-
);
1637+
abstract BlockLoader blockLoaderFromSource(SourceValueFetcher sourceValueFetcher, BlockSourceReader.LeafIteratorLookup lookup);
16701638
}
16711639

16721640
public static class NumberFieldType extends SimpleMappedFieldType {
@@ -1805,8 +1773,7 @@ public BlockLoader blockLoader(BlockLoaderContext blContext) {
18051773
BlockSourceReader.LeafIteratorLookup lookup = isStored() || isIndexed()
18061774
? BlockSourceReader.lookupFromFieldNames(blContext.fieldNames(), name())
18071775
: BlockSourceReader.lookupMatchingAll();
1808-
var sourceMode = blContext.indexSettings().getIndexMappingSourceMode();
1809-
return type.blockLoaderFromSource(sourceValueFetcher(blContext.sourcePaths(name())), lookup, sourceMode);
1776+
return type.blockLoaderFromSource(sourceValueFetcher(blContext.sourcePaths(name())), lookup);
18101777
}
18111778

18121779
@Override

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

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1007,20 +1007,8 @@ protected String delegatingTo() {
10071007
if (isStored()) {
10081008
return new BlockStoredFieldsReader.BytesFromStringsBlockLoader(name());
10091009
}
1010-
if (isSyntheticSource && syntheticSourceDelegate == null) {
1011-
/*
1012-
* When we're in synthetic source mode we don't currently
1013-
* support text fields that are not stored and are not children
1014-
* of perfect keyword fields. We'd have to load from the parent
1015-
* field and then convert the result to a string. In this case,
1016-
* even if we would synthesize the source, the current field
1017-
* would be missing.
1018-
*/
1019-
return null;
1020-
}
10211010
SourceValueFetcher fetcher = SourceValueFetcher.toString(blContext.sourcePaths(name()));
1022-
var sourceMode = blContext.indexSettings().getIndexMappingSourceMode();
1023-
return new BlockSourceReader.BytesRefsBlockLoader(fetcher, blockReaderDisiLookup(blContext), sourceMode);
1011+
return new BlockSourceReader.BytesRefsBlockLoader(fetcher, blockReaderDisiLookup(blContext));
10241012
}
10251013

10261014
/**

0 commit comments

Comments
 (0)