From 1a739c2ed28ddc274f1943158187126f72d92a19 Mon Sep 17 00:00:00 2001 From: Britta Weber Date: Tue, 12 Aug 2014 20:33:13 +0200 Subject: [PATCH 1/2] function_score: remove explanation of query score fromfunctions --- .../function/FieldValueFactorFunction.java | 1 - .../search/function/RandomScoreFunction.java | 1 - .../search/function/ScriptScoreFunction.java | 7 +- .../function/RandomScoreFunctionTests.java | 1 - .../functionscore/FunctionScoreTests.java | 90 +++++++++++++++++++ 5 files changed, 95 insertions(+), 5 deletions(-) create mode 100644 src/test/java/org/elasticsearch/search/functionscore/FunctionScoreTests.java diff --git a/src/main/java/org/elasticsearch/common/lucene/search/function/FieldValueFactorFunction.java b/src/main/java/org/elasticsearch/common/lucene/search/function/FieldValueFactorFunction.java index a09e10f88854d..391046a772144 100644 --- a/src/main/java/org/elasticsearch/common/lucene/search/function/FieldValueFactorFunction.java +++ b/src/main/java/org/elasticsearch/common/lucene/search/function/FieldValueFactorFunction.java @@ -77,7 +77,6 @@ public Explanation explainScore(int docId, Explanation subQueryExpl) { exp.setValue(CombineFunction.toFloat(score)); exp.setDescription("field value function: " + modifierStr + "(" + "doc['" + field + "'].value * factor=" + boostFactor + ")"); - exp.addDetail(subQueryExpl); return exp; } diff --git a/src/main/java/org/elasticsearch/common/lucene/search/function/RandomScoreFunction.java b/src/main/java/org/elasticsearch/common/lucene/search/function/RandomScoreFunction.java index ff6f788764126..9879bdd81bc6c 100644 --- a/src/main/java/org/elasticsearch/common/lucene/search/function/RandomScoreFunction.java +++ b/src/main/java/org/elasticsearch/common/lucene/search/function/RandomScoreFunction.java @@ -47,7 +47,6 @@ public double score(int docId, float subQueryScore) { public Explanation explainScore(int docId, Explanation subQueryExpl) { Explanation exp = new Explanation(); exp.setDescription("random score function (seed: " + prng.originalSeed + ")"); - exp.addDetail(subQueryExpl); return exp; } diff --git a/src/main/java/org/elasticsearch/common/lucene/search/function/ScriptScoreFunction.java b/src/main/java/org/elasticsearch/common/lucene/search/function/ScriptScoreFunction.java index ea2e76c3bace1..a36781c09562e 100644 --- a/src/main/java/org/elasticsearch/common/lucene/search/function/ScriptScoreFunction.java +++ b/src/main/java/org/elasticsearch/common/lucene/search/function/ScriptScoreFunction.java @@ -110,8 +110,11 @@ public Explanation explainScore(int docId, Explanation subQueryExpl) { exp = ((ExplainableSearchScript) script).explain(subQueryExpl); } else { double score = score(docId, subQueryExpl.getValue()); - exp = new Explanation(CombineFunction.toFloat(score), "script score function: composed of:"); - exp.addDetail(subQueryExpl); + String explanation = "script score function, computed with script:\"" + sScript; + if (params != null) { + explanation += "\" and parameters: \n" + params.toString(); + } + exp = new Explanation(CombineFunction.toFloat(score), explanation); } return exp; } diff --git a/src/test/java/org/elasticsearch/common/lucene/search/function/RandomScoreFunctionTests.java b/src/test/java/org/elasticsearch/common/lucene/search/function/RandomScoreFunctionTests.java index d03d21260b866..ab55327dae0fa 100644 --- a/src/test/java/org/elasticsearch/common/lucene/search/function/RandomScoreFunctionTests.java +++ b/src/test/java/org/elasticsearch/common/lucene/search/function/RandomScoreFunctionTests.java @@ -187,7 +187,6 @@ public void testExplainScoreReportsOriginalSeed() { // Original seed should be there assertThat(randomExplanation.getDescription(), containsString("" + seed)); - assertThat(randomExplanation.getDetails(), arrayContaining(subExplanation)); } diff --git a/src/test/java/org/elasticsearch/search/functionscore/FunctionScoreTests.java b/src/test/java/org/elasticsearch/search/functionscore/FunctionScoreTests.java new file mode 100644 index 0000000000000..3c4eec9c7e610 --- /dev/null +++ b/src/test/java/org/elasticsearch/search/functionscore/FunctionScoreTests.java @@ -0,0 +1,90 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.search.functionscore; + +import org.elasticsearch.action.search.SearchResponse; +import org.elasticsearch.action.search.SearchType; +import org.elasticsearch.test.ElasticsearchIntegrationTest; +import org.junit.Test; + +import java.io.IOException; +import java.util.concurrent.ExecutionException; + +import static org.elasticsearch.client.Requests.searchRequest; +import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder; +import static org.elasticsearch.index.query.QueryBuilders.functionScoreQuery; +import static org.elasticsearch.index.query.QueryBuilders.termQuery; +import static org.elasticsearch.index.query.functionscore.ScoreFunctionBuilders.*; +import static org.elasticsearch.search.builder.SearchSourceBuilder.searchSource; +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.greaterThan; + +public class FunctionScoreTests extends ElasticsearchIntegrationTest { + + @Test + public void testExplainQueryOnlyOnce() throws IOException, ExecutionException, InterruptedException { + assertAcked(prepareCreate("test").addMapping( + "type1", + jsonBuilder().startObject().startObject("type1").startObject("properties").startObject("test").field("type", "string") + .endObject().startObject("num").field("type", "float").endObject().endObject().endObject().endObject())); + ensureYellow(); + + client().prepareIndex() + .setType("type1") + .setId("1") + .setIndex("test") + .setSource( + jsonBuilder().startObject().field("test", "value").field("num", 10).endObject()).get(); + refresh(); + + SearchResponse response = client().search( + searchRequest().searchType(SearchType.QUERY_THEN_FETCH).source( + searchSource().explain(true).query( + functionScoreQuery(termQuery("test", "value")).add(gaussDecayFunction("num", 5, 5)).add(exponentialDecayFunction("num", 5, 5)).add(linearDecayFunction("num", 5, 5))))).get(); + String explanation = response.getHits().getAt(0).explanation().toString(); + + checkQueryExplanationAppearsOnlyOnce(explanation); + response = client().search( + searchRequest().searchType(SearchType.QUERY_THEN_FETCH).source( + searchSource().explain(true).query( + functionScoreQuery(termQuery("test", "value")).add(fieldValueFactorFunction("num"))))).get(); + explanation = response.getHits().getAt(0).explanation().toString(); + checkQueryExplanationAppearsOnlyOnce(explanation); + + response = client().search( + searchRequest().searchType(SearchType.QUERY_THEN_FETCH).source( + searchSource().explain(true).query( + functionScoreQuery(termQuery("test", "value")).add(randomFunction(10))))).get(); + explanation = response.getHits().getAt(0).explanation().toString(); + + checkQueryExplanationAppearsOnlyOnce(explanation); + } + + private void checkQueryExplanationAppearsOnlyOnce(String explanation) { + // use some substring of the query explanation and see if it appears twice + String queryExplanation = "idf(docFreq=1, maxDocs=1)"; + int queryExplanationIndex = explanation.indexOf(queryExplanation, 0); + assertThat(queryExplanationIndex, greaterThan(-1)); + queryExplanationIndex = explanation.indexOf(queryExplanation, queryExplanationIndex + 1); + assertThat(queryExplanationIndex, equalTo(-1)); + } + +} From 74deb08c4999789d1ae2212c9e05557693e9b51d Mon Sep 17 00:00:00 2001 From: Britta Weber Date: Thu, 14 Aug 2014 15:49:54 +0200 Subject: [PATCH 2/2] remove explanation from parameter list of ScoreFunction#explainScore() and leave only the score This also removes ExplainableSearchScript which is not used anywhere and was the only reason to have the Explanation in the parameter anyway. --- .../search/function/BoostScoreFunction.java | 2 +- .../function/FieldValueFactorFunction.java | 4 +-- .../function/FiltersFunctionScoreQuery.java | 2 +- .../search/function/FunctionScoreQuery.java | 2 +- .../search/function/RandomScoreFunction.java | 2 +- .../lucene/search/function/ScoreFunction.java | 2 +- .../search/function/ScriptScoreFunction.java | 20 ++++------- .../functionscore/DecayFunctionParser.java | 4 +-- .../script/ExplainableSearchScript.java | 36 ------------------- .../elasticsearch/script/SearchScript.java | 2 -- .../function/RandomScoreFunctionTests.java | 2 +- 11 files changed, 16 insertions(+), 62 deletions(-) delete mode 100644 src/main/java/org/elasticsearch/script/ExplainableSearchScript.java diff --git a/src/main/java/org/elasticsearch/common/lucene/search/function/BoostScoreFunction.java b/src/main/java/org/elasticsearch/common/lucene/search/function/BoostScoreFunction.java index 43f7aadebcc96..1db8fc6b75730 100644 --- a/src/main/java/org/elasticsearch/common/lucene/search/function/BoostScoreFunction.java +++ b/src/main/java/org/elasticsearch/common/lucene/search/function/BoostScoreFunction.java @@ -49,7 +49,7 @@ public double score(int docId, float subQueryScore) { } @Override - public Explanation explainScore(int docId, Explanation subQueryExpl) { + public Explanation explainScore(int docId, float subQueryScore) { Explanation exp = new Explanation(boost, "static boost factor"); exp.addDetail(new Explanation(boost, "boostFactor")); return exp; diff --git a/src/main/java/org/elasticsearch/common/lucene/search/function/FieldValueFactorFunction.java b/src/main/java/org/elasticsearch/common/lucene/search/function/FieldValueFactorFunction.java index 391046a772144..1f89a456637ae 100644 --- a/src/main/java/org/elasticsearch/common/lucene/search/function/FieldValueFactorFunction.java +++ b/src/main/java/org/elasticsearch/common/lucene/search/function/FieldValueFactorFunction.java @@ -70,10 +70,10 @@ public double score(int docId, float subQueryScore) { } @Override - public Explanation explainScore(int docId, Explanation subQueryExpl) { + public Explanation explainScore(int docId, float subQueryScore) { Explanation exp = new Explanation(); String modifierStr = modifier != null ? modifier.toString() : ""; - double score = score(docId, subQueryExpl.getValue()); + double score = score(docId, subQueryScore); exp.setValue(CombineFunction.toFloat(score)); exp.setDescription("field value function: " + modifierStr + "(" + "doc['" + field + "'].value * factor=" + boostFactor + ")"); diff --git a/src/main/java/org/elasticsearch/common/lucene/search/function/FiltersFunctionScoreQuery.java b/src/main/java/org/elasticsearch/common/lucene/search/function/FiltersFunctionScoreQuery.java index d0c6157485f61..ef8489ef5429d 100644 --- a/src/main/java/org/elasticsearch/common/lucene/search/function/FiltersFunctionScoreQuery.java +++ b/src/main/java/org/elasticsearch/common/lucene/search/function/FiltersFunctionScoreQuery.java @@ -180,7 +180,7 @@ public Explanation explain(AtomicReaderContext context, int doc) throws IOExcept filterFunction.filter.getDocIdSet(context, context.reader().getLiveDocs())); if (docSet.get(doc)) { filterFunction.function.setNextReader(context); - Explanation functionExplanation = filterFunction.function.explainScore(doc, subQueryExpl); + Explanation functionExplanation = filterFunction.function.explainScore(doc, subQueryExpl.getValue()); double factor = functionExplanation.getValue(); float sc = CombineFunction.toFloat(factor); ComplexExplanation filterExplanation = new ComplexExplanation(true, sc, "function score, product of:"); 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 29f3408387e20..0e5dfb73474a9 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 @@ -131,7 +131,7 @@ public Explanation explain(AtomicReaderContext context, int doc) throws IOExcept return subQueryExpl; } function.setNextReader(context); - Explanation functionExplanation = function.explainScore(doc, subQueryExpl); + Explanation functionExplanation = function.explainScore(doc, subQueryExpl.getValue()); return combineFunction.explain(getBoost(), subQueryExpl, functionExplanation, maxBoost); } } diff --git a/src/main/java/org/elasticsearch/common/lucene/search/function/RandomScoreFunction.java b/src/main/java/org/elasticsearch/common/lucene/search/function/RandomScoreFunction.java index 9879bdd81bc6c..62660f084cf77 100644 --- a/src/main/java/org/elasticsearch/common/lucene/search/function/RandomScoreFunction.java +++ b/src/main/java/org/elasticsearch/common/lucene/search/function/RandomScoreFunction.java @@ -44,7 +44,7 @@ public double score(int docId, float subQueryScore) { } @Override - public Explanation explainScore(int docId, Explanation subQueryExpl) { + public Explanation explainScore(int docId, float subQueryScore) { Explanation exp = new Explanation(); exp.setDescription("random score function (seed: " + prng.originalSeed + ")"); return exp; diff --git a/src/main/java/org/elasticsearch/common/lucene/search/function/ScoreFunction.java b/src/main/java/org/elasticsearch/common/lucene/search/function/ScoreFunction.java index 671a14d10cd11..9c8a3aabc5e06 100644 --- a/src/main/java/org/elasticsearch/common/lucene/search/function/ScoreFunction.java +++ b/src/main/java/org/elasticsearch/common/lucene/search/function/ScoreFunction.java @@ -33,7 +33,7 @@ public abstract class ScoreFunction { public abstract double score(int docId, float subQueryScore); - public abstract Explanation explainScore(int docId, Explanation subQueryExpl); + public abstract Explanation explainScore(int docId, float subQueryScore); public CombineFunction getDefaultScoreCombiner() { return scoreCombiner; diff --git a/src/main/java/org/elasticsearch/common/lucene/search/function/ScriptScoreFunction.java b/src/main/java/org/elasticsearch/common/lucene/search/function/ScriptScoreFunction.java index a36781c09562e..2e6d8f1fe8c55 100644 --- a/src/main/java/org/elasticsearch/common/lucene/search/function/ScriptScoreFunction.java +++ b/src/main/java/org/elasticsearch/common/lucene/search/function/ScriptScoreFunction.java @@ -22,7 +22,6 @@ import org.apache.lucene.index.AtomicReaderContext; import org.apache.lucene.search.Explanation; import org.apache.lucene.search.Scorer; -import org.elasticsearch.script.ExplainableSearchScript; import org.elasticsearch.script.SearchScript; import java.io.IOException; @@ -101,21 +100,14 @@ public double score(int docId, float subQueryScore) { } @Override - public Explanation explainScore(int docId, Explanation subQueryExpl) { + public Explanation explainScore(int docId, float subQueryScore) { Explanation exp; - if (script instanceof ExplainableSearchScript) { - script.setNextDocId(docId); - scorer.docid = docId; - scorer.score = subQueryExpl.getValue(); - exp = ((ExplainableSearchScript) script).explain(subQueryExpl); - } else { - double score = score(docId, subQueryExpl.getValue()); - String explanation = "script score function, computed with script:\"" + sScript; - if (params != null) { - explanation += "\" and parameters: \n" + params.toString(); - } - exp = new Explanation(CombineFunction.toFloat(score), explanation); + double score = score(docId, subQueryScore); + String explanation = "script score function, computed with script:\"" + sScript; + if (params != null) { + explanation += "\" and parameters: \n" + params.toString(); } + exp = new Explanation(CombineFunction.toFloat(score), explanation); return exp; } diff --git a/src/main/java/org/elasticsearch/index/query/functionscore/DecayFunctionParser.java b/src/main/java/org/elasticsearch/index/query/functionscore/DecayFunctionParser.java index 67310f559319b..2151300e90ca8 100644 --- a/src/main/java/org/elasticsearch/index/query/functionscore/DecayFunctionParser.java +++ b/src/main/java/org/elasticsearch/index/query/functionscore/DecayFunctionParser.java @@ -454,9 +454,9 @@ public double score(int docId, float subQueryScore) { protected abstract String getFieldName(); @Override - public Explanation explainScore(int docId, Explanation subQueryExpl) { + public Explanation explainScore(int docId, float subQueryScore) { ComplexExplanation ce = new ComplexExplanation(); - ce.setValue(CombineFunction.toFloat(score(docId, subQueryExpl.getValue()))); + ce.setValue(CombineFunction.toFloat(score(docId, subQueryScore))); ce.setMatch(true); ce.setDescription("Function for field " + getFieldName() + ":"); ce.addDetail(func.explainFunction(getDistanceString(docId), distance(docId), scale)); diff --git a/src/main/java/org/elasticsearch/script/ExplainableSearchScript.java b/src/main/java/org/elasticsearch/script/ExplainableSearchScript.java deleted file mode 100644 index c4a5a65813dba..0000000000000 --- a/src/main/java/org/elasticsearch/script/ExplainableSearchScript.java +++ /dev/null @@ -1,36 +0,0 @@ -/* - * Licensed to Elasticsearch under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -package org.elasticsearch.script; - -import org.apache.lucene.search.Explanation; - -/** - * To be implemented by {@link SearchScript} which can provided an {@link Explanation} of the score - */ -public interface ExplainableSearchScript extends SearchScript { - - /** - * Build the explanation of the current document being scored - * - * @param subQueryExpl the explanation of the subQuery - */ - Explanation explain(Explanation subQueryExpl); - -} diff --git a/src/main/java/org/elasticsearch/script/SearchScript.java b/src/main/java/org/elasticsearch/script/SearchScript.java index 52210590c71af..15955d98d9983 100644 --- a/src/main/java/org/elasticsearch/script/SearchScript.java +++ b/src/main/java/org/elasticsearch/script/SearchScript.java @@ -27,8 +27,6 @@ /** * A search script. - * - * @see {@link ExplainableSearchScript} for script which can explain a score */ public interface SearchScript extends ExecutableScript, ReaderContextAware, ScorerAware { diff --git a/src/test/java/org/elasticsearch/common/lucene/search/function/RandomScoreFunctionTests.java b/src/test/java/org/elasticsearch/common/lucene/search/function/RandomScoreFunctionTests.java index ab55327dae0fa..c63dda614ce82 100644 --- a/src/test/java/org/elasticsearch/common/lucene/search/function/RandomScoreFunctionTests.java +++ b/src/test/java/org/elasticsearch/common/lucene/search/function/RandomScoreFunctionTests.java @@ -183,7 +183,7 @@ public void testExplainScoreReportsOriginalSeed() { function.score(0, 1.0f); // Generate the randomScore explanation - Explanation randomExplanation = function.explainScore(0, subExplanation); + Explanation randomExplanation = function.explainScore(0, subExplanation.getValue()); // Original seed should be there assertThat(randomExplanation.getDescription(), containsString("" + seed));