Sometimes delegate to SourceLoader in ValueSourceReaderOperator for r…

…equired stored fields (elastic#115114)

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 elastic#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 elastic#115076

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

@@ -364,8 +364,7 @@ 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());
-            var sourceMode = blContext.indexSettings().getIndexMappingSourceMode();
-            return new BlockSourceReader.BytesRefsBlockLoader(fetcher, lookup, sourceMode);
+            return new BlockSourceReader.BytesRefsBlockLoader(fetcher, lookup);
         }
 
         @Override

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

@@ -319,8 +319,7 @@ public BlockLoader blockLoader(BlockLoaderContext blContext) {
             BlockSourceReader.LeafIteratorLookup lookup = isStored() || isIndexed()
                 ? BlockSourceReader.lookupFromFieldNames(blContext.fieldNames(), name())
                 : BlockSourceReader.lookupMatchingAll();
-            var sourceMode = blContext.indexSettings().getIndexMappingSourceMode();
-            return new BlockSourceReader.DoublesBlockLoader(valueFetcher, lookup, sourceMode);
+            return new BlockSourceReader.DoublesBlockLoader(valueFetcher, lookup);
         }
 
         @Override

muted-tests.yml

@@ -282,18 +282,6 @@ tests:
 - class: org.elasticsearch.xpack.inference.DefaultEndPointsIT
   method: testInferDeploysDefaultElser
   issue: https://github.com/elastic/elasticsearch/issues/114913
-- 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
 - class: org.elasticsearch.xpack.test.rest.XPackRestIT
   method: test {p0=esql/60_usage/Basic ESQL usage output (telemetry)}
   issue: https://github.com/elastic/elasticsearch/issues/115231

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

@@ -189,8 +189,7 @@ 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())
-            var sourceMode = blContext.indexSettings().getIndexMappingSourceMode();
-            return new BlockSourceReader.GeometriesBlockLoader(fetcher, BlockSourceReader.lookupMatchingAll(), sourceMode);
+            return new BlockSourceReader.GeometriesBlockLoader(fetcher, BlockSourceReader.lookupMatchingAll());
         }
 
         protected abstract Object nullValueAsSource(T nullValue);

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

@@ -22,22 +22,13 @@
 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;
@@ -100,12 +91,10 @@ 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, SourceFieldMapper.Mode sourceMode) {
+        private SourceBlockLoader(ValueFetcher fetcher, LeafIteratorLookup lookup) {
             this.fetcher = fetcher;
             this.lookup = lookup;
-            this.sourceMode = sourceMode;
         }
 
         @Override
@@ -115,7 +104,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.NEEDS_SOURCE;
         }
 
         @Override
@@ -151,8 +140,8 @@ public final String toString() {
      * Load {@code boolean}s from {@code _source}.
      */
     public static class BooleansBlockLoader extends SourceBlockLoader {
-        public BooleansBlockLoader(ValueFetcher fetcher, LeafIteratorLookup lookup, SourceFieldMapper.Mode sourceMode) {
-            super(fetcher, lookup, sourceMode);
+        public BooleansBlockLoader(ValueFetcher fetcher, LeafIteratorLookup lookup) {
+            super(fetcher, lookup);
         }
 
         @Override
@@ -191,8 +180,8 @@ public String toString() {
      * Load {@link BytesRef}s from {@code _source}.
      */
     public static class BytesRefsBlockLoader extends SourceBlockLoader {
-        public BytesRefsBlockLoader(ValueFetcher fetcher, LeafIteratorLookup lookup, SourceFieldMapper.Mode sourceMode) {
-            super(fetcher, lookup, sourceMode);
+        public BytesRefsBlockLoader(ValueFetcher fetcher, LeafIteratorLookup lookup) {
+            super(fetcher, lookup);
         }
 
         @Override
@@ -202,7 +191,7 @@ public final Builder builder(BlockFactory factory, int expectedCount) {
 
         @Override
         protected RowStrideReader rowStrideReader(LeafReaderContext context, DocIdSetIterator iter) throws IOException {
-            return new BytesRefs(fetcher, iter, null);
+            return new BytesRefs(fetcher, iter);
         }
 
         @Override
@@ -212,8 +201,8 @@ protected String name() {
     }
 
     public static class GeometriesBlockLoader extends SourceBlockLoader {
-        public GeometriesBlockLoader(ValueFetcher fetcher, LeafIteratorLookup lookup, SourceFieldMapper.Mode sourceMode) {
-            super(fetcher, lookup, sourceMode);
+        public GeometriesBlockLoader(ValueFetcher fetcher, LeafIteratorLookup lookup) {
+            super(fetcher, lookup);
         }
 
         @Override
@@ -223,7 +212,7 @@ public final Builder builder(BlockFactory factory, int expectedCount) {
 
         @Override
         protected RowStrideReader rowStrideReader(LeafReaderContext context, DocIdSetIterator iter) {
-            return new Geometries(fetcher, iter, null);
+            return new Geometries(fetcher, iter);
         }
 
         @Override
@@ -235,7 +224,7 @@ protected String name() {
     private static class BytesRefs extends BlockSourceReader {
         private final BytesRef scratch = new BytesRef();
 
-        BytesRefs(ValueFetcher fetcher, DocIdSetIterator iter, SourceFieldMapper.Mode sourceMode) {
+        BytesRefs(ValueFetcher fetcher, DocIdSetIterator iter) {
             super(fetcher, iter);
         }
 
@@ -252,7 +241,7 @@ public String toString() {
 
     private static class Geometries extends BlockSourceReader {
 
-        Geometries(ValueFetcher fetcher, DocIdSetIterator iter, SourceFieldMapper.Mode sourceMode) {
+        Geometries(ValueFetcher fetcher, DocIdSetIterator iter) {
             super(fetcher, iter);
         }
 
@@ -275,8 +264,8 @@ public String toString() {
      * Load {@code double}s from {@code _source}.
      */
     public static class DoublesBlockLoader extends SourceBlockLoader {
-        public DoublesBlockLoader(ValueFetcher fetcher, LeafIteratorLookup lookup, SourceFieldMapper.Mode sourceMode) {
-            super(fetcher, lookup, sourceMode);
+        public DoublesBlockLoader(ValueFetcher fetcher, LeafIteratorLookup lookup) {
+            super(fetcher, lookup);
         }
 
         @Override
@@ -315,8 +304,8 @@ public String toString() {
      * Load {@code int}s from {@code _source}.
      */
     public static class IntsBlockLoader extends SourceBlockLoader {
-        public IntsBlockLoader(ValueFetcher fetcher, LeafIteratorLookup lookup, SourceFieldMapper.Mode sourceMode) {
-            super(fetcher, lookup, sourceMode);
+        public IntsBlockLoader(ValueFetcher fetcher, LeafIteratorLookup lookup) {
+            super(fetcher, lookup);
         }
 
         @Override
@@ -355,8 +344,8 @@ public String toString() {
      * Load {@code long}s from {@code _source}.
      */
     public static class LongsBlockLoader extends SourceBlockLoader {
-        public LongsBlockLoader(ValueFetcher fetcher, LeafIteratorLookup lookup, SourceFieldMapper.Mode sourceMode) {
-            super(fetcher, lookup, sourceMode);
+        public LongsBlockLoader(ValueFetcher fetcher, LeafIteratorLookup lookup) {
+            super(fetcher, lookup);
         }
 
         @Override

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

@@ -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, blContext.indexSettings().getIndexMappingSourceMode());
+            return new BlockSourceReader.BooleansBlockLoader(fetcher, lookup);
         }
 
         @Override

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

@@ -793,8 +793,7 @@ public BlockLoader blockLoader(BlockLoaderContext blContext) {
             BlockSourceReader.LeafIteratorLookup lookup = isStored() || isIndexed()
                 ? BlockSourceReader.lookupFromFieldNames(blContext.fieldNames(), name())
                 : BlockSourceReader.lookupMatchingAll();
-            var sourceMode = blContext.indexSettings().getIndexMappingSourceMode();
-            return new BlockSourceReader.LongsBlockLoader(sourceValueFetcher(blContext.sourcePaths(name())), lookup, sourceMode);
+            return new BlockSourceReader.LongsBlockLoader(sourceValueFetcher(blContext.sourcePaths(name())), lookup);
         }
 
         @Override

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

@@ -634,8 +634,7 @@ public BlockLoader blockLoader(BlockLoaderContext blContext) {
                 return new BlockStoredFieldsReader.BytesFromBytesRefsBlockLoader(name());
             }
             SourceValueFetcher fetcher = sourceValueFetcher(blContext.sourcePaths(name()));
-            var sourceMode = blContext.indexSettings().getIndexMappingSourceMode();
-            return new BlockSourceReader.BytesRefsBlockLoader(fetcher, sourceBlockLoaderLookup(blContext), sourceMode);
+            return new BlockSourceReader.BytesRefsBlockLoader(fetcher, sourceBlockLoaderLookup(blContext));
         }
 
         private BlockSourceReader.LeafIteratorLookup sourceBlockLoaderLookup(BlockLoaderContext blContext) {

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

@@ -462,12 +462,8 @@ BlockLoader blockLoaderFromDocValues(String fieldName) {
             }
 
             @Override
-            BlockLoader blockLoaderFromSource(
-                SourceValueFetcher sourceValueFetcher,
-                BlockSourceReader.LeafIteratorLookup lookup,
-                SourceFieldMapper.Mode sourceMode
-            ) {
-                return new BlockSourceReader.DoublesBlockLoader(sourceValueFetcher, lookup, sourceMode);
+            BlockLoader blockLoaderFromSource(SourceValueFetcher sourceValueFetcher, BlockSourceReader.LeafIteratorLookup lookup) {
+                return new BlockSourceReader.DoublesBlockLoader(sourceValueFetcher, lookup);
             }
         },
         FLOAT("float", NumericType.FLOAT) {
@@ -650,12 +646,8 @@ BlockLoader blockLoaderFromDocValues(String fieldName) {
             }
 
             @Override
-            BlockLoader blockLoaderFromSource(
-                SourceValueFetcher sourceValueFetcher,
-                BlockSourceReader.LeafIteratorLookup lookup,
-                SourceFieldMapper.Mode sourceMode
-            ) {
-                return new BlockSourceReader.DoublesBlockLoader(sourceValueFetcher, lookup, sourceMode);
+            BlockLoader blockLoaderFromSource(SourceValueFetcher sourceValueFetcher, BlockSourceReader.LeafIteratorLookup lookup) {
+                return new BlockSourceReader.DoublesBlockLoader(sourceValueFetcher, lookup);
             }
         },
         DOUBLE("double", NumericType.DOUBLE) {
@@ -804,12 +796,8 @@ BlockLoader blockLoaderFromDocValues(String fieldName) {
             }
 
             @Override
-            BlockLoader blockLoaderFromSource(
-                SourceValueFetcher sourceValueFetcher,
-                BlockSourceReader.LeafIteratorLookup lookup,
-                SourceFieldMapper.Mode sourceMode
-            ) {
-                return new BlockSourceReader.DoublesBlockLoader(sourceValueFetcher, lookup, sourceMode);
+            BlockLoader blockLoaderFromSource(SourceValueFetcher sourceValueFetcher, BlockSourceReader.LeafIteratorLookup lookup) {
+                return new BlockSourceReader.DoublesBlockLoader(sourceValueFetcher, lookup);
             }
         },
         BYTE("byte", NumericType.BYTE) {
@@ -921,12 +909,8 @@ BlockLoader blockLoaderFromDocValues(String fieldName) {
             }
 
             @Override
-            BlockLoader blockLoaderFromSource(
-                SourceValueFetcher sourceValueFetcher,
-                BlockSourceReader.LeafIteratorLookup lookup,
-                SourceFieldMapper.Mode sourceMode
-            ) {
-                return new BlockSourceReader.IntsBlockLoader(sourceValueFetcher, lookup, sourceMode);
+            BlockLoader blockLoaderFromSource(SourceValueFetcher sourceValueFetcher, BlockSourceReader.LeafIteratorLookup lookup) {
+                return new BlockSourceReader.IntsBlockLoader(sourceValueFetcher, lookup);
             }
 
             private boolean isOutOfRange(Object value) {
@@ -1038,12 +1022,8 @@ BlockLoader blockLoaderFromDocValues(String fieldName) {
             }
 
             @Override
-            BlockLoader blockLoaderFromSource(
-                SourceValueFetcher sourceValueFetcher,
-                BlockSourceReader.LeafIteratorLookup lookup,
-                SourceFieldMapper.Mode sourceMode
-            ) {
-                return new BlockSourceReader.IntsBlockLoader(sourceValueFetcher, lookup, sourceMode);
+            BlockLoader blockLoaderFromSource(SourceValueFetcher sourceValueFetcher, BlockSourceReader.LeafIteratorLookup lookup) {
+                return new BlockSourceReader.IntsBlockLoader(sourceValueFetcher, lookup);
             }
 
             private boolean isOutOfRange(Object value) {
@@ -1229,12 +1209,8 @@ BlockLoader blockLoaderFromDocValues(String fieldName) {
             }
 
             @Override
-            BlockLoader blockLoaderFromSource(
-                SourceValueFetcher sourceValueFetcher,
-                BlockSourceReader.LeafIteratorLookup lookup,
-                SourceFieldMapper.Mode sourceMode
-            ) {
-                return new BlockSourceReader.IntsBlockLoader(sourceValueFetcher, lookup, sourceMode);
+            BlockLoader blockLoaderFromSource(SourceValueFetcher sourceValueFetcher, BlockSourceReader.LeafIteratorLookup lookup) {
+                return new BlockSourceReader.IntsBlockLoader(sourceValueFetcher, lookup);
             }
         },
         LONG("long", NumericType.LONG) {
@@ -1380,12 +1356,8 @@ BlockLoader blockLoaderFromDocValues(String fieldName) {
             }
 
             @Override
-            BlockLoader blockLoaderFromSource(
-                SourceValueFetcher sourceValueFetcher,
-                BlockSourceReader.LeafIteratorLookup lookup,
-                SourceFieldMapper.Mode sourceMode
-            ) {
-                return new BlockSourceReader.LongsBlockLoader(sourceValueFetcher, lookup, sourceMode);
+            BlockLoader blockLoaderFromSource(SourceValueFetcher sourceValueFetcher, BlockSourceReader.LeafIteratorLookup lookup) {
+                return new BlockSourceReader.LongsBlockLoader(sourceValueFetcher, lookup);
             }
 
             private boolean isOutOfRange(Object value) {
@@ -1663,11 +1635,7 @@ protected void writeValue(XContentBuilder b, long value) throws IOException {
 
         abstract BlockLoader blockLoaderFromDocValues(String fieldName);
 
-        abstract BlockLoader blockLoaderFromSource(
-            SourceValueFetcher sourceValueFetcher,
-            BlockSourceReader.LeafIteratorLookup lookup,
-            SourceFieldMapper.Mode sourceMode
-        );
+        abstract BlockLoader blockLoaderFromSource(SourceValueFetcher sourceValueFetcher, BlockSourceReader.LeafIteratorLookup lookup);
     }
 
     public static class NumberFieldType extends SimpleMappedFieldType {
@@ -1806,8 +1774,7 @@ public BlockLoader blockLoader(BlockLoaderContext blContext) {
             BlockSourceReader.LeafIteratorLookup lookup = isStored() || isIndexed()
                 ? BlockSourceReader.lookupFromFieldNames(blContext.fieldNames(), name())
                 : BlockSourceReader.lookupMatchingAll();
-            var sourceMode = blContext.indexSettings().getIndexMappingSourceMode();
-            return type.blockLoaderFromSource(sourceValueFetcher(blContext.sourcePaths(name())), lookup, sourceMode);
+            return type.blockLoaderFromSource(sourceValueFetcher(blContext.sourcePaths(name())), lookup);
         }
 
         @Override

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

@@ -1007,20 +1007,8 @@ protected String delegatingTo() {
             if (isStored()) {
                 return new BlockStoredFieldsReader.BytesFromStringsBlockLoader(name());
             }
-            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. 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()));
-            var sourceMode = blContext.indexSettings().getIndexMappingSourceMode();
-            return new BlockSourceReader.BytesRefsBlockLoader(fetcher, blockReaderDisiLookup(blContext), sourceMode);
+            return new BlockSourceReader.BytesRefsBlockLoader(fetcher, blockReaderDisiLookup(blContext));
         }
 
         /**

server/src/test/java/org/elasticsearch/index/mapper/BlockSourceReaderTests.java

@@ -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, null);
+        BlockLoader loader = new BlockSourceReader.BytesRefsBlockLoader(valueFetcher, lookup);
         assertThat(loader.columnAtATimeReader(ctx), nullValue());
         BlockLoader.RowStrideReader reader = loader.rowStrideReader(ctx);
         assertThat(loader.rowStrideStoredFieldSpec(), equalTo(StoredFieldsSpec.NEEDS_SOURCE));