Skip to content

Commit dbc7102

Browse files
authored
Fix inner hits retrieval when stored fields are disabled (_none_) (#33018)
Now that types are unique per mapping we can retrieve the document mapper without referencing the type. This fixes an NPE when stored fields are disabled. For 6x we'll need a different fix since mappings can still have multiple types. Relates #32941
1 parent 17c7f99 commit dbc7102

File tree

5 files changed

+51
-17
lines changed

5 files changed

+51
-17
lines changed

rest-api-spec/src/main/resources/rest-api-spec/test/search/100_stored_fields.yml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,5 @@ setup:
3939
stored_fields: "_none_"
4040

4141
- is_false: hits.hits.0._id
42-
- is_false: hits.hits.0._type
4342
- is_false: hits.hits.0._source
4443

server/src/main/java/org/elasticsearch/search/fetch/FetchPhase.java

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -192,20 +192,15 @@ private SearchHit createSearchHit(SearchContext context,
192192
int subDocId,
193193
Map<String, Set<String>> storedToRequestedFields,
194194
LeafReaderContext subReaderContext) {
195+
DocumentMapper documentMapper = context.mapperService().documentMapper();
196+
Text typeText = documentMapper.typeText();
195197
if (fieldsVisitor == null) {
196-
return new SearchHit(docId);
198+
return new SearchHit(docId, null, typeText, null);
197199
}
198200

199201
Map<String, DocumentField> searchFields = getSearchFields(context, fieldsVisitor, subDocId,
200202
storedToRequestedFields, subReaderContext);
201203

202-
DocumentMapper documentMapper = context.mapperService().documentMapper(fieldsVisitor.uid().type());
203-
Text typeText;
204-
if (documentMapper == null) {
205-
typeText = new Text(fieldsVisitor.uid().type());
206-
} else {
207-
typeText = documentMapper.typeText();
208-
}
209204
SearchHit searchHit = new SearchHit(docId, fieldsVisitor.uid().id(), typeText, searchFields);
210205
// Set _source if requested.
211206
SourceLookup sourceLookup = context.lookup().source();
@@ -275,7 +270,7 @@ private SearchHit createNestedSearchHit(SearchContext context,
275270
storedToRequestedFields, subReaderContext);
276271
}
277272

278-
DocumentMapper documentMapper = context.mapperService().documentMapper(uid.type());
273+
DocumentMapper documentMapper = context.mapperService().documentMapper();
279274
SourceLookup sourceLookup = context.lookup().source();
280275
sourceLookup.setSegmentAndDocument(subReaderContext, nestedSubDocId);
281276

server/src/test/java/org/elasticsearch/search/aggregations/metrics/TopHitsIT.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1052,7 +1052,7 @@ public void testNoStoredFields() throws Exception {
10521052
for (SearchHit hit : hits) {
10531053
assertThat(hit.getSourceAsMap(), nullValue());
10541054
assertThat(hit.getId(), nullValue());
1055-
assertThat(hit.getType(), nullValue());
1055+
assertThat(hit.getType(), equalTo("type"));
10561056
}
10571057
}
10581058
}

server/src/test/java/org/elasticsearch/search/source/MetadataFetchingIT.java

Lines changed: 44 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,20 @@
1818
*/
1919
package org.elasticsearch.search.source;
2020

21+
import org.apache.lucene.search.join.ScoreMode;
2122
import org.elasticsearch.ExceptionsHelper;
2223
import org.elasticsearch.action.search.SearchPhaseExecutionException;
2324
import org.elasticsearch.action.search.SearchResponse;
25+
import org.elasticsearch.index.query.InnerHitBuilder;
26+
import org.elasticsearch.index.query.NestedQueryBuilder;
27+
import org.elasticsearch.index.query.TermQueryBuilder;
2428
import org.elasticsearch.search.SearchContextException;
29+
import org.elasticsearch.search.SearchHits;
30+
import org.elasticsearch.search.fetch.subphase.FetchSourceContext;
2531
import org.elasticsearch.test.ESIntegTestCase;
2632

33+
import java.util.Collections;
34+
2735
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
2836
import static org.hamcrest.Matchers.equalTo;
2937
import static org.hamcrest.Matchers.nullValue;
@@ -33,7 +41,7 @@ public void testSimple() {
3341
assertAcked(prepareCreate("test"));
3442
ensureGreen();
3543

36-
client().prepareIndex("test", "type1", "1").setSource("field", "value").execute().actionGet();
44+
client().prepareIndex("test", "_doc", "1").setSource("field", "value").execute().actionGet();
3745
refresh();
3846

3947
SearchResponse response = client()
@@ -42,23 +50,53 @@ public void testSimple() {
4250
.setFetchSource(false)
4351
.get();
4452
assertThat(response.getHits().getAt(0).getId(), nullValue());
45-
assertThat(response.getHits().getAt(0).getType(), nullValue());
53+
assertThat(response.getHits().getAt(0).getType(), equalTo("_doc"));
4654
assertThat(response.getHits().getAt(0).getSourceAsString(), nullValue());
4755

4856
response = client()
4957
.prepareSearch("test")
5058
.storedFields("_none_")
5159
.get();
5260
assertThat(response.getHits().getAt(0).getId(), nullValue());
53-
assertThat(response.getHits().getAt(0).getType(), nullValue());
61+
assertThat(response.getHits().getAt(0).getType(), equalTo("_doc"));
62+
assertThat(response.getHits().getAt(0).getSourceAsString(), nullValue());
63+
}
64+
65+
public void testInnerHits() {
66+
assertAcked(prepareCreate("test").addMapping("_doc", "nested", "type=nested"));
67+
ensureGreen();
68+
client().prepareIndex("test", "_doc", "1")
69+
.setSource("field", "value", "nested", Collections.singletonMap("title", "foo")).execute().actionGet();
70+
refresh();
71+
72+
SearchResponse response = client()
73+
.prepareSearch("test")
74+
.storedFields("_none_")
75+
.setFetchSource(false)
76+
.setQuery(
77+
new NestedQueryBuilder("nested", new TermQueryBuilder("nested.title", "foo"), ScoreMode.Total)
78+
.innerHit(new InnerHitBuilder()
79+
.setStoredFieldNames(Collections.singletonList("_none_"))
80+
.setFetchSourceContext(new FetchSourceContext(false)))
81+
)
82+
.get();
83+
assertThat(response.getHits().totalHits, equalTo(1L));
84+
assertThat(response.getHits().getAt(0).getId(), nullValue());
85+
assertThat(response.getHits().getAt(0).getType(), equalTo("_doc"));
5486
assertThat(response.getHits().getAt(0).getSourceAsString(), nullValue());
87+
assertThat(response.getHits().getAt(0).getInnerHits().size(), equalTo(1));
88+
SearchHits hits = response.getHits().getAt(0).getInnerHits().get("nested");
89+
assertThat(hits.totalHits, equalTo(1L));
90+
assertThat(hits.getAt(0).getId(), nullValue());
91+
assertThat(hits.getAt(0).getType(), equalTo("_doc"));
92+
assertThat(hits.getAt(0).getSourceAsString(), nullValue());
5593
}
5694

5795
public void testWithRouting() {
5896
assertAcked(prepareCreate("test"));
5997
ensureGreen();
6098

61-
client().prepareIndex("test", "type1", "1").setSource("field", "value").setRouting("toto").execute().actionGet();
99+
client().prepareIndex("test", "_doc", "1").setSource("field", "value").setRouting("toto").execute().actionGet();
62100
refresh();
63101

64102
SearchResponse response = client()
@@ -67,7 +105,7 @@ public void testWithRouting() {
67105
.setFetchSource(false)
68106
.get();
69107
assertThat(response.getHits().getAt(0).getId(), nullValue());
70-
assertThat(response.getHits().getAt(0).getType(), nullValue());
108+
assertThat(response.getHits().getAt(0).getType(), equalTo("_doc"));
71109
assertThat(response.getHits().getAt(0).field("_routing"), nullValue());
72110
assertThat(response.getHits().getAt(0).getSourceAsString(), nullValue());
73111

@@ -76,7 +114,7 @@ public void testWithRouting() {
76114
.storedFields("_none_")
77115
.get();
78116
assertThat(response.getHits().getAt(0).getId(), nullValue());
79-
assertThat(response.getHits().getAt(0).getType(), nullValue());
117+
assertThat(response.getHits().getAt(0).getType(), equalTo("_doc"));
80118
assertThat(response.getHits().getAt(0).getSourceAsString(), nullValue());
81119
}
82120

test/framework/src/main/java/org/elasticsearch/search/aggregations/AggregatorTestCase.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
import org.elasticsearch.common.lease.Releasables;
3838
import org.elasticsearch.common.lucene.index.ElasticsearchDirectoryReader;
3939
import org.elasticsearch.common.settings.Settings;
40+
import org.elasticsearch.common.text.Text;
4041
import org.elasticsearch.common.util.MockBigArrays;
4142
import org.elasticsearch.common.util.MockPageCacheRecycler;
4243
import org.elasticsearch.index.Index;
@@ -137,6 +138,7 @@ protected AggregatorFactory<?> createAggregatorFactory(Query query,
137138
when(mapperService.getIndexSettings()).thenReturn(indexSettings);
138139
when(mapperService.hasNested()).thenReturn(false);
139140
DocumentMapper mapper = mock(DocumentMapper.class);
141+
when(mapper.typeText()).thenReturn(new Text(TYPE_NAME));
140142
when(mapper.type()).thenReturn(TYPE_NAME);
141143
when(mapperService.documentMapper()).thenReturn(mapper);
142144
when(searchContext.mapperService()).thenReturn(mapperService);

0 commit comments

Comments
 (0)