Skip to content

Commit

Permalink
Deprecate negative field boosts in analyzed queries (#38176)
Browse files Browse the repository at this point in the history
This change forbids negative field boost in the `query_string`, `simple_query_string`
and `multi_match` queries.
Negative boosts are not allowed in Lucene 8 (scores must be positive).
The backport of this change to 6x will turn the error into a deprecation warning
in order to raise the awareness of this breaking change in 7.0.

Closes #33309
  • Loading branch information
jimczi authored Feb 1, 2019
1 parent 3a40fb1 commit 51791f9
Show file tree
Hide file tree
Showing 10 changed files with 64 additions and 14 deletions.
8 changes: 4 additions & 4 deletions docs/reference/migration/migrate_6_0/search.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -153,10 +153,10 @@ information.
[float]
===== Negative scores are deprecated in Function Score Query

Negative scores in the Function Score Query are deprecated. If a negative
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.
this major version, and an error will be thrown in the next major version.

[float]
==== Fielddata on `_uid`
Expand Down Expand Up @@ -266,8 +266,8 @@ rewrite any prefix query on the field to a a single term query that matches the
[float]
==== Negative boosts are deprecated

Setting a negative `boost` in a query is deprecated and will throw an error in the next version.
To deboost a specific query you can use a `boost` comprise between 0 and 1.
Setting a negative `boost` for a query or a field is deprecated and will throw an error in the next
major version. To deboost a specific query or field you can use a `boost` comprise between 0 and 1.

[float]
==== Limit the number of open scroll contexts
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ protected AbstractQueryBuilder() {

protected AbstractQueryBuilder(StreamInput in) throws IOException {
boost = in.readFloat();
checkNegativeBoost(boost);
queryName = in.readOptionalString();
}

Expand Down Expand Up @@ -158,17 +159,21 @@ public final float boost() {
return this.boost;
}

protected final void checkNegativeBoost(float boost) {
if (Float.compare(boost, 0f) < 0) {
deprecationLogger.deprecatedAndMaybeLog("negative boost", "setting a negative [boost] on a query " +
"is deprecated and will throw an error in the next major version. You can use a value between 0 and 1 to deboost.");
}
}

/**
* Sets the boost for this query. Documents matching this query will (in addition to the normal
* weightings) have their score multiplied by the boost provided.
*/
@SuppressWarnings("unchecked")
@Override
public final QB boost(float boost) {
if (Float.compare(boost, 0f) < 0) {
deprecationLogger.deprecatedAndMaybeLog("negative boost", "setting a negative [boost] on a query " +
"is deprecated and will throw an error in the next version. You can use a value between 0 and 1 to deboost.");
}
checkNegativeBoost(boost);
this.boost = boost;
return (QB) this;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,10 @@ public MultiMatchQueryBuilder(StreamInput in) throws IOException {
int size = in.readVInt();
fieldsBoosts = new TreeMap<>();
for (int i = 0; i < size; i++) {
fieldsBoosts.put(in.readString(), in.readFloat());
String field = in.readString();
float boost = in.readFloat();
checkNegativeBoost(boost);
fieldsBoosts.put(field, boost);
}
type = Type.readFromStream(in);
operator = Operator.readFromStream(in);
Expand Down Expand Up @@ -293,6 +296,7 @@ public MultiMatchQueryBuilder field(String field, float boost) {
if (Strings.isEmpty(field)) {
throw new IllegalArgumentException("supplied field is null or empty.");
}
checkNegativeBoost(boost);
this.fieldsBoosts.put(field, boost);
return this;
}
Expand All @@ -301,6 +305,9 @@ public MultiMatchQueryBuilder field(String field, float boost) {
* Add several fields to run the query against with a specific boost.
*/
public MultiMatchQueryBuilder fields(Map<String, Float> fields) {
for (float fieldBoost : fields.values()) {
checkNegativeBoost(fieldBoost);
}
this.fieldsBoosts.putAll(fields);
return this;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,10 @@ public QueryStringQueryBuilder(StreamInput in) throws IOException {
defaultField = in.readOptionalString();
int size = in.readVInt();
for (int i = 0; i < size; i++) {
fieldsAndWeights.put(in.readString(), in.readFloat());
String field = in.readString();
Float weight = in.readFloat();
checkNegativeBoost(weight);
fieldsAndWeights.put(field, weight);
}
defaultOperator = Operator.readFromStream(in);
analyzer = in.readOptionalString();
Expand Down Expand Up @@ -339,6 +342,7 @@ public QueryStringQueryBuilder field(String field) {
* Adds a field to run the query string against with a specific boost.
*/
public QueryStringQueryBuilder field(String field, float boost) {
checkNegativeBoost(boost);
this.fieldsAndWeights.put(field, boost);
return this;
}
Expand All @@ -347,6 +351,9 @@ public QueryStringQueryBuilder field(String field, float boost) {
* Add several fields to run the query against with a specific boost.
*/
public QueryStringQueryBuilder fields(Map<String, Float> fields) {
for (float fieldBoost : fields.values()) {
checkNegativeBoost(fieldBoost);
}
this.fieldsAndWeights.putAll(fields);
return this;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,7 @@ public SimpleQueryStringBuilder(StreamInput in) throws IOException {
for (int i = 0; i < size; i++) {
String field = in.readString();
Float weight = in.readFloat();
checkNegativeBoost(weight);
fields.put(field, weight);
}
fieldsAndWeights.putAll(fields);
Expand Down Expand Up @@ -258,13 +259,17 @@ public SimpleQueryStringBuilder field(String field, float boost) {
if (Strings.isEmpty(field)) {
throw new IllegalArgumentException("supplied field is null or empty");
}
checkNegativeBoost(boost);
this.fieldsAndWeights.put(field, boost);
return this;
}

/** Add several fields to run the query against with a specific boost. */
public SimpleQueryStringBuilder fields(Map<String, Float> fields) {
Objects.requireNonNull(fields, "fields cannot be null");
for (float fieldBoost : fields.values()) {
checkNegativeBoost(fieldBoost);
}
this.fieldsAndWeights.putAll(fields);
return this;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,11 +96,10 @@ private List<Query> buildFieldQueries(MultiMatchQueryBuilder.Type type, Map<Stri
// ignore unmapped fields
continue;
}
Float boostValue = fieldNames.get(fieldName);
float boostValue = fieldNames.getOrDefault(fieldName, 1.0f);
Query query = parse(type.matchQueryType(), fieldName, value);
query = Queries.maybeApplyMinimumShouldMatch(query, minimumShouldMatch);
if (query != null
&& boostValue != null
&& boostValue != AbstractQueryBuilder.DEFAULT_BOOST
&& query instanceof MatchNoDocsQuery == false) {
query = new BoostQuery(query, boostValue);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -485,7 +485,7 @@ public void testWithStopWords() throws Exception {

public void testDisMaxDeprecation() throws Exception {
String json =
"{\n" +
"{\n" +
" \"multi_match\" : {\n" +
" \"query\" : \"foo:bar\",\n" +
" \"use_dis_max\" : true\n" +
Expand All @@ -496,6 +496,15 @@ public void testDisMaxDeprecation() throws Exception {
assertWarnings("Deprecated field [use_dis_max] used, replaced by [use tie_breaker instead]");
}

public void testNegativeFieldBoost() throws IOException {
Query query = new MultiMatchQueryBuilder("the quick fox")
.field(STRING_FIELD_NAME, -1.0f)
.field(STRING_FIELD_NAME_2)
.toQuery(createShardContext());
assertWarnings("setting a negative [boost] on a query is deprecated and will throw an error in the next major " +
"version. You can use a value between 0 and 1 to deboost.");
}

private static IndexMetaData newIndexMeta(String name, Settings oldIndexSettings, Settings indexSettings) {
Settings build = Settings.builder().put(oldIndexSettings)
.put(indexSettings)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1488,6 +1488,15 @@ public void testAnalyzedPrefix() throws Exception {
assertEquals(expected, query);
}

public void testNegativeFieldBoost() throws IOException {
Query query = new QueryStringQueryBuilder("the quick fox")
.field(STRING_FIELD_NAME, -1.0f)
.field(STRING_FIELD_NAME_2)
.toQuery(createShardContext());
assertWarnings("setting a negative [boost] on a query is deprecated and will throw an error in the next major " +
"version. You can use a value between 0 and 1 to deboost.");
}

private static IndexMetaData newIndexMeta(String name, Settings oldIndexSettings, Settings indexSettings) {
Settings build = Settings.builder().put(oldIndexSettings)
.put(indexSettings)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -739,6 +739,15 @@ public void testUnmappedFieldNoTokenWithAndOperator() throws IOException {
assertEquals(expected, query);
}

public void testNegativeFieldBoost() throws IOException {
Query query = new SimpleQueryStringBuilder("the quick fox")
.field(STRING_FIELD_NAME, -1.0f)
.field(STRING_FIELD_NAME_2)
.toQuery(createShardContext());
assertWarnings("setting a negative [boost] on a query is deprecated and will throw an error in the next major " +
"version. You can use a value between 0 and 1 to deboost.");
}

private static IndexMetaData newIndexMeta(String name, Settings oldIndexSettings, Settings indexSettings) {
Settings build = Settings.builder().put(oldIndexSettings)
.put(indexSettings)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ public void testNegativeBoosts() {
QB testQuery = createTestQueryBuilder();
testQuery.boost(-0.5f);
assertWarnings("setting a negative [boost] on a query" +
" is deprecated and will throw an error in the next version. You can use a value between 0 and 1 to deboost.");
" is deprecated and will throw an error in the next major version. You can use a value between 0 and 1 to deboost.");
}

/**
Expand Down

0 comments on commit 51791f9

Please sign in to comment.