From e9c9c1086ca0b46673333edb9dea874de8cb7aa9 Mon Sep 17 00:00:00 2001 From: Mark Tozzi Date: Wed, 22 Aug 2018 17:47:05 -0400 Subject: [PATCH 1/5] Added deprecation warning for rescore in scroll queries --- .../java/org/elasticsearch/action/search/SearchRequest.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/server/src/main/java/org/elasticsearch/action/search/SearchRequest.java b/server/src/main/java/org/elasticsearch/action/search/SearchRequest.java index 05b06e65dea65..ebf5dcf1112a7 100644 --- a/server/src/main/java/org/elasticsearch/action/search/SearchRequest.java +++ b/server/src/main/java/org/elasticsearch/action/search/SearchRequest.java @@ -137,6 +137,10 @@ public ActionRequestValidationException validate() { if (source != null && source.size() == 0 && scroll != null) { validationException = addValidationError("[size] cannot be [0] in a scroll context", validationException); } + if (source != null && source.rescores() != null && source.rescores().isEmpty() == false && scroll != null) { + DEPRECATION_LOGGER.deprecated("Using [rescore] for a scroll query is deprecated and will return a 400 error in future " + + "versions"); + } return validationException; } From 1721d48264d78de8141864a45a33d0e0fefd3842 Mon Sep 17 00:00:00 2001 From: Mark Tozzi Date: Thu, 23 Aug 2018 16:47:36 -0400 Subject: [PATCH 2/5] Respons to PR feedback --- .../migration/migrate_6_0/search.asciidoc | 5 +++ .../action/search/SearchRequest.java | 4 +- .../search/SearchRequestTests.java | 45 +++++++++++++++++++ 3 files changed, 52 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..ce557a2a0c455 100644 --- a/docs/reference/migration/migrate_6_0/search.asciidoc +++ b/docs/reference/migration/migrate_6_0/search.asciidoc @@ -180,6 +180,11 @@ In prior versions the source field names were relative to the inner hit. The `from` parameter can no longer be used in the search request body when initiating a scroll. The parameter was already ignored in these situations, now in addition an error is thrown. +Using `rescore` with a scroll query now raises a deprecation warning and +ignores the parameter. In earlier 6.x releases, rescore on scroll queries was +silently ignored. In future versions, including we will return a `400 - Bad +Request` with a validation error. + ==== Limit on from/size in top hits and inner hits The maximum number of results (`from` + `size`) that is allowed to be retrieved diff --git a/server/src/main/java/org/elasticsearch/action/search/SearchRequest.java b/server/src/main/java/org/elasticsearch/action/search/SearchRequest.java index ebf5dcf1112a7..2090681aafdd9 100644 --- a/server/src/main/java/org/elasticsearch/action/search/SearchRequest.java +++ b/server/src/main/java/org/elasticsearch/action/search/SearchRequest.java @@ -138,8 +138,8 @@ public ActionRequestValidationException validate() { validationException = addValidationError("[size] cannot be [0] in a scroll context", validationException); } if (source != null && source.rescores() != null && source.rescores().isEmpty() == false && scroll != null) { - DEPRECATION_LOGGER.deprecated("Using [rescore] for a scroll query is deprecated and will return a 400 error in future " + - "versions"); + DEPRECATION_LOGGER.deprecated("Using [rescore] for a scroll query is deprecated and will be ignored. Future versions will " + + "return a 400 error"); } return validationException; } diff --git a/server/src/test/java/org/elasticsearch/search/SearchRequestTests.java b/server/src/test/java/org/elasticsearch/search/SearchRequestTests.java index eb2018f40c0a9..0ba61c4d36704 100644 --- a/server/src/test/java/org/elasticsearch/search/SearchRequestTests.java +++ b/server/src/test/java/org/elasticsearch/search/SearchRequestTests.java @@ -26,9 +26,15 @@ import org.elasticsearch.common.io.stream.BytesStreamOutput; import org.elasticsearch.common.io.stream.NamedWriteableAwareStreamInput; import org.elasticsearch.common.io.stream.StreamInput; +import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.common.util.ArrayUtils; +import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.index.query.QueryRewriteContext; +import org.elasticsearch.index.query.QueryShardContext; import org.elasticsearch.search.builder.SearchSourceBuilder; +import org.elasticsearch.search.rescore.RescorerBuilder; +import org.elasticsearch.search.rescore.RescoreContext; import java.io.IOException; import java.util.ArrayList; @@ -124,6 +130,17 @@ public void testValidate() throws IOException { assertEquals(1, validationErrors.validationErrors().size()); assertEquals("[size] cannot be [0] in a scroll context", validationErrors.validationErrors().get(0)); } + { + // Rescore is deprecated on scroll requests + SearchRequest searchRequest = createSearchRequest().source(new SearchSourceBuilder()); + searchRequest.source().addRescorer(new NoOpRescorerBuilder()); + searchRequest.requestCache(false); + searchRequest.scroll(new TimeValue(1000)); + ActionRequestValidationException validationErrors = searchRequest.validate(); + assertNull(validationErrors); + assertWarnings("Using [rescore] for a scroll query is deprecated and will be ignored. Future versions will return a 400 error"); + } + } public void testEqualsAndHashcode() throws IOException { @@ -165,4 +182,32 @@ private static SearchRequest copyRequest(SearchRequest searchRequest) throws IOE } return result; } + + private static class NoOpRescorerBuilder extends RescorerBuilder { + NoOpRescorerBuilder() throws IOException { + } + + @Override + public String getWriteableName() { + return "test"; + } + + @Override + public RescorerBuilder rewrite(QueryRewriteContext ctx) throws IOException { + return this; + } + + @Override + protected void doWriteTo(StreamOutput out) throws IOException { + } + + @Override + protected void doXContent(XContentBuilder builder, Params params) throws IOException { + } + + @Override + public RescoreContext innerBuildContext(int windowSize, QueryShardContext context) throws IOException { + return null; + } + } } From 23b4f52c1907c1b77ec357e74ef44fd86682e5c8 Mon Sep 17 00:00:00 2001 From: Mark Tozzi Date: Fri, 24 Aug 2018 14:38:26 -0400 Subject: [PATCH 3/5] Response to PR feedback --- .../migration/migrate_6_0/search.asciidoc | 5 --- docs/reference/migration/migrate_6_5.asciidoc | 10 +++++ .../action/search/SearchRequest.java | 2 +- .../search/SearchRequestTests.java | 40 ++----------------- 4 files changed, 15 insertions(+), 42 deletions(-) diff --git a/docs/reference/migration/migrate_6_0/search.asciidoc b/docs/reference/migration/migrate_6_0/search.asciidoc index ce557a2a0c455..c16e981a92d20 100644 --- a/docs/reference/migration/migrate_6_0/search.asciidoc +++ b/docs/reference/migration/migrate_6_0/search.asciidoc @@ -180,11 +180,6 @@ In prior versions the source field names were relative to the inner hit. The `from` parameter can no longer be used in the search request body when initiating a scroll. The parameter was already ignored in these situations, now in addition an error is thrown. -Using `rescore` with a scroll query now raises a deprecation warning and -ignores the parameter. In earlier 6.x releases, rescore on scroll queries was -silently ignored. In future versions, including we will return a `400 - Bad -Request` with a validation error. - ==== Limit on from/size in top hits and inner hits The maximum number of results (`from` + `size`) that is allowed to be retrieved diff --git a/docs/reference/migration/migrate_6_5.asciidoc b/docs/reference/migration/migrate_6_5.asciidoc index 913d9567cb850..522dae38270c1 100644 --- a/docs/reference/migration/migrate_6_5.asciidoc +++ b/docs/reference/migration/migrate_6_5.asciidoc @@ -23,3 +23,13 @@ Elasticsearch will log a warning on startup and log with the new pattern. It will not change the logging configuration files though. You should make this change before 7.0 because in 7.0 Elasticsearch will no longer automatically add the node name to the logging configuration if it isn't already present. + +=== Search changes + +==== Scroll + +Using `rescore` with a scroll query now raises a deprecation warning and +ignores the parameter. In earlier 6.x releases, rescore on scroll queries was +silently ignored. In 7.0 and later, we will return a `400 - Bad Request` with +a validation error. + diff --git a/server/src/main/java/org/elasticsearch/action/search/SearchRequest.java b/server/src/main/java/org/elasticsearch/action/search/SearchRequest.java index 2090681aafdd9..105d5d3aa7b08 100644 --- a/server/src/main/java/org/elasticsearch/action/search/SearchRequest.java +++ b/server/src/main/java/org/elasticsearch/action/search/SearchRequest.java @@ -138,7 +138,7 @@ public ActionRequestValidationException validate() { validationException = addValidationError("[size] cannot be [0] in a scroll context", validationException); } if (source != null && source.rescores() != null && source.rescores().isEmpty() == false && scroll != null) { - DEPRECATION_LOGGER.deprecated("Using [rescore] for a scroll query is deprecated and will be ignored. Future versions will " + + DEPRECATION_LOGGER.deprecated("Using [rescore] for a scroll query is deprecated and will be ignored. From 7.0 on will " + "return a 400 error"); } return validationException; diff --git a/server/src/test/java/org/elasticsearch/search/SearchRequestTests.java b/server/src/test/java/org/elasticsearch/search/SearchRequestTests.java index 0ba61c4d36704..d3423eafb2e77 100644 --- a/server/src/test/java/org/elasticsearch/search/SearchRequestTests.java +++ b/server/src/test/java/org/elasticsearch/search/SearchRequestTests.java @@ -26,15 +26,11 @@ import org.elasticsearch.common.io.stream.BytesStreamOutput; import org.elasticsearch.common.io.stream.NamedWriteableAwareStreamInput; import org.elasticsearch.common.io.stream.StreamInput; -import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.common.util.ArrayUtils; -import org.elasticsearch.common.xcontent.XContentBuilder; -import org.elasticsearch.index.query.QueryRewriteContext; -import org.elasticsearch.index.query.QueryShardContext; import org.elasticsearch.search.builder.SearchSourceBuilder; -import org.elasticsearch.search.rescore.RescorerBuilder; -import org.elasticsearch.search.rescore.RescoreContext; +import org.elasticsearch.search.rescore.QueryRescorerBuilder; +import org.elasticsearch.index.query.QueryBuilders; import java.io.IOException; import java.util.ArrayList; @@ -133,12 +129,12 @@ public void testValidate() throws IOException { { // Rescore is deprecated on scroll requests SearchRequest searchRequest = createSearchRequest().source(new SearchSourceBuilder()); - searchRequest.source().addRescorer(new NoOpRescorerBuilder()); + searchRequest.source().addRescorer(new QueryRescorerBuilder(QueryBuilders.matchAllQuery())); searchRequest.requestCache(false); searchRequest.scroll(new TimeValue(1000)); ActionRequestValidationException validationErrors = searchRequest.validate(); assertNull(validationErrors); - assertWarnings("Using [rescore] for a scroll query is deprecated and will be ignored. Future versions will return a 400 error"); + assertWarnings("Using [rescore] for a scroll query is deprecated and will be ignored. From 7.0 on will return a 400 error"); } } @@ -182,32 +178,4 @@ private static SearchRequest copyRequest(SearchRequest searchRequest) throws IOE } return result; } - - private static class NoOpRescorerBuilder extends RescorerBuilder { - NoOpRescorerBuilder() throws IOException { - } - - @Override - public String getWriteableName() { - return "test"; - } - - @Override - public RescorerBuilder rewrite(QueryRewriteContext ctx) throws IOException { - return this; - } - - @Override - protected void doWriteTo(StreamOutput out) throws IOException { - } - - @Override - protected void doXContent(XContentBuilder builder, Params params) throws IOException { - } - - @Override - public RescoreContext innerBuildContext(int windowSize, QueryShardContext context) throws IOException { - return null; - } - } } From 6f3a7af3a003dcbf2ff30d682ff7aadf8056511f Mon Sep 17 00:00:00 2001 From: Mark Tozzi Date: Fri, 24 Aug 2018 15:52:50 -0400 Subject: [PATCH 4/5] fixed import ordering --- .../test/java/org/elasticsearch/search/SearchRequestTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/test/java/org/elasticsearch/search/SearchRequestTests.java b/server/src/test/java/org/elasticsearch/search/SearchRequestTests.java index d3423eafb2e77..6fc0816168688 100644 --- a/server/src/test/java/org/elasticsearch/search/SearchRequestTests.java +++ b/server/src/test/java/org/elasticsearch/search/SearchRequestTests.java @@ -28,9 +28,9 @@ import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.common.util.ArrayUtils; +import org.elasticsearch.index.query.QueryBuilders; import org.elasticsearch.search.builder.SearchSourceBuilder; import org.elasticsearch.search.rescore.QueryRescorerBuilder; -import org.elasticsearch.index.query.QueryBuilders; import java.io.IOException; import java.util.ArrayList; From cfa6870454562e929b9ca5806ddb0f412b8dff7d Mon Sep 17 00:00:00 2001 From: Mark Tozzi Date: Mon, 27 Aug 2018 14:43:33 -0400 Subject: [PATCH 5/5] linked search changes section in docs --- docs/reference/migration/migrate_6_5.asciidoc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/reference/migration/migrate_6_5.asciidoc b/docs/reference/migration/migrate_6_5.asciidoc index fee6ce52009c8..432fe999982c6 100644 --- a/docs/reference/migration/migrate_6_5.asciidoc +++ b/docs/reference/migration/migrate_6_5.asciidoc @@ -5,6 +5,7 @@ This section discusses the changes that you need to be aware of when migrating your application to Elasticsearch 6.5. * <> +* <> * <> See also <> and <>. @@ -25,6 +26,7 @@ will not change the logging configuration files though. You should make this change before 7.0 because in 7.0 Elasticsearch will no longer automatically add the node name to the logging configuration if it isn't already present. +[[breaking_65_search_changes]] === Search changes ==== Scroll