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 bc9fff9483b35..ea2e76c3bace1 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 @@ -21,17 +21,60 @@ 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; import java.util.Map; public class ScriptScoreFunction extends ScoreFunction { + static final class CannedScorer extends Scorer { + protected int docid; + protected float score; + + public CannedScorer() { + super(null); + } + + @Override + public int docID() { + return docid; + } + + @Override + public float score() throws IOException { + return score; + } + + @Override + public int freq() throws IOException { + throw new UnsupportedOperationException(); + } + + @Override + public int nextDoc() throws IOException { + throw new UnsupportedOperationException(); + } + + @Override + public int advance(int target) throws IOException { + throw new UnsupportedOperationException(); + } + + @Override + public long cost() { + return 1; + } + } + private final String sScript; private final Map params; + private final CannedScorer scorer; + private final SearchScript script; @@ -40,6 +83,8 @@ public ScriptScoreFunction(String sScript, Map params, SearchScr this.sScript = sScript; this.params = params; this.script = script; + this.scorer = new CannedScorer(); + script.setScorer(scorer); } @Override @@ -50,7 +95,8 @@ public void setNextReader(AtomicReaderContext ctx) { @Override public double score(int docId, float subQueryScore) { script.setNextDocId(docId); - script.setNextScore(subQueryScore); + scorer.docid = docId; + scorer.score = subQueryScore; return script.runAsDouble(); } @@ -59,7 +105,8 @@ public Explanation explainScore(int docId, Explanation subQueryExpl) { Explanation exp; if (script instanceof ExplainableSearchScript) { script.setNextDocId(docId); - script.setNextScore(subQueryExpl.getValue()); + scorer.docid = docId; + scorer.score = subQueryExpl.getValue(); exp = ((ExplainableSearchScript) script).explain(subQueryExpl); } else { double score = score(docId, subQueryExpl.getValue()); diff --git a/src/main/java/org/elasticsearch/script/AbstractSearchScript.java b/src/main/java/org/elasticsearch/script/AbstractSearchScript.java index 769aac02158e8..13ac54bd5772c 100644 --- a/src/main/java/org/elasticsearch/script/AbstractSearchScript.java +++ b/src/main/java/org/elasticsearch/script/AbstractSearchScript.java @@ -29,6 +29,7 @@ import org.elasticsearch.search.lookup.SearchLookup; import org.elasticsearch.search.lookup.SourceLookup; +import java.io.IOException; import java.util.Map; /** @@ -44,16 +45,6 @@ public abstract class AbstractSearchScript extends AbstractExecutableScript impl private SearchLookup lookup; - private float score = Float.NaN; - - /** - * Returns the current score and only applicable when used as a scoring script in a custom score query!. - * For other cases, use {@link #doc()} and get the score from it. - */ - protected final float score() { - return score; - } - /** * Returns the doc lookup allowing to access field data (cached) values as well as the current document score * (where applicable). @@ -128,11 +119,6 @@ public void setNextSource(Map source) { lookup.source().setNextSource(source); } - @Override - public void setNextScore(float score) { - this.score = score; - } - @Override public float runAsFloat() { return ((Number) run()).floatValue(); diff --git a/src/main/java/org/elasticsearch/script/SearchScript.java b/src/main/java/org/elasticsearch/script/SearchScript.java index 33ed6e67fbe81..52210590c71af 100644 --- a/src/main/java/org/elasticsearch/script/SearchScript.java +++ b/src/main/java/org/elasticsearch/script/SearchScript.java @@ -36,8 +36,6 @@ public interface SearchScript extends ExecutableScript, ReaderContextAware, Scor void setNextSource(Map source); - void setNextScore(float score); - float runAsFloat(); long runAsLong(); diff --git a/src/main/java/org/elasticsearch/script/expression/ExpressionScript.java b/src/main/java/org/elasticsearch/script/expression/ExpressionScript.java index a1e2913ccc249..10b4cc99c4b0b 100644 --- a/src/main/java/org/elasticsearch/script/expression/ExpressionScript.java +++ b/src/main/java/org/elasticsearch/script/expression/ExpressionScript.java @@ -38,65 +38,25 @@ */ class ExpressionScript implements SearchScript { - /** Fake scorer for a single document */ - static class CannedScorer extends Scorer { - protected int docid; - protected float score; - - public CannedScorer() { - super(null); - } - - @Override - public int docID() { - return docid; - } - - @Override - public float score() throws IOException { - return score; - } - - @Override - public int freq() throws IOException { - throw new UnsupportedOperationException(); - } - - @Override - public int nextDoc() throws IOException { - throw new UnsupportedOperationException(); - } - - @Override - public int advance(int target) throws IOException { - throw new UnsupportedOperationException(); - } - - @Override - public long cost() { - return 1; - } - } - final Expression expression; final XSimpleBindings bindings; - final CannedScorer scorer; final ValueSource source; - final Map context; final ReplaceableConstValueSource specialValue; // _value + Map context; + Scorer scorer; FunctionValues values; + int docid; ExpressionScript(Expression e, XSimpleBindings b, ReplaceableConstValueSource v) { expression = e; bindings = b; - scorer = new CannedScorer(); - context = Collections.singletonMap("scorer", scorer); + context = Collections.EMPTY_MAP; source = expression.getValueSource(bindings); specialValue = v; } double evaluate() { - return values.doubleVal(scorer.docid); + return values.doubleVal(docid); } @Override @@ -116,17 +76,7 @@ public long cost() { @Override public void setNextDocId(int d) { - scorer.docid = d; - } - - @Override - public void setNextScore(float score) { - // TODO: fix this API to remove setNextScore and just use a Scorer - // Expressions know if they use the score or not, and should be able to pull from the scorer only - // if they need it. Right now, score can only be used within a ScriptScoreFunction. But there shouldn't - // be any reason a script values or aggregation can't use the score. It is also possible - // these layers are preventing inlining of scoring into expressions. - scorer.score = score; + docid = d; } @Override @@ -140,8 +90,8 @@ public void setNextReader(AtomicReaderContext leaf) { @Override public void setScorer(Scorer s) { - // noop: The scorer isn't actually ever set. Instead setNextScore is called. - // NOTE: This seems broken. Why can't we just use the scorer and get rid of setNextScore? + scorer = s; + context = Collections.singletonMap("scorer", scorer); } @Override diff --git a/src/main/java/org/elasticsearch/script/groovy/GroovyScriptEngineService.java b/src/main/java/org/elasticsearch/script/groovy/GroovyScriptEngineService.java index 5f12729062fa9..76bb1f8043647 100644 --- a/src/main/java/org/elasticsearch/script/groovy/GroovyScriptEngineService.java +++ b/src/main/java/org/elasticsearch/script/groovy/GroovyScriptEngineService.java @@ -45,6 +45,7 @@ import org.elasticsearch.script.ScriptEngineService; import org.elasticsearch.script.ScriptException; import org.elasticsearch.script.SearchScript; +import org.elasticsearch.search.lookup.DocLookup; import org.elasticsearch.search.lookup.SearchLookup; import java.io.IOException; @@ -188,7 +189,6 @@ public static final class GroovyScript implements ExecutableScript, SearchScript private final Script script; private final SearchLookup lookup; private final Map variables; - private final UpdateableFloat score; private final ESLogger logger; public GroovyScript(Script script, ESLogger logger) { @@ -200,10 +200,8 @@ public GroovyScript(Script script, SearchLookup lookup, ESLogger logger) { this.lookup = lookup; this.logger = logger; this.variables = script.getBinding().getVariables(); - this.score = new UpdateableFloat(0); - // Add the _score variable, which will be updated per-document by - // setting .value on the UpdateableFloat instance - this.variables.put("_score", this.score); + // Add the _score variable, which will access score from lookup.doc() + this.variables.put("_score", new ScoreAccessor()); } @Override @@ -227,12 +225,6 @@ public void setNextDocId(int doc) { } } - @SuppressWarnings({"unchecked"}) - @Override - public void setNextScore(float score) { - this.score.value = score; - } - @SuppressWarnings({"unchecked"}) @Override public void setNextVar(String name, Object value) { @@ -284,32 +276,34 @@ public Object unwrap(Object value) { * so that updating the _score for the next document incurs only the * overhead of setting a member variable */ - private final class UpdateableFloat extends Number { - - public float value; + private final class ScoreAccessor extends Number { - public UpdateableFloat(float value) { - this.value = value; + float score() { + try { + return lookup.doc().score(); + } catch (IOException e) { + throw new GroovyScriptExecutionException("Could not get score", e); + } } @Override public int intValue() { - return (int)value; + return (int)score(); } @Override public long longValue() { - return (long)value; + return (long)score(); } @Override public float floatValue() { - return value; + return score(); } @Override public double doubleValue() { - return value; + return score(); } } } diff --git a/src/main/java/org/elasticsearch/script/groovy/GroovyScriptExecutionException.java b/src/main/java/org/elasticsearch/script/groovy/GroovyScriptExecutionException.java index 5c0a531f0af7e..cb19b488deb36 100644 --- a/src/main/java/org/elasticsearch/script/groovy/GroovyScriptExecutionException.java +++ b/src/main/java/org/elasticsearch/script/groovy/GroovyScriptExecutionException.java @@ -30,6 +30,9 @@ public class GroovyScriptExecutionException extends ElasticsearchException { public GroovyScriptExecutionException(String message) { super(message); } + public GroovyScriptExecutionException(String message, Throwable t) { + super(message, t); + } @Override public RestStatus status() { diff --git a/src/main/java/org/elasticsearch/script/mvel/MvelScriptEngineService.java b/src/main/java/org/elasticsearch/script/mvel/MvelScriptEngineService.java index 0a2383282772c..5ce15b60be92c 100644 --- a/src/main/java/org/elasticsearch/script/mvel/MvelScriptEngineService.java +++ b/src/main/java/org/elasticsearch/script/mvel/MvelScriptEngineService.java @@ -36,6 +36,7 @@ import org.mvel2.compiler.ExecutableStatement; import org.mvel2.integration.impl.MapVariableResolverFactory; +import java.io.IOException; import java.lang.reflect.Method; import java.lang.reflect.Modifier; import java.util.HashMap; @@ -142,6 +143,43 @@ public Object unwrap(Object value) { public static class MvelSearchScript implements SearchScript { + /** + * Float encapsulation that allows updating the value with public + * member access. This is used to encapsulate the _score of a document + * so that updating the _score for the next document incurs only the + * overhead of setting a member variable + */ + private final class ScoreAccessor extends Number { + + float score() { + try { + return lookup.doc().score(); + } catch (IOException e) { + throw new RuntimeException("Could not get score", e); + } + } + + @Override + public int intValue() { + return (int)score(); + } + + @Override + public long longValue() { + return (long)score(); + } + + @Override + public float floatValue() { + return score(); + } + + @Override + public double doubleValue() { + return score(); + } + } + private final ExecutableStatement script; private final SearchLookup lookup; @@ -159,6 +197,7 @@ public MvelSearchScript(Object script, SearchLookup lookup, Map for (Map.Entry entry : lookup.asMap().entrySet()) { resolver.createVariable(entry.getKey(), entry.getValue()); } + resolver.createVariable("_score", new ScoreAccessor()); } @Override @@ -176,11 +215,6 @@ public void setNextDocId(int doc) { lookup.setNextDocId(doc); } - @Override - public void setNextScore(float score) { - resolver.createVariable("_score", score); - } - @Override public void setNextVar(String name, Object value) { resolver.createVariable(name, value); diff --git a/src/test/java/org/elasticsearch/search/aggregations/support/FieldDataSourceTests.java b/src/test/java/org/elasticsearch/search/aggregations/support/FieldDataSourceTests.java index 4f18bd4ee456a..27d0f7f56e573 100644 --- a/src/test/java/org/elasticsearch/search/aggregations/support/FieldDataSourceTests.java +++ b/src/test/java/org/elasticsearch/search/aggregations/support/FieldDataSourceTests.java @@ -90,10 +90,6 @@ public void setNextDocId(int doc) { public void setNextSource(Map source) { } - @Override - public void setNextScore(float score) { - } - @Override public float runAsFloat() { throw new UnsupportedOperationException(); diff --git a/src/test/java/org/elasticsearch/search/aggregations/support/ScriptValuesTests.java b/src/test/java/org/elasticsearch/search/aggregations/support/ScriptValuesTests.java index f1d839d38c252..088ed9fd11571 100644 --- a/src/test/java/org/elasticsearch/search/aggregations/support/ScriptValuesTests.java +++ b/src/test/java/org/elasticsearch/search/aggregations/support/ScriptValuesTests.java @@ -81,10 +81,6 @@ public void setNextDocId(int doc) { public void setNextSource(Map source) { } - @Override - public void setNextScore(float score) { - } - @Override public float runAsFloat() { throw new UnsupportedOperationException();