From 02f8d04049ed8a2a8db2c24cb901e6f09bcb2761 Mon Sep 17 00:00:00 2001 From: Britta Weber Date: Mon, 30 Mar 2015 18:08:44 +0200 Subject: [PATCH] [function_score] apply min_score to sub query score if no function provided For optimization pruposes a function score query with an empty function will just result in the original sub query. However, sometimes one might want to use function_score query to actually filter out docs within for example bool clauses by using the min_score functionallity. Therefore the sub query should only be used without wrapping inside a function_score query if min_score was also not set. closes #10253 closes #10326 --- .../search/function/FunctionScoreQuery.java | 29 ++++++++++----- .../FunctionScoreQueryParser.java | 12 ++++--- .../functionscore/FunctionScoreTests.java | 35 +++++++++++++++++++ 3 files changed, 63 insertions(+), 13 deletions(-) diff --git a/src/main/java/org/elasticsearch/common/lucene/search/function/FunctionScoreQuery.java b/src/main/java/org/elasticsearch/common/lucene/search/function/FunctionScoreQuery.java index 5b03ce72ade8a..d4aec566d3d62 100644 --- a/src/main/java/org/elasticsearch/common/lucene/search/function/FunctionScoreQuery.java +++ b/src/main/java/org/elasticsearch/common/lucene/search/function/FunctionScoreQuery.java @@ -27,6 +27,7 @@ import org.apache.lucene.util.ToStringUtils; import java.io.IOException; +import java.util.Objects; import java.util.Set; /** @@ -43,7 +44,7 @@ public class FunctionScoreQuery extends Query { public FunctionScoreQuery(Query subQuery, ScoreFunction function, Float minScore) { this.subQuery = subQuery; this.function = function; - this.combineFunction = function.getDefaultScoreCombiner(); + this.combineFunction = function == null? combineFunction.MULT : function.getDefaultScoreCombiner(); this.minScore = minScore; } @@ -128,7 +129,9 @@ public Scorer scorer(AtomicReaderContext context, Bits acceptDocs) throws IOExce if (subQueryScorer == null) { return null; } - function.setNextReader(context); + if (function != null) { + function.setNextReader(context); + } return new FunctionFactorScorer(this, subQueryScorer, function, maxBoost, combineFunction, minScore); } @@ -138,9 +141,13 @@ public Explanation explain(AtomicReaderContext context, int doc) throws IOExcept if (!subQueryExpl.isMatch()) { return subQueryExpl; } - function.setNextReader(context); - Explanation functionExplanation = function.explainScore(doc, subQueryExpl); - return combineFunction.explain(getBoost(), subQueryExpl, functionExplanation, maxBoost); + if (function != null) { + function.setNextReader(context); + Explanation functionExplanation = function.explainScore(doc, subQueryExpl); + return combineFunction.explain(getBoost(), subQueryExpl, functionExplanation, maxBoost); + } else { + return subQueryExpl; + } } } @@ -157,8 +164,12 @@ private FunctionFactorScorer(CustomBoostFactorWeight w, Scorer scorer, ScoreFunc @Override public float innerScore() throws IOException { float score = scorer.score(); - return scoreCombiner.combine(subQueryBoost, score, - function.score(scorer.docID(), score), maxBoost); + if (function == null) { + return subQueryBoost * score; + } else { + return scoreCombiner.combine(subQueryBoost, score, + function.score(scorer.docID(), score), maxBoost); + } } } @@ -173,11 +184,11 @@ public boolean equals(Object o) { if (o == null || getClass() != o.getClass()) return false; FunctionScoreQuery other = (FunctionScoreQuery) o; - return this.getBoost() == other.getBoost() && this.subQuery.equals(other.subQuery) && this.function.equals(other.function) + return this.getBoost() == other.getBoost() && this.subQuery.equals(other.subQuery) && (this.function != null ? this.function.equals(other.function) : other.function == null) && this.maxBoost == other.maxBoost; } public int hashCode() { - return subQuery.hashCode() + 31 * function.hashCode() ^ Float.floatToIntBits(getBoost()); + return subQuery.hashCode() + 31 * Objects.hashCode(function) ^ Float.floatToIntBits(getBoost()); } } diff --git a/src/main/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryParser.java b/src/main/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryParser.java index b00b406bb5422..28b22a2c77122 100644 --- a/src/main/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryParser.java +++ b/src/main/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryParser.java @@ -86,7 +86,7 @@ public Query parse(QueryParseContext parseContext) throws IOException, QueryPars FiltersFunctionScoreQuery.ScoreMode scoreMode = FiltersFunctionScoreQuery.ScoreMode.Multiply; ArrayList filterFunctions = new ArrayList<>(); - float maxBoost = Float.MAX_VALUE; + Float maxBoost = null; Float minScore = null; String currentFieldName = null; @@ -153,13 +153,17 @@ public Query parse(QueryParseContext parseContext) throws IOException, QueryPars query = new XFilteredQuery(query, filter); } // if all filter elements returned null, just use the query - if (filterFunctions.isEmpty()) { + if (filterFunctions.isEmpty() && combineFunction == null) { return query; } + if (maxBoost == null) { + maxBoost = Float.MAX_VALUE; + } // handle cases where only one score function and no filter was // provided. In this case we create a FunctionScoreQuery. - if (filterFunctions.size() == 1 && (filterFunctions.get(0).filter == null || filterFunctions.get(0).filter instanceof MatchAllDocsFilter)) { - FunctionScoreQuery theQuery = new FunctionScoreQuery(query, filterFunctions.get(0).function, minScore); + if (filterFunctions.size() == 0 || filterFunctions.size() == 1 && (filterFunctions.get(0).filter == null || filterFunctions.get(0).filter instanceof MatchAllDocsFilter)) { + ScoreFunction function = filterFunctions.size() == 0 ? null : filterFunctions.get(0).function; + FunctionScoreQuery theQuery = new FunctionScoreQuery(query, function, minScore); if (combineFunction != null) { theQuery.setCombineFunction(combineFunction); } diff --git a/src/test/java/org/elasticsearch/search/functionscore/FunctionScoreTests.java b/src/test/java/org/elasticsearch/search/functionscore/FunctionScoreTests.java index 5c7b90ccac110..455a3ea45f15c 100644 --- a/src/test/java/org/elasticsearch/search/functionscore/FunctionScoreTests.java +++ b/src/test/java/org/elasticsearch/search/functionscore/FunctionScoreTests.java @@ -549,5 +549,40 @@ public void testFilterAndQueryGiven() throws IOException, ExecutionException, In assertThat(Float.parseFloat(hit.getId()), equalTo(hit.getScore())); } } + + @Test + public void testWithEmptyFunctions() throws IOException, ExecutionException, InterruptedException { + assertAcked(prepareCreate("test")); + ensureYellow(); + index("test", "testtype", "1", jsonBuilder().startObject().field("text", "test text").endObject()); + refresh(); + + // make sure that min_score works if functions is empty, see https://github.com/elastic/elasticsearch/issues/10253 + float termQueryScore = 0.19178301f; + testMinScoreApplied("sum", termQueryScore); + testMinScoreApplied("avg", termQueryScore); + testMinScoreApplied("max", termQueryScore); + testMinScoreApplied("min", termQueryScore); + testMinScoreApplied("multiply", termQueryScore); + testMinScoreApplied("replace", termQueryScore); + } + + protected void testMinScoreApplied(String boostMode, float expectedScore) throws InterruptedException, ExecutionException { + SearchResponse response = client().search( + searchRequest().source( + searchSource().explain(true).query( + functionScoreQuery(termQuery("text", "text")).boostMode(boostMode).setMinScore(0.1f)))).get(); + assertSearchResponse(response); + assertThat(response.getHits().totalHits(), equalTo(1l)); + assertThat(response.getHits().getAt(0).getScore(), equalTo(expectedScore)); + + response = client().search( + searchRequest().source( + searchSource().explain(true).query( + functionScoreQuery(termQuery("text", "text")).boostMode(boostMode).setMinScore(2f)))).get(); + + assertSearchResponse(response); + assertThat(response.getHits().totalHits(), equalTo(0l)); + } }