Merge pull request #18247 from rmuir/needsScore

painless: implement needsScore() correctly.

modules/lang-painless/src/main/java/org/elasticsearch/painless/DynamicCallSite.java

@@ -29,8 +29,9 @@ import java.lang.invoke.MutableCallSite;
 /**
  * Painless invokedynamic call site.
  * <p>
- * Has 3 flavors (passed as static bootstrap parameters): dynamic method call,
+ * Has 5 flavors (passed as static bootstrap parameters): dynamic method call,
- * dynamic field load (getter), and dynamic field store (setter).
+ * dynamic field load (getter), and dynamic field store (setter), dynamic array load,
+ * and dynamic array store.
  * <p>
  * When a new type is encountered at the call site, we lookup from the appropriate
  * whitelist, and cache with a guard. If we encounter too many types, we stop caching.

modules/lang-painless/src/main/java/org/elasticsearch/painless/NeedsScore.java

@@ -0,0 +1,23 @@
+/*
+ * 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.painless;
+
+/** Marker interface that a generated {@link Executable} uses the {@code _score} value */
+public interface NeedsScore {
+}

modules/lang-painless/src/main/java/org/elasticsearch/painless/PainlessScriptEngineService.java

@@ -215,11 +215,10 @@ public class PainlessScriptEngineService extends AbstractComponent implements Sc
 
             /**
              * Whether or not the score is needed.
-             * @return Always true as it's assumed score is needed.
              */
             @Override
             public boolean needsScores() {
-                return true;
+                return compiledScript.compiled() instanceof NeedsScore;
             }
         };
     }

modules/lang-painless/src/main/java/org/elasticsearch/painless/Writer.java

@@ -84,7 +84,6 @@ import static org.elasticsearch.painless.WriterConstants.MAP_GET;
 import static org.elasticsearch.painless.WriterConstants.MAP_TYPE;
 import static org.elasticsearch.painless.WriterConstants.SCORE_ACCESSOR_FLOAT;
 import static org.elasticsearch.painless.WriterConstants.SCORE_ACCESSOR_TYPE;
-import static org.elasticsearch.painless.WriterConstants.SIGNATURE;
 
 class Writer extends PainlessParserBaseVisitor<Void> {
     static byte[] write(Metadata metadata) {
@@ -116,7 +115,7 @@ class Writer extends PainlessParserBaseVisitor<Void> {
         writeBegin();
         writeConstructor();
 
-        execute = new GeneratorAdapter(Opcodes.ACC_PUBLIC, EXECUTE, SIGNATURE, null, writer);
+        execute = new GeneratorAdapter(Opcodes.ACC_PUBLIC, EXECUTE, null, null, writer);
 
         final WriterUtility utility = new WriterUtility(metadata, execute);
         final WriterCaster caster = new WriterCaster(execute);
@@ -135,7 +134,14 @@ class Writer extends PainlessParserBaseVisitor<Void> {
         final String base = BASE_CLASS_TYPE.getInternalName();
         final String name = CLASS_TYPE.getInternalName();
 
-        writer.visit(version, access, name, null, base, null);
+        // apply marker interface NeedsScore if we use the score!
+        final String interfaces[];
+        if (metadata.scoreValueUsed) {
+            interfaces = new String[] { WriterConstants.NEEDS_SCORE_TYPE.getInternalName() };
+        } else {
+            interfaces = null;
+        }
+        writer.visit(version, access, name, null, base, interfaces);
         writer.visitSource(source, null);
     }
 

modules/lang-painless/src/main/java/org/elasticsearch/painless/WriterConstants.java

@@ -38,11 +38,12 @@ class WriterConstants {
 
     final static Method CONSTRUCTOR = getAsmMethod(void.class, "<init>", Definition.class, String.class, String.class);
     final static Method EXECUTE     = getAsmMethod(Object.class, "execute", Map.class);
-    final static String SIGNATURE   = "(Ljava/util/Map<Ljava/lang/String;Ljava/lang/Object;>;)Ljava/lang/Object;";
 
     final static Type PAINLESS_ERROR_TYPE = Type.getType(PainlessError.class);
 
     final static Type DEFINITION_TYPE = Type.getType(Definition.class);
+    
+    final static Type NEEDS_SCORE_TYPE = Type.getType(NeedsScore.class);
 
     final static Type OBJECT_TYPE = Type.getType(Object.class);
 

modules/lang-painless/src/test/java/org/elasticsearch/painless/NeedsScoreTests.java

@@ -0,0 +1,65 @@
+/*
+ * 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.painless;
+
+import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.index.IndexService;
+import org.elasticsearch.script.CompiledScript;
+import org.elasticsearch.script.ScriptService.ScriptType;
+import org.elasticsearch.script.SearchScript;
+import org.elasticsearch.search.lookup.SearchLookup;
+import org.elasticsearch.test.ESSingleNodeTestCase;
+
+import java.util.Collections;
+
+/**
+ * Test that needsScores() is reported correctly depending on whether _score is used
+ */
+// TODO: can we test this better? this is a port of the ExpressionsTests method.
+public class NeedsScoreTests extends ESSingleNodeTestCase {
+
+    public void testNeedsScores() {
+        IndexService index = createIndex("test", Settings.EMPTY, "type", "d", "type=double");
+
+        PainlessScriptEngineService service = new PainlessScriptEngineService(Settings.EMPTY);
+        SearchLookup lookup = new SearchLookup(index.mapperService(), index.fieldData(), null);
+
+        Object compiled = service.compile("1.2", Collections.emptyMap());
+        SearchScript ss = service.search(new CompiledScript(ScriptType.INLINE, "randomName", "painless", compiled), 
+                                         lookup, Collections.<String, Object>emptyMap());
+        assertFalse(ss.needsScores());
+
+        compiled = service.compile("input.doc['d'].value", Collections.emptyMap());
+        ss = service.search(new CompiledScript(ScriptType.INLINE, "randomName", "painless", compiled), 
+                            lookup, Collections.<String, Object>emptyMap());
+        assertFalse(ss.needsScores());
+
+        compiled = service.compile("1/_score", Collections.emptyMap());
+        ss = service.search(new CompiledScript(ScriptType.INLINE, "randomName", "painless", compiled), 
+                            lookup, Collections.<String, Object>emptyMap());
+        assertTrue(ss.needsScores());
+
+        compiled = service.compile("input.doc['d'].value * _score", Collections.emptyMap());
+        ss = service.search(new CompiledScript(ScriptType.INLINE, "randomName", "painless", compiled), 
+                            lookup, Collections.<String, Object>emptyMap());
+        assertTrue(ss.needsScores());
+    }
+
+}