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

Deprecate negative scores in functon_score query #35865

Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions docs/reference/migration/migrate_6_0/search.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -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`

Expand Down
6 changes: 6 additions & 0 deletions docs/reference/query-dsl/function-score-query.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,9 @@ GET /_search
// CONSOLE
// TEST[setup:twitter]

NOTE: Scores produced by the `script_score` function must be non-negative,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can maybe make it a WARNING: ?

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.
Expand Down Expand Up @@ -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`.

NOTE: Scores produced by the `field_value_score` function must be
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here

non-negative, otherwise a deprecation warning will be issued.

[[function-decay]]
==== Decay functions

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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"
}
}
}]
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,16 @@

import org.apache.lucene.index.LeafReaderContext;
import org.apache.lucene.search.Explanation;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
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;
Expand All @@ -39,6 +42,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;
Expand Down Expand Up @@ -83,6 +89,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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,19 +19,24 @@

package org.elasticsearch.common.lucene.search.function;

import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
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;
Expand Down Expand Up @@ -79,6 +84,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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down