Skip to content

Commit ff67baa

Browse files
jtibshiranijtibshirani
committed
Make sure shared source always represents the top-level root document. (#66725)
We started passing down the root document's _source when processing nested hits, to avoid reloading and reparsing the root source for each hit. Unfortunately the approach did not work when there are multiple layers of `inner_hits`. In this case, the second-layer inner hit received its immediate parent's source instead of the root source. This parent source is filtered to just contain the parts corresponding to the nested document, but the source parsing logic is designed to always operate on the top-level root source. This caused failures when loading the second-layer inner hits. This PR makes sure to always pass the root document's _source when processing inner hits, even if there are multiple layers.
1 parent 0fad7f6 commit ff67baa

File tree

4 files changed

+158
-11
lines changed

4 files changed

+158
-11
lines changed

‎rest-api-spec/src/main/resources/rest-api-spec/test/search.inner_hits/10_basic.yml

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,3 +92,69 @@ setup:
9292
- match: { hits.hits.0.fields._seq_no: [1] }
9393
- match: { hits.hits.0.inner_hits.nested_field.hits.hits.0._version: 2 }
9494
- match: { hits.hits.0.inner_hits.nested_field.hits.hits.0.fields._seq_no: [1] }
95+
96+
---
97+
"Inner hits with disabled _source":
98+
- do:
99+
indices.create:
100+
index: disabled_source
101+
body:
102+
mappings:
103+
_source:
104+
enabled: false
105+
properties:
106+
nested_field:
107+
type: nested
108+
properties:
109+
sub_nested_field:
110+
type: nested
111+
112+
- do:
113+
index:
114+
index: disabled_source
115+
id: 1
116+
body:
117+
nested_field:
118+
field: value
119+
sub_nested_field:
120+
field: value
121+
122+
- do:
123+
indices.refresh: {}
124+
125+
- do:
126+
search:
127+
index: disabled_source
128+
rest_total_hits_as_int: true
129+
body:
130+
query:
131+
nested:
132+
path: "nested_field.sub_nested_field"
133+
query: { match_all: {}}
134+
inner_hits: {}
135+
- match: { hits.total: 1 }
136+
- match: { hits.hits.0._id: "1" }
137+
- match: { hits.hits.0.inner_hits.nested_field\.sub_nested_field.hits.hits.0._id: "1" }
138+
- is_false: hits.hits.0.inner_hits.nested_field\.sub_nested_field.hits.hits.0._source
139+
140+
- do:
141+
search:
142+
index: disabled_source
143+
rest_total_hits_as_int: true
144+
body:
145+
query:
146+
nested:
147+
path: "nested_field"
148+
inner_hits: {}
149+
query:
150+
nested:
151+
path: "nested_field.sub_nested_field"
152+
query: { match_all: {}}
153+
inner_hits: {}
154+
155+
- match: { hits.total: 1 }
156+
- match: { hits.hits.0._id: "1" }
157+
- match: { hits.hits.0.inner_hits.nested_field.hits.hits.0._id: "1" }
158+
- is_false: hits.hits.0.inner_hits.nested_field.hits.hits.0._source
159+
- match: { hits.hits.0.inner_hits.nested_field.hits.hits.0.inner_hits.nested_field\.sub_nested_field.hits.hits.0._id: "1" }
160+
- is_false: hits.hits.0.inner_hits.nested_field.hits.hits.0.inner_hits.nested_field\.sub_nested_field.hits.hits.0._source

‎server/src/internalClusterTest/java/org/elasticsearch/search/fetch/subphase/InnerHitsIT.java

Lines changed: 64 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -279,10 +279,14 @@ public void testNestedMultipleLayers() throws Exception {
279279
requests.add(client().prepareIndex("articles", "article", "1").setSource(jsonBuilder().startObject()
280280
.field("title", "quick brown fox")
281281
.startArray("comments")
282-
.startObject()
283-
.field("message", "fox eat quick")
284-
.startArray("remarks").startObject().field("message", "good").endObject().endArray()
285-
.endObject()
282+
.startObject()
283+
.field("message", "fox eat quick")
284+
.startArray("remarks").startObject().field("message", "good").endObject().endArray()
285+
.endObject()
286+
.startObject()
287+
.field("message", "hippo is hungry")
288+
.startArray("remarks").startObject().field("message", "neutral").endObject().endArray()
289+
.endObject()
286290
.endArray()
287291
.endObject()));
288292
requests.add(client().prepareIndex("articles", "article", "2").setSource(jsonBuilder().startObject()
@@ -296,6 +300,7 @@ public void testNestedMultipleLayers() throws Exception {
296300
.endObject()));
297301
indexRandom(true, requests);
298302

303+
// Check we can load the first doubly-nested document.
299304
SearchResponse response = client().prepareSearch("articles")
300305
.setQuery(
301306
nestedQuery("comments",
@@ -322,6 +327,33 @@ public void testNestedMultipleLayers() throws Exception {
322327
assertThat(innerHits.getAt(0).getNestedIdentity().getChild().getField().string(), equalTo("remarks"));
323328
assertThat(innerHits.getAt(0).getNestedIdentity().getChild().getOffset(), equalTo(0));
324329

330+
// Check we can load the second doubly-nested document.
331+
response = client().prepareSearch("articles")
332+
.setQuery(
333+
nestedQuery("comments",
334+
nestedQuery("comments.remarks", matchQuery("comments.remarks.message", "neutral"), ScoreMode.Avg)
335+
.innerHit(new InnerHitBuilder("remark")),
336+
ScoreMode.Avg).innerHit(new InnerHitBuilder())
337+
).get();
338+
assertNoFailures(response);
339+
assertHitCount(response, 1);
340+
assertSearchHit(response, 1, hasId("1"));
341+
assertThat(response.getHits().getAt(0).getInnerHits().size(), equalTo(1));
342+
innerHits = response.getHits().getAt(0).getInnerHits().get("comments");
343+
assertThat(innerHits.getTotalHits().value, equalTo(1L));
344+
assertThat(innerHits.getHits().length, equalTo(1));
345+
assertThat(innerHits.getAt(0).getId(), equalTo("1"));
346+
assertThat(innerHits.getAt(0).getNestedIdentity().getField().string(), equalTo("comments"));
347+
assertThat(innerHits.getAt(0).getNestedIdentity().getOffset(), equalTo(1));
348+
innerHits = innerHits.getAt(0).getInnerHits().get("remark");
349+
assertThat(innerHits.getTotalHits().value, equalTo(1L));
350+
assertThat(innerHits.getHits().length, equalTo(1));
351+
assertThat(innerHits.getAt(0).getId(), equalTo("1"));
352+
assertThat(innerHits.getAt(0).getNestedIdentity().getField().string(), equalTo("comments"));
353+
assertThat(innerHits.getAt(0).getNestedIdentity().getOffset(), equalTo(1));
354+
assertThat(innerHits.getAt(0).getNestedIdentity().getChild().getField().string(), equalTo("remarks"));
355+
assertThat(innerHits.getAt(0).getNestedIdentity().getChild().getOffset(), equalTo(0));
356+
325357
// Directly refer to the second level:
326358
response = client().prepareSearch("articles")
327359
.setQuery(nestedQuery("comments.remarks", matchQuery("comments.remarks.message", "bad"), ScoreMode.Avg)
@@ -364,6 +396,34 @@ public void testNestedMultipleLayers() throws Exception {
364396
assertThat(innerHits.getAt(0).getNestedIdentity().getOffset(), equalTo(0));
365397
assertThat(innerHits.getAt(0).getNestedIdentity().getChild().getField().string(), equalTo("remarks"));
366398
assertThat(innerHits.getAt(0).getNestedIdentity().getChild().getOffset(), equalTo(0));
399+
400+
// Check that inner hits contain _source even when it's disabled on the parent request.
401+
response = client().prepareSearch("articles")
402+
.setFetchSource(false)
403+
.setQuery(
404+
nestedQuery("comments",
405+
nestedQuery("comments.remarks", matchQuery("comments.remarks.message", "good"), ScoreMode.Avg)
406+
.innerHit(new InnerHitBuilder("remark")), ScoreMode.Avg)
407+
.innerHit(new InnerHitBuilder())
408+
).get();
409+
assertNoFailures(response);
410+
innerHits = response.getHits().getAt(0).getInnerHits().get("comments");
411+
innerHits = innerHits.getAt(0).getInnerHits().get("remark");
412+
assertNotNull(innerHits.getAt(0).getSourceAsMap());
413+
assertFalse(innerHits.getAt(0).getSourceAsMap().isEmpty());
414+
415+
response = client().prepareSearch("articles")
416+
.setQuery(
417+
nestedQuery("comments",
418+
nestedQuery("comments.remarks", matchQuery("comments.remarks.message", "good"), ScoreMode.Avg)
419+
.innerHit(new InnerHitBuilder("remark")), ScoreMode.Avg)
420+
.innerHit(new InnerHitBuilder().setFetchSourceContext(new FetchSourceContext(false)))
421+
).get();
422+
assertNoFailures(response);
423+
innerHits = response.getHits().getAt(0).getInnerHits().get("comments");
424+
innerHits = innerHits.getAt(0).getInnerHits().get("remark");
425+
assertNotNull(innerHits.getAt(0).getSourceAsMap());
426+
assertFalse(innerHits.getAt(0).getSourceAsMap().isEmpty());
367427
}
368428

369429
// Issue #9723

‎server/src/main/java/org/elasticsearch/search/fetch/FetchContext.java

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,11 +29,13 @@
2929
import org.elasticsearch.search.fetch.subphase.FetchSourceContext;
3030
import org.elasticsearch.search.fetch.subphase.FieldAndFormat;
3131
import org.elasticsearch.search.fetch.subphase.InnerHitsContext;
32+
import org.elasticsearch.search.fetch.subphase.InnerHitsContext.InnerHitSubContext;
3233
import org.elasticsearch.search.fetch.subphase.ScriptFieldsContext;
3334
import org.elasticsearch.search.fetch.subphase.highlight.SearchHighlightContext;
3435
import org.elasticsearch.search.internal.ContextIndexSearcher;
3536
import org.elasticsearch.search.internal.SearchContext;
3637
import org.elasticsearch.search.lookup.SearchLookup;
38+
import org.elasticsearch.search.lookup.SourceLookup;
3739
import org.elasticsearch.search.rescore.RescoreContext;
3840

3941
import java.util.Collections;
@@ -204,4 +206,23 @@ public ScriptFieldsContext scriptFields() {
204206
public SearchExtBuilder getSearchExt(String name) {
205207
return searchContext.getSearchExt(name);
206208
}
209+
210+
/**
211+
* For a hit document that's being processed, return the source lookup representing the
212+
* root document. This method is used to pass down the root source when processing this
213+
* document's nested inner hits.
214+
*
215+
* @param hitContext The context of the hit that's being processed.
216+
*/
217+
public SourceLookup getRootSourceLookup(FetchSubPhase.HitContext hitContext) {
218+
// Usually the root source simply belongs to the hit we're processing. But if
219+
// there are multiple layers of inner hits and we're in a nested context, then
220+
// the root source is found on the inner hits context.
221+
if (searchContext instanceof InnerHitSubContext && hitContext.hit().getNestedIdentity() != null) {
222+
InnerHitSubContext innerHitsContext = (InnerHitSubContext) searchContext;
223+
return innerHitsContext.getRootLookup();
224+
} else {
225+
return hitContext.sourceLookup();
226+
}
227+
}
207228
}

‎server/src/main/java/org/elasticsearch/search/fetch/subphase/InnerHitsPhase.java

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -59,16 +59,16 @@ public void setNextReader(LeafReaderContext readerContext) {
5959

6060
@Override
6161
public void process(HitContext hitContext) throws IOException {
62-
hitExecute(innerHits, hitContext);
62+
SearchHit hit = hitContext.hit();
63+
SourceLookup rootLookup = searchContext.getRootSourceLookup(hitContext);
64+
hitExecute(innerHits, hit, rootLookup);
6365
}
6466
};
6567
}
6668

67-
private void hitExecute(Map<String, InnerHitsContext.InnerHitSubContext> innerHits, HitContext hitContext) throws IOException {
68-
69-
SearchHit hit = hitContext.hit();
70-
SourceLookup sourceLookup = hitContext.sourceLookup();
71-
69+
private void hitExecute(Map<String, InnerHitsContext.InnerHitSubContext> innerHits,
70+
SearchHit hit,
71+
SourceLookup rootLookup) throws IOException {
7272
for (Map.Entry<String, InnerHitsContext.InnerHitSubContext> entry : innerHits.entrySet()) {
7373
InnerHitsContext.InnerHitSubContext innerHitsContext = entry.getValue();
7474
TopDocsAndMaxScore topDoc = innerHitsContext.topDocs(hit);
@@ -84,7 +84,7 @@ private void hitExecute(Map<String, InnerHitsContext.InnerHitSubContext> innerHi
8484
}
8585
innerHitsContext.docIdsToLoad(docIdsToLoad, 0, docIdsToLoad.length);
8686
innerHitsContext.setRootId(new Uid(hit.getType(), hit.getId()));
87-
innerHitsContext.setRootLookup(sourceLookup);
87+
innerHitsContext.setRootLookup(rootLookup);
8888

8989
fetchPhase.execute(innerHitsContext);
9090
FetchSearchResult fetchResult = innerHitsContext.fetchResult();

0 commit comments

Comments
 (0)