Switch to Sending a Bad Request User When Function Score Query Generates Negative Scores (#133357)

Author: john-wagster

docs/changelog/133357.yaml

@@ -0,0 +1,7 @@
+pr: 133357
+summary: Switch to Sending a Bad Request User When Function Score Query Generates
+  Negative Scores
+area: Search
+type: bug
+issues: 
+ - 133358

rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search/610_function_score.yml

@@ -138,3 +138,47 @@
             }
   - length: { hits.hits: 2 }
   - match: { hits.total.value: 2 }
+
+---
+"formulating a function score query with a negative number returns bad request":
+  - requires:
+      cluster_features: [ "search.negative.function.score.bad.request" ]
+      reason: "Testing the behaviour change with this feature"
+  - do:
+      indices.create:
+        index: test
+        body:
+          settings:
+            number_of_shards: 1
+            number_of_replicas: 1
+          mappings:
+            properties:
+              qty:
+                type: float
+  - do:
+      index:
+        index: test
+        id: "1"
+        body: { qty: -1, uuid: 3456 }
+  - do:
+      indices.refresh: {}
+  - do:
+      catch: bad_request
+      search:
+        index: test
+        body:
+          query:
+            "function_score": {
+              "query": {
+                "match_all": {}
+              },
+              "field_value_factor": {
+                "field": "qty",
+                "factor": 1.2,
+                "missing": 1,
+                "modifier": "ln1p"
+              }
+            }
+  - match: { status: 400 }
+  - match: { error.root_cause.0.type: "illegal_argument_exception" }
+  - match: { error.root_cause.0.reason: "function score query returned an invalid score: NaN for doc: 0; score must be a non-negative real number" }

server/src/main/java/org/elasticsearch/common/lucene/search/function/FunctionScoreQuery.java

@@ -21,7 +21,6 @@
 import org.apache.lucene.search.ScorerSupplier;
 import org.apache.lucene.search.Weight;
 import org.apache.lucene.util.Bits;
-import org.elasticsearch.ElasticsearchException;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.io.stream.Writeable;
@@ -448,7 +447,13 @@ public float score() throws IOException {
                   These scores are invalid for score based {@link org.apache.lucene.search.TopDocsCollector}s.
                   See {@link org.apache.lucene.search.TopScoreDocCollector} for details.
                  */
-                throw new ElasticsearchException("function score query returned an invalid score: " + finalScore + " for doc: " + docId);
+                throw new IllegalArgumentException(
+                    "function score query returned an invalid score: "
+                        + finalScore
+                        + " for doc: "
+                        + docId
+                        + "; score must be a non-negative real number"
+                );
             }
             return finalScore;
         }

server/src/main/java/org/elasticsearch/search/SearchFeatures.java

@@ -34,6 +34,7 @@ public Set<NodeFeature> getFeatures() {
     public static final NodeFeature BBQ_HNSW_DEFAULT_INDEXING = new NodeFeature("search.vectors.mappers.default_bbq_hnsw");
     public static final NodeFeature SEARCH_WITH_NO_DIMENSIONS_BUGFIX = new NodeFeature("search.vectors.no_dimensions_bugfix");
     public static final NodeFeature SEARCH_RESCORE_SCRIPT = new NodeFeature("search.rescore.script");
+    public static final NodeFeature NEGATIVE_FUNCTION_SCORE_BAD_REQUEST = new NodeFeature("search.negative.function.score.bad.request");
 
     @Override
     public Set<NodeFeature> getTestFeatures() {
@@ -45,7 +46,8 @@ public Set<NodeFeature> getTestFeatures() {
             MULTI_MATCH_CHECKS_POSITIONS,
             BBQ_HNSW_DEFAULT_INDEXING,
             SEARCH_WITH_NO_DIMENSIONS_BUGFIX,
-            SEARCH_RESCORE_SCRIPT
+            SEARCH_RESCORE_SCRIPT,
+            NEGATIVE_FUNCTION_SCORE_BAD_REQUEST
         );
     }
 }

server/src/test/java/org/elasticsearch/index/query/functionscore/FunctionScoreTests.java

@@ -31,7 +31,6 @@
 import org.apache.lucene.tests.search.RandomApproximationQuery;
 import org.apache.lucene.util.Accountable;
 import org.apache.lucene.util.BytesRef;
-import org.elasticsearch.ElasticsearchException;
 import org.elasticsearch.common.lucene.search.function.CombineFunction;
 import org.elasticsearch.common.lucene.search.function.FieldValueFactorFunction;
 import org.elasticsearch.common.lucene.search.function.FunctionScoreQuery;
@@ -944,7 +943,7 @@ public void testWithInvalidScores() {
             null,
             Float.POSITIVE_INFINITY
         );
-        ElasticsearchException exc = expectThrows(ElasticsearchException.class, () -> localSearcher.search(query1, 1));
+        IllegalArgumentException exc = expectThrows(IllegalArgumentException.class, () -> localSearcher.search(query1, 1));
         assertThat(exc.getMessage(), containsString("function score query returned an invalid score: " + Float.NaN));
         FunctionScoreQuery query2 = new FunctionScoreQuery(
             new TermQuery(new Term(FIELD, "out")),
@@ -953,7 +952,7 @@ public void testWithInvalidScores() {
             null,
             Float.POSITIVE_INFINITY
         );
-        exc = expectThrows(ElasticsearchException.class, () -> localSearcher.search(query2, 1));
+        exc = expectThrows(IllegalArgumentException.class, () -> localSearcher.search(query2, 1));
         assertThat(exc.getMessage(), containsString("function score query returned an invalid score: " + Float.NEGATIVE_INFINITY));
     }