function_score parser throws exception if both functions:[] and single function given
In addition, add a special warning if the misplaced function is a "boost_factor" function to avoid confusion of "boost" and "boost_function". closes #5995
This commit is contained in:
parent
a557ee8daf
commit
2e44040388
|
@ -23,6 +23,7 @@ import com.google.common.collect.ImmutableMap;
|
|||
import com.google.common.collect.ImmutableMap.Builder;
|
||||
import org.apache.lucene.search.Filter;
|
||||
import org.apache.lucene.search.Query;
|
||||
import org.elasticsearch.ElasticsearchParseException;
|
||||
import org.elasticsearch.common.Strings;
|
||||
import org.elasticsearch.common.inject.Inject;
|
||||
import org.elasticsearch.common.lucene.search.Queries;
|
||||
|
@ -35,9 +36,11 @@ import org.elasticsearch.common.xcontent.XContentParser;
|
|||
import org.elasticsearch.index.query.QueryParseContext;
|
||||
import org.elasticsearch.index.query.QueryParser;
|
||||
import org.elasticsearch.index.query.QueryParsingException;
|
||||
import org.elasticsearch.index.query.functionscore.factor.FactorParser;
|
||||
|
||||
import java.io.IOException;
|
||||
import java.util.ArrayList;
|
||||
import java.util.Arrays;
|
||||
|
||||
/**
|
||||
*
|
||||
|
@ -46,6 +49,9 @@ public class FunctionScoreQueryParser implements QueryParser {
|
|||
|
||||
public static final String NAME = "function_score";
|
||||
ScoreFunctionParserMapper funtionParserMapper;
|
||||
// For better readability of error message
|
||||
static final String MISPLACED_FUNCTION_MESSAGE_PREFIX = "You can either define \"functions\":[...] or a single function, not both. ";
|
||||
static final String MISPLACED_BOOST_FUNCTION_MESSAGE_SUFFIX = " Did you mean \"boost\" instead?";
|
||||
|
||||
@Inject
|
||||
public FunctionScoreQueryParser(ScoreFunctionParserMapper funtionParserMapper) {
|
||||
|
@ -82,6 +88,11 @@ public class FunctionScoreQueryParser implements QueryParser {
|
|||
String currentFieldName = null;
|
||||
XContentParser.Token token;
|
||||
CombineFunction combineFunction = CombineFunction.MULT;
|
||||
// Either define array of functions and filters or only one function
|
||||
boolean functionArrayFound = false;
|
||||
boolean singleFunctionFound = false;
|
||||
String singleFunctionName = null;
|
||||
|
||||
while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
|
||||
if (token == XContentParser.Token.FIELD_NAME) {
|
||||
currentFieldName = parser.currentName();
|
||||
|
@ -98,13 +109,24 @@ public class FunctionScoreQueryParser implements QueryParser {
|
|||
} else if ("boost".equals(currentFieldName)) {
|
||||
boost = parser.floatValue();
|
||||
} else if ("functions".equals(currentFieldName)) {
|
||||
if (singleFunctionFound) {
|
||||
String errorString = "Found \"" + singleFunctionName + "\" already, now encountering \"functions\": [...].";
|
||||
handleMisplacedFunctionsDeclaration(errorString, singleFunctionName);
|
||||
}
|
||||
currentFieldName = parseFiltersAndFunctions(parseContext, parser, filterFunctions, currentFieldName);
|
||||
functionArrayFound = true;
|
||||
} else {
|
||||
// we tru to parse a score function. If there is no score
|
||||
// we try to parse a score function. If there is no score
|
||||
// function for the current field name,
|
||||
// funtionParserMapper.get() will throw an Exception.
|
||||
filterFunctions.add(new FiltersFunctionScoreQuery.FilterFunction(null, funtionParserMapper.get(parseContext.index(),
|
||||
currentFieldName).parse(parseContext, parser)));
|
||||
// functionParserMapper.get() will throw an Exception.
|
||||
ScoreFunctionParser currentFunctionParser = funtionParserMapper.get(parseContext.index(), currentFieldName);
|
||||
singleFunctionName = currentFieldName;
|
||||
if (functionArrayFound) {
|
||||
String errorString = "Found \"functions\": [...] already, now encountering \"" + currentFieldName + "\".";
|
||||
handleMisplacedFunctionsDeclaration(errorString, currentFieldName);
|
||||
}
|
||||
filterFunctions.add(new FiltersFunctionScoreQuery.FilterFunction(null, currentFunctionParser.parse(parseContext, parser)));
|
||||
singleFunctionFound = true;
|
||||
}
|
||||
}
|
||||
if (query == null) {
|
||||
|
@ -136,6 +158,14 @@ public class FunctionScoreQueryParser implements QueryParser {
|
|||
}
|
||||
}
|
||||
|
||||
private void handleMisplacedFunctionsDeclaration(String errorString, String functionName) {
|
||||
errorString = MISPLACED_FUNCTION_MESSAGE_PREFIX + errorString;
|
||||
if (Arrays.asList(FactorParser.NAMES).contains(functionName)) {
|
||||
errorString = errorString + MISPLACED_BOOST_FUNCTION_MESSAGE_SUFFIX;
|
||||
}
|
||||
throw new ElasticsearchParseException(errorString);
|
||||
}
|
||||
|
||||
private String parseFiltersAndFunctions(QueryParseContext parseContext, XContentParser parser,
|
||||
ArrayList<FiltersFunctionScoreQuery.FilterFunction> filterFunctions, String currentFieldName) throws IOException {
|
||||
XContentParser.Token token;
|
||||
|
|
|
@ -27,6 +27,8 @@ import org.elasticsearch.action.search.SearchResponse;
|
|||
import org.elasticsearch.action.search.SearchType;
|
||||
import org.elasticsearch.common.geo.GeoPoint;
|
||||
import org.elasticsearch.common.lucene.search.function.CombineFunction;
|
||||
import org.elasticsearch.common.xcontent.XContentBuilder;
|
||||
import org.elasticsearch.common.xcontent.XContentFactory;
|
||||
import org.elasticsearch.index.query.MatchAllFilterBuilder;
|
||||
import org.elasticsearch.index.query.QueryBuilders;
|
||||
import org.elasticsearch.index.query.functionscore.DecayFunctionBuilder;
|
||||
|
@ -799,4 +801,59 @@ public class DecayFunctionScoreTests extends ElasticsearchIntegrationTest {
|
|||
sh = sr.getHits();
|
||||
assertThat((double) (sh.getAt(0).getScore()), closeTo((double) (sh.getAt(1).getScore()), 1.e-6d));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void errorMessageForFaultyFunctionScoreBody() throws Exception {
|
||||
assertAcked(prepareCreate("test").addMapping(
|
||||
"type",
|
||||
jsonBuilder().startObject().startObject("type").startObject("properties").startObject("test").field("type", "string")
|
||||
.endObject().startObject("num").field("type", "double").endObject().endObject().endObject().endObject()));
|
||||
ensureYellow();
|
||||
client().index(
|
||||
indexRequest("test").type("type").source(jsonBuilder().startObject().field("test", "value").field("num", 1.0).endObject()))
|
||||
.actionGet();
|
||||
refresh();
|
||||
|
||||
XContentBuilder query = XContentFactory.jsonBuilder();
|
||||
// query that contains a functions[] array but also a single function
|
||||
query.startObject().startObject("function_score").startArray("functions").startObject().field("boost_factor", "1.3").endObject().endArray().field("boost_factor", "1").endObject().endObject();
|
||||
try {
|
||||
client().search(
|
||||
searchRequest().source(
|
||||
searchSource().query(query))).actionGet();
|
||||
fail("Search should result in SearchPhaseExecutionException");
|
||||
} catch (SearchPhaseExecutionException e) {
|
||||
logger.info(e.shardFailures()[0].reason());
|
||||
assertTrue(e.shardFailures()[0].reason().contains("Found \"functions\": [...] already, now encountering \"boost_factor\". Did you mean \"boost\" instead?"));
|
||||
}
|
||||
|
||||
query = XContentFactory.jsonBuilder();
|
||||
// query that contains a single function and a functions[] array
|
||||
query.startObject().startObject("function_score").field("boost_factor", "1").startArray("functions").startObject().field("boost_factor", "1.3").endObject().endArray().endObject().endObject();
|
||||
try {
|
||||
client().search(
|
||||
searchRequest().source(
|
||||
searchSource().query(query))).actionGet();
|
||||
fail("Search should result in SearchPhaseExecutionException");
|
||||
} catch (SearchPhaseExecutionException e) {
|
||||
logger.info(e.shardFailures()[0].reason());
|
||||
assertTrue(e.shardFailures()[0].reason().contains("Found \"boost_factor\" already, now encountering \"functions\": [...]. Did you mean \"boost\" instead?"));
|
||||
}
|
||||
|
||||
query = XContentFactory.jsonBuilder();
|
||||
// query that contains a single function (but not boost factor) and a functions[] array
|
||||
query.startObject().startObject("function_score").startObject("random_score").field("seed", 3).endObject().startArray("functions").startObject().startObject("random_score").field("seed", 3).endObject().endObject().endArray().endObject().endObject();
|
||||
try {
|
||||
client().search(
|
||||
searchRequest().source(
|
||||
searchSource().query(query))).actionGet();
|
||||
fail("Search should result in SearchPhaseExecutionException");
|
||||
} catch (SearchPhaseExecutionException e) {
|
||||
logger.info(e.shardFailures()[0].reason());
|
||||
assertTrue(e.shardFailures()[0].reason().contains("Found \"random_score\" already, now encountering \"functions\": [...]."));
|
||||
assertFalse(e.shardFailures()[0].reason().contains("Did you mean \"boost\" instead?"));
|
||||
|
||||
}
|
||||
}
|
||||
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue