Skip to content

Commit

Permalink
Skip explain fetch sub phase when request holds only suggestions (#41739
Browse files Browse the repository at this point in the history 
)

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 #31260
  • Loading branch information
@javanna
javanna committed May 22, 2019
1 parent 3416cda commit 39d4c7c
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 61 deletions.
2 changes: 1 addition & 1 deletion server/src/main/java/org/elasticsearch/search/fetch/subphase/ExplainFetchSubPhase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public final class ExplainFetchSubPhase implements FetchSubPhase {

@Override
public void hitExecute(SearchContext context, HitContext hitContext) {
if (context.explain() == false) {
if (context.explain() == false || context.hasOnlySuggest()) {
return;
}
try {
Expand Down
1 change: 0 additions & 1 deletion server/src/main/java/org/elasticsearch/search/query/QueryPhase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,6 @@ public void preProcess(SearchContext context) {
public void execute(SearchContext searchContext) throws QueryPhaseExecutionException {
if (searchContext.hasOnlySuggest()) {
suggestPhase.execute(searchContext);
// TODO: fix this once we can fetch docs for suggestions
searchContext.queryResult().topDocs(new TopDocsAndMaxScore(
new TopDocs(new TotalHits(0, TotalHits.Relation.EQUAL_TO), Lucene.EMPTY_SCORE_DOCS), Float.NaN),
new DocValueFormat[0]);
Expand Down
120 changes: 61 additions & 59 deletions server/src/test/java/org/elasticsearch/search/suggest/CompletionSuggestSearchIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -379,17 +379,13 @@ public void testThatWeightsAreWorking() throws Exception {
public void testThatWeightMustBeAnInteger() throws Exception {
createIndexAndMapping(completionMappingBuilder);

try {
client().prepareIndex(INDEX, TYPE, "1").setSource(jsonBuilder()
.startObject().startObject(FIELD)
.startArray("input").value("sth").endArray()
.field("weight", 2.5)
.endObject().endObject()
).get();
fail("Indexing with a float weight was successful, but should not be");
} catch (MapperParsingException e) {
assertThat(e.toString(), containsString("2.5"));
}
MapperParsingException e = expectThrows(MapperParsingException.class,
() -> client().prepareIndex(INDEX, TYPE, "1").setSource(jsonBuilder()
.startObject().startObject(FIELD)
.startArray("input").value("sth").endArray()
.field("weight", 2.5)
.endObject().endObject()).get());
assertThat(e.toString(), containsString("2.5"));
}

public void testThatWeightCanBeAString() throws Exception {
Expand Down Expand Up @@ -422,34 +418,28 @@ public void testThatWeightCanBeAString() throws Exception {
public void testThatWeightMustNotBeANonNumberString() throws Exception {
createIndexAndMapping(completionMappingBuilder);

try {
client().prepareIndex(INDEX, TYPE, "1").setSource(jsonBuilder()
.startObject().startObject(FIELD)
.startArray("input").value("sth").endArray()
.field("weight", "thisIsNotValid")
.endObject().endObject()
).get();
fail("Indexing with a non-number representing string as weight was successful, but should not be");
} catch (MapperParsingException e) {
assertThat(e.toString(), containsString("thisIsNotValid"));
}
MapperParsingException e = expectThrows(MapperParsingException.class,
() -> client().prepareIndex(INDEX, TYPE, "1").setSource(jsonBuilder()
.startObject().startObject(FIELD)
.startArray("input").value("sth").endArray()
.field("weight", "thisIsNotValid")
.endObject().endObject()
).get());
assertThat(e.toString(), containsString("thisIsNotValid"));
}

public void testThatWeightAsStringMustBeInt() throws Exception {
createIndexAndMapping(completionMappingBuilder);

String weight = String.valueOf(Long.MAX_VALUE - 4);
try {
client().prepareIndex(INDEX, TYPE, "1").setSource(jsonBuilder()
.startObject().startObject(FIELD)
.startArray("input").value("testing").endArray()
.field("weight", weight)
.endObject().endObject()
).get();
fail("Indexing with weight string representing value > Int.MAX_VALUE was successful, but should not be");
} catch (MapperParsingException e) {
assertThat(e.toString(), containsString(weight));
}

MapperParsingException e = expectThrows(MapperParsingException.class,
() -> client().prepareIndex(INDEX, TYPE, "1").setSource(jsonBuilder()
.startObject().startObject(FIELD)
.startArray("input").value("testing").endArray()
.field("weight", weight)
.endObject().endObject()).get());
assertThat(e.toString(), containsString(weight));
}

public void testThatInputCanBeAStringInsteadOfAnArray() throws Exception {
Expand Down Expand Up @@ -821,13 +811,11 @@ public void testThatSortingOnCompletionFieldReturnsUsefulException() throws Exce
).get();

refresh();
try {
client().prepareSearch(INDEX).setTypes(TYPE).addSort(new FieldSortBuilder(FIELD)).get();
fail("Expected an exception due to trying to sort on completion field, but did not happen");
} catch (SearchPhaseExecutionException e) {
assertThat(e.status().getStatus(), is(400));
assertThat(e.toString(), containsString("Fielddata is not supported on field [" + FIELD + "] of type [completion]"));
}

SearchPhaseExecutionException e = expectThrows(SearchPhaseExecutionException.class,
() -> client().prepareSearch(INDEX).setTypes(TYPE).addSort(new FieldSortBuilder(FIELD)).get());
assertThat(e.status().getStatus(), is(400));
assertThat(e.toString(), containsString("Fielddata is not supported on field [" + FIELD + "] of type [completion]"));
}

public void testThatSuggestStopFilterWorks() throws Exception {
Expand Down Expand Up @@ -1118,17 +1106,12 @@ public void testReservedChars() throws IOException {
.endObject()).get());
// can cause stack overflow without the default max_input_length
String string = "foo" + (char) 0x00 + "bar";
try {
client().prepareIndex(INDEX, TYPE, "1").setSource(jsonBuilder()
.startObject().startObject(FIELD)
.startArray("input").value(string).endArray()
.field("output", "foobar")
.endObject().endObject()
).get();
fail("Expected MapperParsingException");
} catch (MapperParsingException e) {
assertThat(e.getMessage(), containsString("failed to parse"));
}
MapperParsingException e = expectThrows(MapperParsingException.class,
() -> client().prepareIndex(INDEX, TYPE, "1").setSource(jsonBuilder().startObject().startObject(FIELD)
.startArray("input").value(string).endArray()
.field("output", "foobar")
.endObject().endObject()).get());
assertThat(e.getMessage(), containsString("failed to parse"));
}

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

try {
client().prepareSearch(INDEX).addAggregation(AggregationBuilders.terms("suggest_agg").field(FIELD)
.collectMode(randomFrom(SubAggCollectionMode.values()))).get();
// Exception must be thrown
assertFalse(true);
} catch (SearchPhaseExecutionException e) {
assertThat(e.toString(), containsString("Fielddata is not supported on field [" + FIELD + "] of type [completion]"));
}
SearchPhaseExecutionException e = expectThrows(SearchPhaseExecutionException.class,
() -> client().prepareSearch(INDEX).addAggregation(AggregationBuilders.terms("suggest_agg").field(FIELD)
.collectMode(randomFrom(SubAggCollectionMode.values()))).get());
assertThat(e.toString(), containsString("Fielddata is not supported on field [" + FIELD + "] of type [completion]"));
}

public void testMultiDocSuggestions() throws Exception {
Expand Down Expand Up @@ -1205,6 +1184,29 @@ public void testSuggestWithFieldAlias() throws Exception {
assertSuggestions("suggestion", suggestionBuilder, "apple");
}

public void testSuggestOnlyExplain() throws Exception {
final CompletionMappingBuilder mapping = new CompletionMappingBuilder();
createIndexAndMapping(mapping);
int numDocs = 10;
List<IndexRequestBuilder> indexRequestBuilders = new ArrayList<>();
for (int i = 1; i <= numDocs; i++) {
indexRequestBuilders.add(client().prepareIndex(INDEX, TYPE, "" + i)
.setSource(jsonBuilder()
.startObject()
.startObject(FIELD)
.field("input", "suggestion" + i)
.field("weight", i)
.endObject()
.endObject()
));
}
indexRandom(true, indexRequestBuilders);
CompletionSuggestionBuilder prefix = SuggestBuilders.completionSuggestion(FIELD).prefix("sugg");
SearchResponse searchResponse = client().prepareSearch(INDEX).setExplain(true)
.suggest(new SuggestBuilder().addSuggestion("foo", prefix)).get();
assertSuggestions(searchResponse, "foo", "suggestion10", "suggestion9", "suggestion8", "suggestion7", "suggestion6");
}

public static boolean isReservedChar(char c) {
switch (c) {
case '\u001F':
Expand Down

0 comments on commit 39d4c7c

Please sign in to comment.