From 101c92a2cf24e237b23eb7ef6841d71c573290e1 Mon Sep 17 00:00:00 2001 From: lipsill Date: Wed, 12 Sep 2018 11:53:26 +0200 Subject: [PATCH 1/2] Deprecate negative `weight` in Function Score Query This change issues a deprecation message for negative `weight` in Function Score query. This is a preparation for Lucene 8 (used in 7.0) where negative scores are forbidden. --- .../functionscore/ScoreFunctionBuilder.java | 16 +++++++++++++--- .../FunctionScoreQueryBuilderTests.java | 7 +++++++ 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/query/functionscore/ScoreFunctionBuilder.java b/server/src/main/java/org/elasticsearch/index/query/functionscore/ScoreFunctionBuilder.java index c64f1b1e403c8..3d14fcebe9937 100644 --- a/server/src/main/java/org/elasticsearch/index/query/functionscore/ScoreFunctionBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/functionscore/ScoreFunctionBuilder.java @@ -19,12 +19,13 @@ package org.elasticsearch.index.query.functionscore; +import org.apache.logging.log4j.LogManager; import org.elasticsearch.common.io.stream.NamedWriteable; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.common.lucene.search.function.ScoreFunction; import org.elasticsearch.common.lucene.search.function.WeightFactorFunction; -import org.elasticsearch.common.xcontent.ToXContent.Params; import org.elasticsearch.common.xcontent.ToXContentFragment; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.index.query.QueryShardContext; @@ -34,6 +35,8 @@ public abstract class ScoreFunctionBuilder> implements ToXContentFragment, NamedWriteable { + private static final DeprecationLogger DEPRECATION_LOGGER = new DeprecationLogger(LogManager.getLogger(ScoreFunctionBuilder.class)); + private Float weight; /** @@ -46,7 +49,7 @@ public ScoreFunctionBuilder() { * Read from a stream. */ public ScoreFunctionBuilder(StreamInput in) throws IOException { - weight = in.readOptionalFloat(); + weight = checkWeight(in.readOptionalFloat()); } @Override @@ -70,10 +73,17 @@ public final void writeTo(StreamOutput out) throws IOException { */ @SuppressWarnings("unchecked") public final FB setWeight(float weight) { - this.weight = weight; + this.weight = checkWeight(weight); return (FB) this; } + private Float checkWeight(Float weight) { + if (weight != null && Float.compare(weight, 0) < 0) { + DEPRECATION_LOGGER.deprecated("[weight] cannot be negative for a filtering function"); + } + return weight; + } + /** * The weight applied to the function before combining. */ diff --git a/server/src/test/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryBuilderTests.java index 40f6605edf11d..dca428a56143b 100644 --- a/server/src/test/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryBuilderTests.java @@ -20,6 +20,7 @@ package org.elasticsearch.index.query.functionscore; import com.fasterxml.jackson.core.JsonParseException; + import org.apache.lucene.index.Term; import org.apache.lucene.search.MatchAllDocsQuery; import org.apache.lucene.search.Query; @@ -285,6 +286,12 @@ public void testIllegalArguments() { expectThrows(IllegalArgumentException.class, () -> builder.boostMode(null)); } + public void testDeprecatedArgumanets() { + float weight = -1 * randomFloat(); + new FunctionScoreQueryBuilder.FilterFunctionBuilder(new WeightBuilder().setWeight(weight)); + assertWarnings("[weight] cannot be negative for a filtering function"); + } + public void testParseFunctionsArray() throws IOException { String functionScoreQuery = "{\n" + " \"function_score\":{\n" + From e3c2fd9bd53de555abbeb185711c34978ea32644 Mon Sep 17 00:00:00 2001 From: lipsill Date: Wed, 12 Sep 2018 17:37:14 +0200 Subject: [PATCH 2/2] address reviewers comments --- docs/reference/migration/migrate_6_0/search.asciidoc | 2 ++ .../index/query/functionscore/ScoreFunctionBuilder.java | 3 ++- .../query/functionscore/FunctionScoreQueryBuilderTests.java | 3 ++- 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/docs/reference/migration/migrate_6_0/search.asciidoc b/docs/reference/migration/migrate_6_0/search.asciidoc index c16e981a92d20..bf24cbb5d6132 100644 --- a/docs/reference/migration/migrate_6_0/search.asciidoc +++ b/docs/reference/migration/migrate_6_0/search.asciidoc @@ -99,6 +99,8 @@ the `match` query but is supported for `match_phrase` and `match_phrase_prefix`. * The deprecated multi term rewrite parameters `constant_score_auto`, `constant_score_filter` (synonyms for `constant_score`) have been removed. +* Setting a negative `weight` in Function Score Query is deprecated. + ==== Search shards API The search shards API no longer accepts the `type` url parameter, which didn't diff --git a/server/src/main/java/org/elasticsearch/index/query/functionscore/ScoreFunctionBuilder.java b/server/src/main/java/org/elasticsearch/index/query/functionscore/ScoreFunctionBuilder.java index 3d14fcebe9937..20a1e660bd3e5 100644 --- a/server/src/main/java/org/elasticsearch/index/query/functionscore/ScoreFunctionBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/functionscore/ScoreFunctionBuilder.java @@ -79,7 +79,8 @@ public final FB setWeight(float weight) { private Float checkWeight(Float weight) { if (weight != null && Float.compare(weight, 0) < 0) { - DEPRECATION_LOGGER.deprecated("[weight] cannot be negative for a filtering function"); + DEPRECATION_LOGGER.deprecated("Setting a negative [weight] in Function Score Query is deprecated " + + "and will throw an error in the next major version"); } return weight; } diff --git a/server/src/test/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryBuilderTests.java index dca428a56143b..363ff6a578bd9 100644 --- a/server/src/test/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryBuilderTests.java @@ -289,7 +289,8 @@ public void testIllegalArguments() { public void testDeprecatedArgumanets() { float weight = -1 * randomFloat(); new FunctionScoreQueryBuilder.FilterFunctionBuilder(new WeightBuilder().setWeight(weight)); - assertWarnings("[weight] cannot be negative for a filtering function"); + assertWarnings("Setting a negative [weight] in Function Score Query is deprecated " + + "and will throw an error in the next major version"); } public void testParseFunctionsArray() throws IOException {