Deprecate negative scores in functon_score query (#35865)

Relates to #33309, backport for #35709

docs/reference/migration/migrate_6_0/search.asciidoc

@@ -150,6 +150,14 @@ produces. BM25 is recommended instead.
 See https://issues.apache.org/jira/browse/LUCENE-7347[`LUCENE-7347`] for more
 information.
 
+[float]
+===== Negative scores are deprecated in Function Score Query
+
+Negative scores in the Function Score Query are deprecated. If a negative 
+score is produced as a result of computation (e.g. in `script_score` or
+`field_value_factor` functions), a deprecation warning will be issued in
+this major version, and an error will be thrown in the next major version. 
+
 [float]
 ==== Fielddata on `_uid`
 

docs/reference/query-dsl/function-score-query.asciidoc

@@ -153,6 +153,9 @@ GET /_search
 // CONSOLE
 // TEST[setup:twitter]
 
+WARNING: Scores produced by the `script_score` function must be 
+non-negative, otherwise a deprecation warning will be issued.
+
 On top of the different scripting field values and expression, the
 `_score` script parameter can be used to retrieve the score based on the
 wrapped query.
@@ -324,6 +327,9 @@ There are a number of options for the `field_value_factor` function:
  values of the field with a range filter to avoid this, or use `log1p` and
  `ln1p`.
 
+WARNING: Scores produced by the `field_value_score` function must be 
+non-negative, otherwise a deprecation warning will be issued.
+
 [[function-decay]]
 ==== Decay functions
 

modules/lang-painless/src/test/resources/rest-api-spec/test/painless/30_search.yml

@@ -148,28 +148,6 @@
     - match: { hits.hits.0._id: "2" }
     - match: { hits.hits.1._id: "1" }
 
-    - do:
-        search:
-            index: test
-            body:
-                query:
-                    function_score:
-                        query:
-                            term:
-                                test: value
-                        "functions": [{
-                            "script_score": {
-                                "script": {
-                                    "lang": "painless",
-                                    "source": "-doc['num1'].value"
-                                }
-                            }
-                        }]
-
-    - match: { hits.total: 2 }
-    - match: { hits.hits.0._id: "1" }
-    - match: { hits.hits.1._id: "2" }
-
     - do:
         search:
             index: test
@@ -442,3 +420,39 @@
     - match: { error.root_cause.0.reason: "Iterable object is self-referencing itself" }
     - match: { error.type: "search_phase_execution_exception" }
     - match: { error.reason: "all shards failed" }
+
+---
+"Warning on negative score":
+    - skip:
+        version: " - 6.59.99"
+        reason: "check on negative scores was added from 6.6.0 on"
+        features: "warnings"
+
+    - do:
+        index:
+            index: test
+            type: test
+            id: 1
+            body: { "test": "value beck", "num1": 1.0 }
+    - do:
+        indices.refresh: {}
+
+    - do:
+        warnings:
+            - "Negative scores for script score function are deprecated, and will throw an error in the next major version. Got score: [-9.0]"
+        search:
+            index: test
+            body:
+                query:
+                    function_score:
+                        query:
+                            term:
+                                test: value
+                        "functions": [{
+                            "script_score": {
+                                "script": {
+                                    "lang": "painless",
+                                    "source": "doc['num1'].value - 10.0"
+                                }
+                            }
+                        }]

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

@@ -21,13 +21,15 @@
 
 import org.apache.lucene.index.LeafReaderContext;
 import org.apache.lucene.search.Explanation;
+import org.apache.logging.log4j.LogManager;
 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;
 import org.elasticsearch.index.fielddata.FieldData;
 import org.elasticsearch.index.fielddata.IndexNumericFieldData;
 import org.elasticsearch.index.fielddata.SortedNumericDoubleValues;
+import org.elasticsearch.common.logging.DeprecationLogger;
 
 import java.io.IOException;
 import java.util.Locale;
@@ -39,6 +41,9 @@
  * and applying a modification (log, ln, sqrt, square, etc) afterwards.
  */
 public class FieldValueFactorFunction extends ScoreFunction {
+
+    private static final DeprecationLogger deprecationLogger = new DeprecationLogger(LogManager.getLogger(FieldValueFactorFunction.class));
+
     private final String field;
     private final float boostFactor;
     private final Modifier modifier;
@@ -83,6 +88,13 @@ public double score(int docId, float subQueryScore) throws IOException {
                 }
                 double val = value * boostFactor;
                 double result = modifier.apply(val);
+                if (result < 0f) {
+                    deprecationLogger.deprecatedAndMaybeLog(
+                        "negative score in function score query",
+                        "Negative scores for field value function are deprecated," + 
+                            " and will throw an error in the next major version. Got: [" + result + "] for field value: [" + value + "]"
+                    );
+                }
                 return result;
             }
 

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

@@ -19,19 +19,23 @@
 
 package org.elasticsearch.common.lucene.search.function;
 
+import org.apache.logging.log4j.LogManager;
 import org.apache.lucene.index.LeafReaderContext;
 import org.apache.lucene.search.DocIdSetIterator;
 import org.apache.lucene.search.Explanation;
 import org.apache.lucene.search.Scorer;
 import org.elasticsearch.script.ExplainableSearchScript;
 import org.elasticsearch.script.ScoreScript;
 import org.elasticsearch.script.Script;
+import org.elasticsearch.common.logging.DeprecationLogger;
 
 import java.io.IOException;
 import java.util.Objects;
 
 public class ScriptScoreFunction extends ScoreFunction {
 
+    private static final DeprecationLogger deprecationLogger = new DeprecationLogger(LogManager.getLogger(ScriptScoreFunction.class));
+
     static final class CannedScorer extends Scorer {
         protected int docid;
         protected float score;
@@ -79,6 +83,12 @@ public double score(int docId, float subQueryScore) throws IOException {
                 scorer.docid = docId;
                 scorer.score = subQueryScore;
                 double result = leafScript.execute();
+                if (result < 0f) {
+                    deprecationLogger.deprecatedAndMaybeLog("negative score in function score query", 
+                        "Negative scores for script score function are deprecated," + 
+                            " and will throw an error in the next major version. Got score: [" + result + "]"
+                    );
+                }    
                 return result;
             }
 

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

@@ -825,6 +825,20 @@ public void testWithInvalidScores() {
         assertThat(exc.getMessage(), containsString("function score query returned an invalid score: " + Float.NEGATIVE_INFINITY));
     }
 
+    public void testWarningOnNegativeScores() throws IOException {
+        IndexSearcher localSearcher = new IndexSearcher(reader);
+        TermQuery termQuery = new TermQuery(new Term(FIELD, "out"));
+
+        // test that field_value_factor function issues a warning on negative scores
+        FieldValueFactorFunction.Modifier modifier = FieldValueFactorFunction.Modifier.NONE;
+        final ScoreFunction fvfFunction = new FieldValueFactorFunction(FIELD, -10,  modifier, 1.0, new IndexNumericFieldDataStub());
+        FunctionScoreQuery fsQuery =
+            new FunctionScoreQuery(termQuery, fvfFunction, CombineFunction.REPLACE, null, Float.POSITIVE_INFINITY);
+        localSearcher.search(fsQuery, 1); 
+        assertWarnings("Negative scores for field value function are deprecated," + 
+            " and will throw an error in the next major version. Got: [-10.0] for field value: [1.0]");
+    }
+
     private static class DummyScoreFunction extends ScoreFunction {
         protected DummyScoreFunction(CombineFunction scoreCombiner) {
             super(scoreCombiner);