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
This commit is contained in:
Jim Ferenczi 2018-09-04 16:25:52 +02:00 committed by GitHub
parent 17c7f99343
commit dbc7102c86
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 51 additions and 17 deletions

View File

@ -39,6 +39,5 @@ setup:
stored_fields: "_none_"
- is_false: hits.hits.0._id
- is_false: hits.hits.0._type
- is_false: hits.hits.0._source

View File

@ -192,20 +192,15 @@ public class FetchPhase implements SearchPhase {
int subDocId,
Map<String, Set<String>> storedToRequestedFields,
LeafReaderContext subReaderContext) {
DocumentMapper documentMapper = context.mapperService().documentMapper();
Text typeText = documentMapper.typeText();
if (fieldsVisitor == null) {
return new SearchHit(docId);
return new SearchHit(docId, null, typeText, null);
}
Map<String, DocumentField> searchFields = getSearchFields(context, fieldsVisitor, subDocId,
storedToRequestedFields, subReaderContext);
DocumentMapper documentMapper = context.mapperService().documentMapper(fieldsVisitor.uid().type());
Text typeText;
if (documentMapper == null) {
typeText = new Text(fieldsVisitor.uid().type());
} else {
typeText = documentMapper.typeText();
}
SearchHit searchHit = new SearchHit(docId, fieldsVisitor.uid().id(), typeText, searchFields);
// Set _source if requested.
SourceLookup sourceLookup = context.lookup().source();
@ -275,7 +270,7 @@ public class FetchPhase implements SearchPhase {
storedToRequestedFields, subReaderContext);
}
DocumentMapper documentMapper = context.mapperService().documentMapper(uid.type());
DocumentMapper documentMapper = context.mapperService().documentMapper();
SourceLookup sourceLookup = context.lookup().source();
sourceLookup.setSegmentAndDocument(subReaderContext, nestedSubDocId);

View File

@ -1052,7 +1052,7 @@ public class TopHitsIT extends ESIntegTestCase {
for (SearchHit hit : hits) {
assertThat(hit.getSourceAsMap(), nullValue());
assertThat(hit.getId(), nullValue());
assertThat(hit.getType(), nullValue());
assertThat(hit.getType(), equalTo("type"));
}
}
}

View File

@ -18,12 +18,20 @@
*/
package org.elasticsearch.search.source;
import org.apache.lucene.search.join.ScoreMode;
import org.elasticsearch.ExceptionsHelper;
import org.elasticsearch.action.search.SearchPhaseExecutionException;
import org.elasticsearch.action.search.SearchResponse;
import org.elasticsearch.index.query.InnerHitBuilder;
import org.elasticsearch.index.query.NestedQueryBuilder;
import org.elasticsearch.index.query.TermQueryBuilder;
import org.elasticsearch.search.SearchContextException;
import org.elasticsearch.search.SearchHits;
import org.elasticsearch.search.fetch.subphase.FetchSourceContext;
import org.elasticsearch.test.ESIntegTestCase;
import java.util.Collections;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.nullValue;
@ -33,7 +41,7 @@ public class MetadataFetchingIT extends ESIntegTestCase {
assertAcked(prepareCreate("test"));
ensureGreen();
client().prepareIndex("test", "type1", "1").setSource("field", "value").execute().actionGet();
client().prepareIndex("test", "_doc", "1").setSource("field", "value").execute().actionGet();
refresh();
SearchResponse response = client()
@ -42,7 +50,7 @@ public class MetadataFetchingIT extends ESIntegTestCase {
.setFetchSource(false)
.get();
assertThat(response.getHits().getAt(0).getId(), nullValue());
assertThat(response.getHits().getAt(0).getType(), nullValue());
assertThat(response.getHits().getAt(0).getType(), equalTo("_doc"));
assertThat(response.getHits().getAt(0).getSourceAsString(), nullValue());
response = client()
@ -50,15 +58,45 @@ public class MetadataFetchingIT extends ESIntegTestCase {
.storedFields("_none_")
.get();
assertThat(response.getHits().getAt(0).getId(), nullValue());
assertThat(response.getHits().getAt(0).getType(), nullValue());
assertThat(response.getHits().getAt(0).getType(), equalTo("_doc"));
assertThat(response.getHits().getAt(0).getSourceAsString(), nullValue());
}
public void testInnerHits() {
assertAcked(prepareCreate("test").addMapping("_doc", "nested", "type=nested"));
ensureGreen();
client().prepareIndex("test", "_doc", "1")
.setSource("field", "value", "nested", Collections.singletonMap("title", "foo")).execute().actionGet();
refresh();
SearchResponse response = client()
.prepareSearch("test")
.storedFields("_none_")
.setFetchSource(false)
.setQuery(
new NestedQueryBuilder("nested", new TermQueryBuilder("nested.title", "foo"), ScoreMode.Total)
.innerHit(new InnerHitBuilder()
.setStoredFieldNames(Collections.singletonList("_none_"))
.setFetchSourceContext(new FetchSourceContext(false)))
)
.get();
assertThat(response.getHits().totalHits, equalTo(1L));
assertThat(response.getHits().getAt(0).getId(), nullValue());
assertThat(response.getHits().getAt(0).getType(), equalTo("_doc"));
assertThat(response.getHits().getAt(0).getSourceAsString(), nullValue());
assertThat(response.getHits().getAt(0).getInnerHits().size(), equalTo(1));
SearchHits hits = response.getHits().getAt(0).getInnerHits().get("nested");
assertThat(hits.totalHits, equalTo(1L));
assertThat(hits.getAt(0).getId(), nullValue());
assertThat(hits.getAt(0).getType(), equalTo("_doc"));
assertThat(hits.getAt(0).getSourceAsString(), nullValue());
}
public void testWithRouting() {
assertAcked(prepareCreate("test"));
ensureGreen();
client().prepareIndex("test", "type1", "1").setSource("field", "value").setRouting("toto").execute().actionGet();
client().prepareIndex("test", "_doc", "1").setSource("field", "value").setRouting("toto").execute().actionGet();
refresh();
SearchResponse response = client()
@ -67,7 +105,7 @@ public class MetadataFetchingIT extends ESIntegTestCase {
.setFetchSource(false)
.get();
assertThat(response.getHits().getAt(0).getId(), nullValue());
assertThat(response.getHits().getAt(0).getType(), nullValue());
assertThat(response.getHits().getAt(0).getType(), equalTo("_doc"));
assertThat(response.getHits().getAt(0).field("_routing"), nullValue());
assertThat(response.getHits().getAt(0).getSourceAsString(), nullValue());
@ -76,7 +114,7 @@ public class MetadataFetchingIT extends ESIntegTestCase {
.storedFields("_none_")
.get();
assertThat(response.getHits().getAt(0).getId(), nullValue());
assertThat(response.getHits().getAt(0).getType(), nullValue());
assertThat(response.getHits().getAt(0).getType(), equalTo("_doc"));
assertThat(response.getHits().getAt(0).getSourceAsString(), nullValue());
}

View File

@ -37,6 +37,7 @@ import org.elasticsearch.common.lease.Releasable;
import org.elasticsearch.common.lease.Releasables;
import org.elasticsearch.common.lucene.index.ElasticsearchDirectoryReader;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.text.Text;
import org.elasticsearch.common.util.MockBigArrays;
import org.elasticsearch.common.util.MockPageCacheRecycler;
import org.elasticsearch.index.Index;
@ -137,6 +138,7 @@ public abstract class AggregatorTestCase extends ESTestCase {
when(mapperService.getIndexSettings()).thenReturn(indexSettings);
when(mapperService.hasNested()).thenReturn(false);
DocumentMapper mapper = mock(DocumentMapper.class);
when(mapper.typeText()).thenReturn(new Text(TYPE_NAME));
when(mapper.type()).thenReturn(TYPE_NAME);
when(mapperService.documentMapper()).thenReturn(mapper);
when(searchContext.mapperService()).thenReturn(mapperService);