Allow FetchSubPhaseProcessors to report their required stored fields (#…

…91269)

Loading of stored fields is currently handled directly in FetchPhase, with
some fairly complex logic examining various bits of the FetchContext to work
out what fields need to be loaded. This is further complicated by synthetic
source, which may have its own stored field requirements.

This commit tries to separate out these concerns a little by adding a new
StoredFieldsSpec record that holds information about which stored fields
need to be loaded. Each FetchSubPhaseProcessor can now report a
StoredFieldsSpec detailing what its requirements are, and these specs can
be merged together, along with requirements from a SourceLoader, to
determine up-front what fields should be loaded by the StoredFieldLoader.
The stored fields themselves are added into the SearchHit by a new
StoredFieldsPhase, which handles alias resolution and value post-
processing. The logic to determine when source should be loaded and
when not, based on the presence of script fields or stored fields, is
moved into FetchContext, which highlights some inconsistencies that
can be fixed in follow-up commits.

docs/reference/search/profile.asciidoc

@@ -193,6 +193,17 @@ The API returns the following result:
               "debug": {
                 "fast_path": 5
               }
+            },
+            {
+              "type": "StoredFieldsPhase",
+              "description": "",
+              "time_in_nanos": 5310,
+              "breakdown": {
+                "next_reader": 745,
+                "next_reader_count": 1,
+                "process": 4445,
+                "process_count": 5
+              }
             }
           ]
         }
@@ -1011,6 +1022,17 @@ And here is the fetch profile:
               "debug": {
                 "fast_path": 4
               }
+            },
+            {
+              "type": "StoredFieldsPhase",
+              "description": "",
+              "time_in_nanos": 5310,
+              "breakdown": {
+                "next_reader": 745,
+                "next_reader_count": 1,
+                "process": 4445,
+                "process_count": 5
+              }
             }
           ]
         }

modules/lang-mustache/src/test/java/org/elasticsearch/script/mustache/SearchTemplateResponseTests.java

@@ -23,7 +23,6 @@
 import org.elasticsearch.xcontent.XContentType;
 
 import java.io.IOException;
-import java.util.Collections;
 import java.util.function.Predicate;
 
 import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertToXContentEquivalent;
@@ -158,7 +157,7 @@ public void testSourceToXContent() throws IOException {
     }
 
     public void testSearchResponseToXContent() throws IOException {
-        SearchHit hit = new SearchHit(1, "id", Collections.emptyMap(), Collections.emptyMap());
+        SearchHit hit = new SearchHit(1, "id");
         hit.score(2.0f);
         SearchHit[] hits = new SearchHit[] { hit };
 

modules/parent-join/build.gradle

@@ -18,4 +18,8 @@ restResources {
   restApi {
     include '_common', 'bulk', 'cluster', 'nodes', 'indices', 'index', 'search'
   }
-}
+}
+
+tasks.named("yamlRestTestV7CompatTransform").configure { task ->
+  task.skipTest("/30_inner_hits/profile fetch", "profile output has changed")
+}

modules/parent-join/src/yamlRestTest/resources/rest-api-spec/test/30_inner_hits.yml

@@ -141,7 +141,7 @@ profile fetch:
   - gt: { profile.shards.0.fetch.breakdown.load_stored_fields_count: 0 }
   - gt: { profile.shards.0.fetch.breakdown.load_stored_fields: 0 }
   - match: { profile.shards.0.fetch.debug.stored_fields: [_id, _routing, _source] }
-  - length: { profile.shards.0.fetch.children: 2 }
+  - length: { profile.shards.0.fetch.children: 3 }
   - match: { profile.shards.0.fetch.children.0.type: FetchSourcePhase }
   - gt: { profile.shards.0.fetch.children.0.breakdown.next_reader_count: 0 }
   - gt: { profile.shards.0.fetch.children.0.breakdown.next_reader: 0 }
@@ -152,3 +152,4 @@ profile fetch:
   - gt: { profile.shards.0.fetch.children.1.breakdown.next_reader: 0 }
   - gt: { profile.shards.0.fetch.children.1.breakdown.next_reader_count: 0 }
   - gt: { profile.shards.0.fetch.children.1.breakdown.next_reader: 0 }
+  - match: { profile.shards.0.fetch.children.2.type: StoredFieldsPhase }

modules/percolator/src/main/java/org/elasticsearch/percolator/PercolatorHighlightSubFetchPhase.java

@@ -18,6 +18,7 @@
 import org.elasticsearch.search.fetch.FetchContext;
 import org.elasticsearch.search.fetch.FetchSubPhase;
 import org.elasticsearch.search.fetch.FetchSubPhaseProcessor;
+import org.elasticsearch.search.fetch.StoredFieldsSpec;
 import org.elasticsearch.search.fetch.subphase.highlight.HighlightField;
 import org.elasticsearch.search.fetch.subphase.highlight.HighlightPhase;
 import org.elasticsearch.search.fetch.subphase.highlight.Highlighter;
@@ -59,6 +60,11 @@ public void setNextReader(LeafReaderContext readerContext) {
                 this.ctx = readerContext;
             }
 
+            @Override
+            public StoredFieldsSpec storedFieldsSpec() {
+                return StoredFieldsSpec.NO_REQUIREMENTS;
+            }
+
             @Override
             public void process(HitContext hit) throws IOException {
                 boolean singlePercolateQuery = percolateQueries.size() == 1;
@@ -83,9 +89,10 @@ public void process(HitContext hit) throws IOException {
                             int slot = (int) matchedSlot;
                             BytesReference document = percolateQuery.getDocuments().get(slot);
                             HitContext subContext = new HitContext(
-                                new SearchHit(slot, "unknown", Collections.emptyMap(), Collections.emptyMap()),
+                                new SearchHit(slot, "unknown"),
                                 percolatorLeafReaderContext,
                                 slot,
+                                Map.of(),
                                 Source.fromBytes(document)
                             );
                             // force source because MemoryIndex does not store fields

modules/percolator/src/main/java/org/elasticsearch/percolator/PercolatorMatchedSlotSubFetchPhase.java

@@ -26,6 +26,7 @@
 import org.elasticsearch.search.fetch.FetchContext;
 import org.elasticsearch.search.fetch.FetchSubPhase;
 import org.elasticsearch.search.fetch.FetchSubPhaseProcessor;
+import org.elasticsearch.search.fetch.StoredFieldsSpec;
 
 import java.io.IOException;
 import java.util.ArrayList;
@@ -69,6 +70,11 @@ public void setNextReader(LeafReaderContext readerContext) {
                 this.ctx = readerContext;
             }
 
+            @Override
+            public StoredFieldsSpec storedFieldsSpec() {
+                return StoredFieldsSpec.NO_REQUIREMENTS;
+            }
+
             @Override
             public void process(HitContext hitContext) throws IOException {
                 for (PercolateContext pc : percolateContexts) {

modules/percolator/src/test/java/org/elasticsearch/percolator/PercolatorMatchedSlotSubFetchPhaseTests.java

@@ -34,6 +34,7 @@
 import org.elasticsearch.test.ESTestCase;
 
 import java.util.Collections;
+import java.util.Map;
 import java.util.stream.IntStream;
 
 import static org.mockito.Mockito.mock;
@@ -55,7 +56,7 @@ public void testHitsExecute() throws Exception {
                 LeafReaderContext context = reader.leaves().get(0);
                 // A match:
                 {
-                    HitContext hit = new HitContext(new SearchHit(0), context, 0, Source.empty(null));
+                    HitContext hit = new HitContext(new SearchHit(0), context, 0, Map.of(), Source.empty(null));
                     PercolateQuery.QueryStore queryStore = ctx -> docId -> new TermQuery(new Term("field", "value"));
                     MemoryIndex memoryIndex = new MemoryIndex();
                     memoryIndex.addField("field", "value", new WhitespaceAnalyzer());
@@ -86,7 +87,7 @@ public void testHitsExecute() throws Exception {
 
                 // No match:
                 {
-                    HitContext hit = new HitContext(new SearchHit(0), context, 0, Source.empty(null));
+                    HitContext hit = new HitContext(new SearchHit(0), context, 0, Map.of(), Source.empty(null));
                     PercolateQuery.QueryStore queryStore = ctx -> docId -> new TermQuery(new Term("field", "value"));
                     MemoryIndex memoryIndex = new MemoryIndex();
                     memoryIndex.addField("field", "value1", new WhitespaceAnalyzer());
@@ -116,7 +117,7 @@ public void testHitsExecute() throws Exception {
 
                 // No query:
                 {
-                    HitContext hit = new HitContext(new SearchHit(0), context, 0, Source.empty(null));
+                    HitContext hit = new HitContext(new SearchHit(0), context, 0, Map.of(), Source.empty(null));
                     PercolateQuery.QueryStore queryStore = ctx -> docId -> null;
                     MemoryIndex memoryIndex = new MemoryIndex();
                     memoryIndex.addField("field", "value", new WhitespaceAnalyzer());

modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/DiscountedCumulativeGainTests.java

@@ -60,7 +60,7 @@ public void testDCGAt() {
         SearchHit[] hits = new SearchHit[6];
         for (int i = 0; i < 6; i++) {
             rated.add(new RatedDocument("index", Integer.toString(i), relevanceRatings[i]));
-            hits[i] = new SearchHit(i, Integer.toString(i), Collections.emptyMap(), Collections.emptyMap());
+            hits[i] = new SearchHit(i, Integer.toString(i));
             hits[i].shard(new SearchShardTarget("testnode", new ShardId("index", "uuid", 0), null));
         }
         DiscountedCumulativeGain dcg = new DiscountedCumulativeGain();
@@ -110,7 +110,7 @@ public void testDCGAtSixMissingRatings() {
                     rated.add(new RatedDocument("index", Integer.toString(i), relevanceRatings[i]));
                 }
             }
-            hits[i] = new SearchHit(i, Integer.toString(i), Collections.emptyMap(), Collections.emptyMap());
+            hits[i] = new SearchHit(i, Integer.toString(i));
             hits[i].shard(new SearchShardTarget("testnode", new ShardId("index", "uuid", 0), null));
         }
         DiscountedCumulativeGain dcg = new DiscountedCumulativeGain();
@@ -167,7 +167,7 @@ public void testDCGAtFourMoreRatings() {
         // only create four hits
         SearchHit[] hits = new SearchHit[4];
         for (int i = 0; i < 4; i++) {
-            hits[i] = new SearchHit(i, Integer.toString(i), Collections.emptyMap(), Collections.emptyMap());
+            hits[i] = new SearchHit(i, Integer.toString(i));
             hits[i].shard(new SearchShardTarget("testnode", new ShardId("index", "uuid", 0), null));
         }
         DiscountedCumulativeGain dcg = new DiscountedCumulativeGain();

modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/ExpectedReciprocalRankTests.java

@@ -103,7 +103,7 @@ private SearchHit[] createSearchHits(List<RatedDocument> rated, Integer[] releva
             if (relevanceRatings[i] != null) {
                 rated.add(new RatedDocument("index", Integer.toString(i), relevanceRatings[i]));
             }
-            hits[i] = new SearchHit(i, Integer.toString(i), Collections.emptyMap(), Collections.emptyMap());
+            hits[i] = new SearchHit(i, Integer.toString(i));
             hits[i].shard(new SearchShardTarget("testnode", new ShardId("index", "uuid", 0), null));
         }
         return hits;

modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/MeanReciprocalRankTests.java

@@ -190,7 +190,7 @@ public void testXContentParsingIsNotLenient() throws IOException {
     private static SearchHit[] createSearchHits(int from, int to, String index) {
         SearchHit[] hits = new SearchHit[to + 1 - from];
         for (int i = from; i <= to; i++) {
-            hits[i] = new SearchHit(i, i + "", Collections.emptyMap(), Collections.emptyMap());
+            hits[i] = new SearchHit(i, i + "");
             hits[i].shard(new SearchShardTarget("testnode", new ShardId(index, "uuid", 0), null));
         }
         return hits;

modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/PrecisionAtKTests.java

@@ -100,7 +100,7 @@ public void testIgnoreUnlabeled() {
         rated.add(createRatedDoc("test", "1", RELEVANT_RATING));
         // add an unlabeled search hit
         SearchHit[] searchHits = Arrays.copyOf(toSearchHits(rated, "test"), 3);
-        searchHits[2] = new SearchHit(2, "2", Collections.emptyMap(), Collections.emptyMap());
+        searchHits[2] = new SearchHit(2, "2");
         searchHits[2].shard(new SearchShardTarget("testnode", new ShardId("index", "uuid", 0), null));
 
         EvalQueryQuality evaluated = (new PrecisionAtK()).evaluate("id", searchHits, rated);
@@ -119,7 +119,7 @@ public void testIgnoreUnlabeled() {
     public void testNoRatedDocs() throws Exception {
         SearchHit[] hits = new SearchHit[5];
         for (int i = 0; i < 5; i++) {
-            hits[i] = new SearchHit(i, i + "", Collections.emptyMap(), Collections.emptyMap());
+            hits[i] = new SearchHit(i, i + "");
             hits[i].shard(new SearchShardTarget("testnode", new ShardId("index", "uuid", 0), null));
         }
         EvalQueryQuality evaluated = (new PrecisionAtK()).evaluate("id", hits, Collections.emptyList());
@@ -248,7 +248,7 @@ private static PrecisionAtK mutate(PrecisionAtK original) {
     private static SearchHit[] toSearchHits(List<RatedDocument> rated, String index) {
         SearchHit[] hits = new SearchHit[rated.size()];
         for (int i = 0; i < rated.size(); i++) {
-            hits[i] = new SearchHit(i, i + "", Collections.emptyMap(), Collections.emptyMap());
+            hits[i] = new SearchHit(i, i + "");
             hits[i].shard(new SearchShardTarget("testnode", new ShardId(index, "uuid", 0), null));
         }
         return hits;

modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RankEvalResponseTests.java

@@ -197,7 +197,7 @@ public void testToXContent() throws IOException {
     }
 
     private static RatedSearchHit searchHit(String index, int docId, Integer rating) {
-        SearchHit hit = new SearchHit(docId, docId + "", Collections.emptyMap(), Collections.emptyMap());
+        SearchHit hit = new SearchHit(docId, docId + "");
         hit.shard(new SearchShardTarget("testnode", new ShardId(index, "uuid", 0), null));
         hit.score(1.0f);
         return new RatedSearchHit(hit, rating != null ? OptionalInt.of(rating) : OptionalInt.empty());

modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RatedSearchHitTests.java

@@ -26,12 +26,7 @@ public class RatedSearchHitTests extends ESTestCase {
 
     public static RatedSearchHit randomRatedSearchHit() {
         OptionalInt rating = randomBoolean() ? OptionalInt.empty() : OptionalInt.of(randomIntBetween(0, 5));
-        SearchHit searchHit = new SearchHit(
-            randomIntBetween(0, 10),
-            randomAlphaOfLength(10),
-            Collections.emptyMap(),
-            Collections.emptyMap()
-        );
+        SearchHit searchHit = new SearchHit(randomIntBetween(0, 10), randomAlphaOfLength(10));
         RatedSearchHit ratedSearchHit = new RatedSearchHit(searchHit, rating);
         return ratedSearchHit;
     }
@@ -41,12 +36,7 @@ private static RatedSearchHit mutateTestItem(RatedSearchHit original) {
         SearchHit hit = original.getSearchHit();
         switch (randomIntBetween(0, 1)) {
             case 0 -> rating = rating.isPresent() ? OptionalInt.of(rating.getAsInt() + 1) : OptionalInt.of(randomInt(5));
-            case 1 -> hit = new SearchHit(
-                hit.docId(),
-                hit.getId() + randomAlphaOfLength(10),
-                Collections.emptyMap(),
-                Collections.emptyMap()
-            );
+            case 1 -> hit = new SearchHit(hit.docId(), hit.getId() + randomAlphaOfLength(10));
             default -> throw new IllegalStateException("The test should only allow two parameters mutated");
         }
         return new RatedSearchHit(hit, rating);

modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RecallAtKTests.java

@@ -102,7 +102,7 @@ public void testNoRatedDocs() throws Exception {
         int k = 5;
         SearchHit[] hits = new SearchHit[k];
         for (int i = 0; i < k; i++) {
-            hits[i] = new SearchHit(i, i + "", Collections.emptyMap(), Collections.emptyMap());
+            hits[i] = new SearchHit(i, i + "");
             hits[i].shard(new SearchShardTarget("testnode", new ShardId("index", "uuid", 0), null));
         }
 
@@ -216,7 +216,7 @@ private static RecallAtK mutate(RecallAtK original) {
     private static SearchHit[] toSearchHits(List<RatedDocument> rated, String index) {
         SearchHit[] hits = new SearchHit[rated.size()];
         for (int i = 0; i < rated.size(); i++) {
-            hits[i] = new SearchHit(i, i + "", Collections.emptyMap(), Collections.emptyMap());
+            hits[i] = new SearchHit(i, i + "");
             hits[i].shard(new SearchShardTarget("testnode", new ShardId(index, "uuid", 0), null));
         }
         return hits;

modules/reindex/src/test/java/org/elasticsearch/reindex/AsyncBulkByScrollActionTests.java

@@ -565,7 +565,7 @@ protected RequestWrapper<?> buildRequest(Hit doc) {
         action.start();
 
         // create a simulated response.
-        SearchHit hit = new SearchHit(0, "id", emptyMap(), emptyMap()).sourceRef(new BytesArray("{}"));
+        SearchHit hit = new SearchHit(0, "id").sourceRef(new BytesArray("{}"));
         SearchHits hits = new SearchHits(
             IntStream.range(0, 100).mapToObj(i -> hit).toArray(SearchHit[]::new),
             new TotalHits(0, TotalHits.Relation.EQUAL_TO),

modules/reindex/src/test/java/org/elasticsearch/reindex/ClientScrollableHitSourceTests.java

@@ -46,7 +46,6 @@
 import java.util.function.Function;
 import java.util.stream.IntStream;
 
-import static java.util.Collections.emptyMap;
 import static org.apache.lucene.tests.util.TestUtil.randomSimpleString;
 import static org.elasticsearch.core.TimeValue.timeValueSeconds;
 import static org.hamcrest.Matchers.instanceOf;
@@ -160,7 +159,7 @@ public void testScrollKeepAlive() {
 
     private SearchResponse createSearchResponse() {
         // create a simulated response.
-        SearchHit hit = new SearchHit(0, "id", emptyMap(), emptyMap()).sourceRef(new BytesArray("{}"));
+        SearchHit hit = new SearchHit(0, "id").sourceRef(new BytesArray("{}"));
         SearchHits hits = new SearchHits(
             IntStream.range(0, randomIntBetween(0, 20)).mapToObj(i -> hit).toArray(SearchHit[]::new),
             new TotalHits(0, TotalHits.Relation.EQUAL_TO),

rest-api-spec/build.gradle

@@ -75,6 +75,9 @@ tasks.named("yamlRestTestV7CompatTransform").configure { task ->
   task.skipTest("search_shards/10_basic/Search shards aliases with and without filters", "Filter representation no longer outputs default boosts")
   task.skipTest("migration/10_get_feature_upgrade_status/Get feature upgrade status", "Awaits backport")
   task.skipTest("search/330_fetch_fields/Test disable source", "Error no longer thrown")
+  task.skipTest("search/370_profile/fetch fields", "profile output has changed")
+  task.skipTest("search/370_profile/fetch source", "profile output has changed")
+  task.skipTest("search/370_profile/fetch nested source", "profile output has changed")
   task.skipTest("search/240_date_nanos/doc value fields are working as expected across date and date_nanos fields", "Fetching docvalues field multiple times is no longer allowed")
 
   task.replaceValueInMatch("_type", "_doc")