Skip to content

Commit ae07dfd

Browse files
committed
fix explain in function_score if no function filter matches
When each function in function_score has a filter but none of them matches we always assume 1 for the combined functions and then combine that with the sub query score. But the explanation did not reflect that because in case no function matched we did not even use the actual score that was computed in the explanation.
1 parent 299c6fc commit ae07dfd

File tree

2 files changed

+37
-7
lines changed

2 files changed

+37
-7
lines changed

core/src/main/java/org/elasticsearch/common/lucene/search/function/FiltersFunctionScoreQuery.java

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -224,17 +224,23 @@ public Explanation explain(LeafReaderContext context, int doc) throws IOExceptio
224224
filterExplanations.add(filterExplanation);
225225
}
226226
}
227+
FiltersFunctionFactorScorer scorer = functionScorer(context);
228+
int actualDoc = scorer.iterator().advance(doc);
229+
assert (actualDoc == doc);
230+
double score = scorer.computeScore(doc, expl.getValue());
231+
Explanation factorExplanation;
227232
if (filterExplanations.size() > 0) {
228-
FiltersFunctionFactorScorer scorer = functionScorer(context);
229-
int actualDoc = scorer.iterator().advance(doc);
230-
assert (actualDoc == doc);
231-
double score = scorer.computeScore(doc, expl.getValue());
232-
Explanation factorExplanation = Explanation.match(
233+
factorExplanation = Explanation.match(
233234
CombineFunction.toFloat(score),
234235
"function score, score mode [" + scoreMode.toString().toLowerCase(Locale.ROOT) + "]",
235236
filterExplanations);
236-
expl = combineFunction.explain(expl, factorExplanation, maxBoost);
237+
238+
} else {
239+
// it is a little weird to add a match although no function matches but that is the way function_score behaves right now
240+
factorExplanation = Explanation.match(1.0f,
241+
"No function matched", new ArrayList<>());
237242
}
243+
expl = combineFunction.explain(expl, factorExplanation, maxBoost);
238244
if (minScore != null && minScore > expl.getValue()) {
239245
expl = Explanation.noMatch("Score value is too low, expected at least " + minScore + " but got " + expl.getValue(), expl);
240246
}

core/src/test/java/org/elasticsearch/index/query/functionscore/FunctionScoreTests.java

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -599,7 +599,7 @@ public void testMinScoreExplain() throws IOException {
599599
Explanation ffsqExpl = searcher.explain(ffsq, 0);
600600
assertTrue(ffsqExpl.isMatch());
601601
assertEquals(queryExpl.getValue(), ffsqExpl.getValue(), 0f);
602-
assertEquals(queryExpl.getDescription(), ffsqExpl.getDescription());
602+
assertEquals(queryExpl.getDescription(), ffsqExpl.getDetails()[0].getDescription());
603603

604604
ffsq = new FiltersFunctionScoreQuery(query, ScoreMode.SUM, new FilterFunction[0], Float.POSITIVE_INFINITY, 10f,
605605
CombineFunction.MULTIPLY);
@@ -726,6 +726,30 @@ public void testFilterFunctionScoreHashCodeAndEquals() {
726726
}
727727
}
728728

729+
public void testExplanationAndScoreEqualsEvenIfNoFunctionMatches() throws IOException {
730+
IndexSearcher localSearcher = newSearcher(reader);
731+
ScoreMode scoreMode = randomFrom(new
732+
ScoreMode[]{ScoreMode.SUM, ScoreMode.AVG, ScoreMode.FIRST, ScoreMode.MIN, ScoreMode.MAX, ScoreMode.MULTIPLY});
733+
CombineFunction combineFunction = randomFrom(new
734+
CombineFunction[]{CombineFunction.SUM, CombineFunction.AVG, CombineFunction.MIN, CombineFunction.MAX,
735+
CombineFunction.MULTIPLY});
736+
// check for document that has no macthing function
737+
FiltersFunctionScoreQuery query = new FiltersFunctionScoreQuery(new TermQuery(new Term(FIELD, "out")), scoreMode,
738+
new FilterFunction[]{new FilterFunction(new TermQuery(new Term("_uid", "2")), new WeightFactorFunction(10))},
739+
Float.MAX_VALUE, Float.NEGATIVE_INFINITY, combineFunction);
740+
TopDocs searchResult = localSearcher.search(query, 1);
741+
Explanation exp1 = localSearcher.explain(query, searchResult.scoreDocs[0].doc);
742+
assertThat(searchResult.scoreDocs[0].score, equalTo(exp1.getValue()));
743+
744+
// check for document that has a matching function
745+
query = new FiltersFunctionScoreQuery(new TermQuery(new Term(FIELD, "out")), scoreMode,
746+
new FilterFunction[]{new FilterFunction(new TermQuery(new Term("_uid", "1")), new WeightFactorFunction(10))},
747+
Float.MAX_VALUE, Float.NEGATIVE_INFINITY, combineFunction);
748+
searchResult = localSearcher.search(query, 1);
749+
exp1 = localSearcher.explain(query, searchResult.scoreDocs[0].doc);
750+
assertThat(searchResult.scoreDocs[0].score, equalTo(exp1.getValue()));
751+
}
752+
729753
private static class DummyScoreFunction extends ScoreFunction {
730754
protected DummyScoreFunction(CombineFunction scoreCombiner) {
731755
super(scoreCombiner);

0 commit comments

Comments
 (0)