-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Do not pass negative scores into function_score or script_score queries
In theory, Lucene scores should never go negative. To stop users from writing `function_score` and `script_score` queries that return negative values, we explicitly check their outputs and throw an exception when negative. Unfortunately, due to a subtle, more complicated bug in multi_match queries, sometimes those might (incorrectly) return negative scores. While that problem is also worth solving, we should protect function and script scoring from throwing an exception just for passing through a negative value that they had no hand in computing. Signed-off-by: Michael Froh <[email protected]>
- Loading branch information
Showing
8 changed files
with
250 additions
and
9 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
114 changes: 114 additions & 0 deletions
114
server/src/test/java/org/opensearch/index/query/NegativeBoostQuery.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,114 @@ | ||
| /* | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| * | ||
| * The OpenSearch Contributors require contributions made to | ||
| * this file be licensed under the Apache-2.0 license or a | ||
| * compatible open source license. | ||
| */ | ||
|
|
||
| package org.opensearch.index.query; | ||
|
|
||
| import org.apache.lucene.index.LeafReaderContext; | ||
| import org.apache.lucene.search.DocIdSetIterator; | ||
| import org.apache.lucene.search.Explanation; | ||
| import org.apache.lucene.search.IndexSearcher; | ||
| import org.apache.lucene.search.Query; | ||
| import org.apache.lucene.search.QueryVisitor; | ||
| import org.apache.lucene.search.ScoreMode; | ||
| import org.apache.lucene.search.Scorer; | ||
| import org.apache.lucene.search.Weight; | ||
|
|
||
| import java.io.IOException; | ||
|
|
||
| /** | ||
| * Similar to Lucene's BoostQuery, but will accept negative boost values (which is normally wrong, since scores | ||
| * should not be negative). Useful for testing that other query types guard against negative input scores. | ||
| */ | ||
| public class NegativeBoostQuery extends Query { | ||
| private final Query query; | ||
| private final float boost; | ||
|
|
||
| public NegativeBoostQuery(Query query, float boost) { | ||
| if (boost >= 0) { | ||
| throw new IllegalArgumentException("Expected negative boost. Use BoostQuery if boost is non-negative."); | ||
| } | ||
| this.boost = boost; | ||
| this.query = query; | ||
| } | ||
|
|
||
| @Override | ||
| public String toString(String field) { | ||
| StringBuilder builder = new StringBuilder(); | ||
| builder.append("("); | ||
| builder.append(query.toString(field)); | ||
| builder.append(")"); | ||
| builder.append("^"); | ||
| builder.append(boost); | ||
| return builder.toString(); | ||
| } | ||
|
|
||
| @Override | ||
| public void visit(QueryVisitor visitor) { | ||
| query.visit(visitor); | ||
| } | ||
|
|
||
| @Override | ||
| public boolean equals(Object other) { | ||
| return sameClassAs(other) && equalsTo(getClass().cast(other)); | ||
| } | ||
|
|
||
| private boolean equalsTo(NegativeBoostQuery other) { | ||
| return query.equals(other.query) && Float.floatToIntBits(boost) == Float.floatToIntBits(other.boost); | ||
| } | ||
|
|
||
| @Override | ||
| public int hashCode() { | ||
| int h = classHash(); | ||
| h = 31 * h + query.hashCode(); | ||
| h = 31 * h + Float.floatToIntBits(boost); | ||
| return h; | ||
| } | ||
|
|
||
| @Override | ||
| public Weight createWeight(IndexSearcher searcher, ScoreMode scoreMode, float boost) throws IOException { | ||
| final float negativeBoost = this.boost; | ||
| Weight delegate = query.createWeight(searcher, scoreMode, boost); | ||
| return new Weight(this) { | ||
| @Override | ||
| public Explanation explain(LeafReaderContext context, int doc) throws IOException { | ||
| return delegate.explain(context, doc); | ||
| } | ||
|
|
||
| @Override | ||
| public Scorer scorer(LeafReaderContext context) throws IOException { | ||
| Scorer delegateScorer = delegate.scorer(context); | ||
| return new Scorer(this) { | ||
| @Override | ||
| public DocIdSetIterator iterator() { | ||
| return delegateScorer.iterator(); | ||
| } | ||
|
|
||
| @Override | ||
| public float getMaxScore(int upTo) throws IOException { | ||
| return delegateScorer.getMaxScore(upTo); | ||
| } | ||
|
|
||
| @Override | ||
| public float score() throws IOException { | ||
| return delegateScorer.score() * negativeBoost; | ||
| } | ||
|
|
||
| @Override | ||
| public int docID() { | ||
| return delegateScorer.docID(); | ||
| } | ||
| }; | ||
| } | ||
|
|
||
| @Override | ||
| public boolean isCacheable(LeafReaderContext ctx) { | ||
| return delegate.isCacheable(ctx); | ||
| } | ||
| }; | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters