Skip to content

Commit cff9f97

Browse files
javannajavanna
committed
Skip explain fetch sub phase when request holds only suggestions (elastic#41739)
In case a search request holds only the suggest section, the query phase is skipped and only the suggest phase is executed instead. There will never be hits returned, and in case the explain flag is set to true, the explain sub phase throws a null pointer exception as the query is null. Usually a null query is replaced with a match all query as part of SearchContext#preProcess which is though skipped as well with suggest only searches. To address this, we skip the explain sub fetch phase for search requests that only requested suggestions. Closes elastic#31260
1 parent 9d56598 commit cff9f97

File tree

3 files changed

+62
-61
lines changed

3 files changed

+62
-61
lines changed

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ public final class ExplainFetchSubPhase implements FetchSubPhase {
3333

3434
@Override
3535
public void hitExecute(SearchContext context, HitContext hitContext) {
36-
if (context.explain() == false) {
36+
if (context.explain() == false || context.hasOnlySuggest()) {
3737
return;
3838
}
3939
try {

‎server/src/main/java/org/elasticsearch/search/query/QueryPhase.java

-1
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,6 @@ public void preProcess(SearchContext context) {
9595
public void execute(SearchContext searchContext) throws QueryPhaseExecutionException {
9696
if (searchContext.hasOnlySuggest()) {
9797
suggestPhase.execute(searchContext);
98-
// TODO: fix this once we can fetch docs for suggestions
9998
searchContext.queryResult().topDocs(new TopDocsAndMaxScore(
10099
new TopDocs(new TotalHits(0, TotalHits.Relation.EQUAL_TO), Lucene.EMPTY_SCORE_DOCS), Float.NaN),
101100
new DocValueFormat[0]);

‎server/src/test/java/org/elasticsearch/search/suggest/CompletionSuggestSearchIT.java

+61-59
Original file line numberDiff line numberDiff line change
@@ -379,17 +379,13 @@ public void testThatWeightsAreWorking() throws Exception {
379379
public void testThatWeightMustBeAnInteger() throws Exception {
380380
createIndexAndMapping(completionMappingBuilder);
381381

382-
try {
383-
client().prepareIndex(INDEX, TYPE, "1").setSource(jsonBuilder()
384-
.startObject().startObject(FIELD)
385-
.startArray("input").value("sth").endArray()
386-
.field("weight", 2.5)
387-
.endObject().endObject()
388-
).get();
389-
fail("Indexing with a float weight was successful, but should not be");
390-
} catch (MapperParsingException e) {
391-
assertThat(e.toString(), containsString("2.5"));
392-
}
382+
MapperParsingException e = expectThrows(MapperParsingException.class,
383+
() -> client().prepareIndex(INDEX, TYPE, "1").setSource(jsonBuilder()
384+
.startObject().startObject(FIELD)
385+
.startArray("input").value("sth").endArray()
386+
.field("weight", 2.5)
387+
.endObject().endObject()).get());
388+
assertThat(e.toString(), containsString("2.5"));
393389
}
394390

395391
public void testThatWeightCanBeAString() throws Exception {
@@ -422,34 +418,28 @@ public void testThatWeightCanBeAString() throws Exception {
422418
public void testThatWeightMustNotBeANonNumberString() throws Exception {
423419
createIndexAndMapping(completionMappingBuilder);
424420

425-
try {
426-
client().prepareIndex(INDEX, TYPE, "1").setSource(jsonBuilder()
427-
.startObject().startObject(FIELD)
428-
.startArray("input").value("sth").endArray()
429-
.field("weight", "thisIsNotValid")
430-
.endObject().endObject()
431-
).get();
432-
fail("Indexing with a non-number representing string as weight was successful, but should not be");
433-
} catch (MapperParsingException e) {
434-
assertThat(e.toString(), containsString("thisIsNotValid"));
435-
}
421+
MapperParsingException e = expectThrows(MapperParsingException.class,
422+
() -> client().prepareIndex(INDEX, TYPE, "1").setSource(jsonBuilder()
423+
.startObject().startObject(FIELD)
424+
.startArray("input").value("sth").endArray()
425+
.field("weight", "thisIsNotValid")
426+
.endObject().endObject()
427+
).get());
428+
assertThat(e.toString(), containsString("thisIsNotValid"));
436429
}
437430

438431
public void testThatWeightAsStringMustBeInt() throws Exception {
439432
createIndexAndMapping(completionMappingBuilder);
440433

441434
String weight = String.valueOf(Long.MAX_VALUE - 4);
442-
try {
443-
client().prepareIndex(INDEX, TYPE, "1").setSource(jsonBuilder()
444-
.startObject().startObject(FIELD)
445-
.startArray("input").value("testing").endArray()
446-
.field("weight", weight)
447-
.endObject().endObject()
448-
).get();
449-
fail("Indexing with weight string representing value > Int.MAX_VALUE was successful, but should not be");
450-
} catch (MapperParsingException e) {
451-
assertThat(e.toString(), containsString(weight));
452-
}
435+
436+
MapperParsingException e = expectThrows(MapperParsingException.class,
437+
() -> client().prepareIndex(INDEX, TYPE, "1").setSource(jsonBuilder()
438+
.startObject().startObject(FIELD)
439+
.startArray("input").value("testing").endArray()
440+
.field("weight", weight)
441+
.endObject().endObject()).get());
442+
assertThat(e.toString(), containsString(weight));
453443
}
454444

455445
public void testThatInputCanBeAStringInsteadOfAnArray() throws Exception {
@@ -821,13 +811,11 @@ public void testThatSortingOnCompletionFieldReturnsUsefulException() throws Exce
821811
).get();
822812

823813
refresh();
824-
try {
825-
client().prepareSearch(INDEX).setTypes(TYPE).addSort(new FieldSortBuilder(FIELD)).get();
826-
fail("Expected an exception due to trying to sort on completion field, but did not happen");
827-
} catch (SearchPhaseExecutionException e) {
828-
assertThat(e.status().getStatus(), is(400));
829-
assertThat(e.toString(), containsString("Fielddata is not supported on field [" + FIELD + "] of type [completion]"));
830-
}
814+
815+
SearchPhaseExecutionException e = expectThrows(SearchPhaseExecutionException.class,
816+
() -> client().prepareSearch(INDEX).setTypes(TYPE).addSort(new FieldSortBuilder(FIELD)).get());
817+
assertThat(e.status().getStatus(), is(400));
818+
assertThat(e.toString(), containsString("Fielddata is not supported on field [" + FIELD + "] of type [completion]"));
831819
}
832820

833821
public void testThatSuggestStopFilterWorks() throws Exception {
@@ -1118,17 +1106,12 @@ public void testReservedChars() throws IOException {
11181106
.endObject()).get());
11191107
// can cause stack overflow without the default max_input_length
11201108
String string = "foo" + (char) 0x00 + "bar";
1121-
try {
1122-
client().prepareIndex(INDEX, TYPE, "1").setSource(jsonBuilder()
1123-
.startObject().startObject(FIELD)
1124-
.startArray("input").value(string).endArray()
1125-
.field("output", "foobar")
1126-
.endObject().endObject()
1127-
).get();
1128-
fail("Expected MapperParsingException");
1129-
} catch (MapperParsingException e) {
1130-
assertThat(e.getMessage(), containsString("failed to parse"));
1131-
}
1109+
MapperParsingException e = expectThrows(MapperParsingException.class,
1110+
() -> client().prepareIndex(INDEX, TYPE, "1").setSource(jsonBuilder().startObject().startObject(FIELD)
1111+
.startArray("input").value(string).endArray()
1112+
.field("output", "foobar")
1113+
.endObject().endObject()).get());
1114+
assertThat(e.getMessage(), containsString("failed to parse"));
11321115
}
11331116

11341117
// see #5930
@@ -1147,14 +1130,10 @@ public void testIssue5930() throws IOException {
11471130
.endObject()
11481131
).setRefreshPolicy(IMMEDIATE).get();
11491132

1150-
try {
1151-
client().prepareSearch(INDEX).addAggregation(AggregationBuilders.terms("suggest_agg").field(FIELD)
1152-
.collectMode(randomFrom(SubAggCollectionMode.values()))).get();
1153-
// Exception must be thrown
1154-
assertFalse(true);
1155-
} catch (SearchPhaseExecutionException e) {
1156-
assertThat(e.toString(), containsString("Fielddata is not supported on field [" + FIELD + "] of type [completion]"));
1157-
}
1133+
SearchPhaseExecutionException e = expectThrows(SearchPhaseExecutionException.class,
1134+
() -> client().prepareSearch(INDEX).addAggregation(AggregationBuilders.terms("suggest_agg").field(FIELD)
1135+
.collectMode(randomFrom(SubAggCollectionMode.values()))).get());
1136+
assertThat(e.toString(), containsString("Fielddata is not supported on field [" + FIELD + "] of type [completion]"));
11581137
}
11591138

11601139
public void testMultiDocSuggestions() throws Exception {
@@ -1205,6 +1184,29 @@ public void testSuggestWithFieldAlias() throws Exception {
12051184
assertSuggestions("suggestion", suggestionBuilder, "apple");
12061185
}
12071186

1187+
public void testSuggestOnlyExplain() throws Exception {
1188+
final CompletionMappingBuilder mapping = new CompletionMappingBuilder();
1189+
createIndexAndMapping(mapping);
1190+
int numDocs = 10;
1191+
List<IndexRequestBuilder> indexRequestBuilders = new ArrayList<>();
1192+
for (int i = 1; i <= numDocs; i++) {
1193+
indexRequestBuilders.add(client().prepareIndex(INDEX, TYPE, "" + i)
1194+
.setSource(jsonBuilder()
1195+
.startObject()
1196+
.startObject(FIELD)
1197+
.field("input", "suggestion" + i)
1198+
.field("weight", i)
1199+
.endObject()
1200+
.endObject()
1201+
));
1202+
}
1203+
indexRandom(true, indexRequestBuilders);
1204+
CompletionSuggestionBuilder prefix = SuggestBuilders.completionSuggestion(FIELD).prefix("sugg");
1205+
SearchResponse searchResponse = client().prepareSearch(INDEX).setExplain(true)
1206+
.suggest(new SuggestBuilder().addSuggestion("foo", prefix)).get();
1207+
assertSuggestions(searchResponse, "foo", "suggestion10", "suggestion9", "suggestion8", "suggestion7", "suggestion6");
1208+
}
1209+
12081210
public static boolean isReservedChar(char c) {
12091211
switch (c) {
12101212
case '\u001F':

0 commit comments

Comments
 (0)