Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

script_score comparison operators failing on 1.4.1 #8828

Closed
jsnod opened this issue Dec 8, 2014 · 3 comments · Fixed by #9094
Closed

script_score comparison operators failing on 1.4.1 #8828

jsnod opened this issue Dec 8, 2014 · 3 comments · Fixed by #9094
Assignees
Labels
>bug :Core/Infra/Scripting Scripting abstractions, Painless, and Mustache v1.4.3 v2.0.0-beta1

Comments

@jsnod
Copy link
Contributor

jsnod commented Dec 8, 2014

Hi,

We recently upgraded from v1.3.2 to v1.4.1. A function_score query using script_score is now failing when using > or < operators on the _score. Our script worked fine in 1.3, but after upgrading to 1.4.1 it fails with the following GroovyRuntimeException:

QueryPhaseExecutionException[[script_score_test][3]: query[filtered(function score (ConstantScore(:),function=script[if (_score > max_val) {0.0} else {_score}], params [{max_val=10.0}]))->cache(_type:my_type)],from[0],size[10]: Query Failed [Failed to execute main query]]; nested: GroovyScriptExecutionException[GroovyRuntimeException[Cannot compare org.elasticsearch.script.ScoreAccessor with value 'org.elasticsearch.script.ScoreAccessor@2553f628' and java.lang.Double with value '10.0']];

Not sure if this is a bug in Groovy or we need to change our script syntax to be compatible. Here are some sample steps to reproduce.

DELETE /script_score_test
PUT /script_score_test/my_type/1
{
    "prop1" : 5.3,
    "prop2" : 6.7
}
PUT /script_score_test/my_type/2
{
    "prop1" : 9.6,
    "prop2" : 15.4
}

The following query fails:

GET /script_score_test/my_type/_search
{
  "query": {
    "function_score": {
      "boost_mode": "replace",
      "script_score": {
        "script": "if (_score < max_val) {0.0} else {_score}",
        "params": {
          "max_val": 10
        }
      }
    }
  }
}

Interestingly, the query works when using the == operator. :

GET /script_score_test/my_type/_search
{
  "query": {
    "function_score": {
      "boost_mode": "replace",
      "script_score": {
        "script": "if (_score == max_val) {0.0} else {_score}",
        "params": {
          "max_val": 10
        }
      }
    }
  }
}

Also, using compareTo seems to work:

GET /script_score_test/my_type/_search
{
  "query": {
    "function_score": {
      "boost_mode": "replace",
      "script_score": {
        "script": "if (_score.compareTo(max_val) < 0) {0.0} else {_score}",
        "params": {
          "max_val": 10
        }
      }
    }
  }
}

We're OK with updating our scripts to use compareTo to make everything work, but we'd prefer to use the simpler < > syntax. I wanted to file this issue in case it's an actual bug or in case someone else comes across it.

@rjernst
Copy link
Member

rjernst commented Dec 8, 2014

@dakrone do you have any ideas? I made the script accessor a Number, and it seemed like that should work the same as the boxed types, but it doesn't appear to in this case? I also am skeptical that == is not just comparing reference equality of the boxed value with the script accessor?

@dakrone
Copy link
Member

dakrone commented Dec 9, 2014

@rjernst the ScriptAccessor needs to implement Comparable in order to work with < and >, this seems to fix it for me:

diff --git a/src/main/java/org/elasticsearch/script/ScoreAccessor.java b/src/main/java/org/elasticsearch/script/ScoreAccessor.java
index 93536e5..e8c4333 100644
--- a/src/main/java/org/elasticsearch/script/ScoreAccessor.java
+++ b/src/main/java/org/elasticsearch/script/ScoreAccessor.java
@@ -30,7 +30,7 @@ import java.io.IOException;
  * The provided {@link DocLookup} is used to retrieve the score
  * for the current document.
  */
-public final class ScoreAccessor extends Number {
+public final class ScoreAccessor extends Number implements Comparable<Number> {

     Scorer scorer;

@@ -65,4 +65,9 @@ public final class ScoreAccessor extends Number {
     public double doubleValue() {
         return score();
     }
+
+    @Override
+    public int compareTo(Number o) {
+        return Float.compare(this.score(), o.floatValue());
+    }
 }

And then the _score < max_val worked.

@s1monw
Copy link
Contributor

s1monw commented Dec 9, 2014

+1 for @dakrone fix

rjernst added a commit to rjernst/elasticsearch that referenced this issue Dec 31, 2014
rjernst added a commit that referenced this issue Dec 31, 2014
rjernst added a commit that referenced this issue Dec 31, 2014
rjernst added a commit that referenced this issue Dec 31, 2014
rjernst added a commit that referenced this issue Dec 31, 2014
mute pushed a commit to mute/elasticsearch that referenced this issue Jul 29, 2015
mute pushed a commit to mute/elasticsearch that referenced this issue Jul 29, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug :Core/Infra/Scripting Scripting abstractions, Painless, and Mustache v1.4.3 v2.0.0-beta1
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants