Browse Source

Allow aggregations using expressions to use _score (#42652)

_score was removed from use in aggregations using expressions 
unintentionally when script contexts were added. This allows _score to once 
again be used.
Jack Conradson 6 years ago
parent
commit
37835cacde

+ 19 - 8
modules/lang-expression/src/main/java/org/elasticsearch/script/expression/ExpressionAggregationScript.java

@@ -38,20 +38,37 @@ class ExpressionAggregationScript implements AggregationScript.LeafFactory {
     final Expression exprScript;
     final SimpleBindings bindings;
     final DoubleValuesSource source;
+    final boolean needsScore;
     final ReplaceableConstDoubleValueSource specialValue; // _value
 
-    ExpressionAggregationScript(Expression e, SimpleBindings b, ReplaceableConstDoubleValueSource v) {
+    ExpressionAggregationScript(Expression e, SimpleBindings b, boolean n, ReplaceableConstDoubleValueSource v) {
         exprScript = e;
         bindings = b;
         source = exprScript.getDoubleValuesSource(bindings);
+        needsScore = n;
         specialValue = v;
     }
 
+    @Override
+    public boolean needs_score() {
+        return needsScore;
+    }
+
     @Override
     public AggregationScript newInstance(final LeafReaderContext leaf) throws IOException {
         return new AggregationScript() {
             // Fake the scorer until setScorer is called.
-            DoubleValues values = source.getValues(leaf, null);
+            DoubleValues values = source.getValues(leaf, new DoubleValues() {
+                @Override
+                public double doubleValue() throws IOException {
+                    return get_score().doubleValue();
+                }
+
+                @Override
+                public boolean advanceExact(int doc) throws IOException {
+                    return true;
+                }
+            });
 
             @Override
             public Object execute() {
@@ -84,10 +101,4 @@ class ExpressionAggregationScript implements AggregationScript.LeafFactory {
             }
         };
     }
-
-    @Override
-    public boolean needs_score() {
-        return false;
-    }
-
 }

+ 7 - 2
modules/lang-expression/src/main/java/org/elasticsearch/script/expression/ExpressionScriptEngine.java

@@ -221,10 +221,14 @@ public class ExpressionScriptEngine implements ScriptEngine {
         // NOTE: if we need to do anything complicated with bindings in the future, we can just extend Bindings,
         // instead of complicating SimpleBindings (which should stay simple)
         SimpleBindings bindings = new SimpleBindings();
+        boolean needsScores = false;
         ReplaceableConstDoubleValueSource specialValue = null;
         for (String variable : expr.variables) {
             try {
-                if (variable.equals("_value")) {
+                if (variable.equals("_score")) {
+                    bindings.add(new SortField("_score", SortField.Type.SCORE));
+                    needsScores = true;
+                } else if (variable.equals("_value")) {
                     specialValue = new ReplaceableConstDoubleValueSource();
                     bindings.add("_value", specialValue);
                     // noop: _value is special for aggregations, and is handled in ExpressionScriptBindings
@@ -237,6 +241,7 @@ public class ExpressionScriptEngine implements ScriptEngine {
                     // delegate valuesource creation based on field's type
                     // there are three types of "fields" to expressions, and each one has a different "api" of variables and methods.
                     final ValueSource valueSource = getDocValueSource(variable, lookup);
+                    needsScores |= valueSource.getSortField(false).needsScores();
                     bindings.add(variable, valueSource.asDoubleValuesSource());
                 }
             } catch (Exception e) {
@@ -244,7 +249,7 @@ public class ExpressionScriptEngine implements ScriptEngine {
                 throw convertToScriptException("link error", expr.sourceText, variable, e);
             }
         }
-        return new ExpressionAggregationScript(expr, bindings, specialValue);
+        return new ExpressionAggregationScript(expr, bindings, needsScores, specialValue);
     }
 
     private FieldScript.LeafFactory newFieldScript(Expression expr, SearchLookup lookup, @Nullable Map<String, Object> vars) {

+ 11 - 2
modules/lang-expression/src/test/java/org/elasticsearch/script/expression/MoreExpressionTests.java

@@ -28,8 +28,8 @@ 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.QueryBuilders;
-import org.elasticsearch.index.query.functionscore.ScoreFunctionBuilder;
 import org.elasticsearch.index.query.functionscore.ScoreFunctionBuilders;
+import org.elasticsearch.index.query.functionscore.ScriptScoreFunctionBuilder;
 import org.elasticsearch.plugins.Plugin;
 import org.elasticsearch.script.Script;
 import org.elasticsearch.script.ScriptType;
@@ -120,7 +120,7 @@ public class MoreExpressionTests extends ESIntegTestCase {
                 client().prepareIndex("test", "doc", "1").setSource("text", "hello goodbye"),
                 client().prepareIndex("test", "doc", "2").setSource("text", "hello hello hello goodbye"),
                 client().prepareIndex("test", "doc", "3").setSource("text", "hello hello goodebye"));
-        ScoreFunctionBuilder<?> score = ScoreFunctionBuilders.scriptFunction(
+        ScriptScoreFunctionBuilder score = ScoreFunctionBuilders.scriptFunction(
                 new Script(ScriptType.INLINE, "expression", "1 / _score", Collections.emptyMap()));
         SearchRequestBuilder req = client().prepareSearch().setIndices("test");
         req.setQuery(QueryBuilders.functionScoreQuery(QueryBuilders.termQuery("text", "hello"), score).boostMode(CombineFunction.REPLACE));
@@ -132,6 +132,15 @@ public class MoreExpressionTests extends ESIntegTestCase {
         assertEquals("1", hits.getAt(0).getId());
         assertEquals("3", hits.getAt(1).getId());
         assertEquals("2", hits.getAt(2).getId());
+
+        req = client().prepareSearch().setIndices("test");
+        req.setQuery(QueryBuilders.functionScoreQuery(QueryBuilders.termQuery("text", "hello"), score).boostMode(CombineFunction.REPLACE));
+        score = ScoreFunctionBuilders.scriptFunction(
+                new Script(ScriptType.INLINE, "expression", "1 / _score", Collections.emptyMap()));
+        req.addAggregation(AggregationBuilders.max("max_score").script((score).getScript()));
+        req.setSearchType(SearchType.DFS_QUERY_THEN_FETCH); // make sure DF is consistent
+        rsp = req.get();
+        assertSearchResponse(rsp);
     }
 
     public void testDateMethods() throws Exception {

+ 18 - 2
modules/lang-expression/src/test/resources/rest-api-spec/test/lang_expression/20_search.yml

@@ -25,9 +25,25 @@ setup:
         rest_total_hits_as_int: true
         body:
           script_fields:
-            my_field :
+            my_field:
               script:
                 lang: expression
                 source: 'doc["age"].value + 19'
 
-  - match:  { hits.hits.0.fields.my_field.0: 42.0 }
+  - match: { hits.hits.0.fields.my_field.0: 42.0 }
+
+---
+"Expressions aggregation score test":
+
+    - do:
+        search:
+          rest_total_hits_as_int: true
+          body:
+            aggs:
+              max_score:
+                max:
+                  script:
+                    lang: expression
+                    source: '_score'
+
+    - match: { aggregations.max_score.value: 1.0 }