From 84b61d073895e45fc2717812798d7f100f48402d Mon Sep 17 00:00:00 2001 From: Mark Tozzi Date: Tue, 28 Aug 2018 15:48:23 -0400 Subject: [PATCH] Scroll queries asking for rescore are considered invalid (#32918) This PR changes our behavior from silently ignoring rescore in a scroll query to instead report to the user that such a query is invalid. Closes #31775 --- .../reference/migration/migrate_7_0/search.asciidoc | 7 +++++++ .../elasticsearch/action/search/SearchRequest.java | 4 ++++ .../elasticsearch/search/SearchRequestTests.java | 13 +++++++++++++ 3 files changed, 24 insertions(+) diff --git a/docs/reference/migration/migrate_7_0/search.asciidoc b/docs/reference/migration/migrate_7_0/search.asciidoc index 76367115e13..a7d32896e97 100644 --- a/docs/reference/migration/migrate_7_0/search.asciidoc +++ b/docs/reference/migration/migrate_7_0/search.asciidoc @@ -54,6 +54,13 @@ Setting `request_cache:true` on a query that creates a scroll (`scroll=1m`) has been deprecated in 6 and will now return a `400 - Bad request`. Scroll queries are not meant to be cached. +==== Scroll queries cannot use `rescore` anymore +Including a rescore clause on a query that creates a scroll (`scroll=1m`) has +been deprecated in 6.5 and will now return a `400 - Bad request`. Allowing +rescore on scroll queries would break the scroll sort. In the 6.x line, the +rescore clause was silently ignored (for scroll queries), and it was allowed in +the 5.x line. + ==== Term Suggesters supported distance algorithms The following string distance algorithms were given additional names in 6.2 and 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 e560e53ed7b..dd7f6872943 100644 --- a/server/src/main/java/org/elasticsearch/action/search/SearchRequest.java +++ b/server/src/main/java/org/elasticsearch/action/search/SearchRequest.java @@ -184,6 +184,10 @@ public final class SearchRequest extends ActionRequest implements IndicesRequest 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) { + validationException = + addValidationError("using [rescore] is not allowed in a scroll context", validationException); + } 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 95a9ae9d707..36d2ef2c4db 100644 --- a/server/src/test/java/org/elasticsearch/search/SearchRequestTests.java +++ b/server/src/test/java/org/elasticsearch/search/SearchRequestTests.java @@ -28,7 +28,9 @@ import org.elasticsearch.common.io.stream.NamedWriteableAwareStreamInput; 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 java.io.IOException; import java.util.ArrayList; @@ -123,6 +125,17 @@ public class SearchRequestTests extends AbstractSearchTestCase { assertEquals(1, validationErrors.validationErrors().size()); assertEquals("[size] cannot be [0] in a scroll context", validationErrors.validationErrors().get(0)); } + { + // Rescore is not allowed on scroll requests + SearchRequest searchRequest = createSearchRequest().source(new SearchSourceBuilder()); + searchRequest.source().addRescorer(new QueryRescorerBuilder(QueryBuilders.matchAllQuery())); + searchRequest.requestCache(false); + searchRequest.scroll(new TimeValue(1000)); + ActionRequestValidationException validationErrors = searchRequest.validate(); + assertNotNull(validationErrors); + assertEquals(1, validationErrors.validationErrors().size()); + assertEquals("using [rescore] is not allowed in a scroll context", validationErrors.validationErrors().get(0)); + } } public void testEqualsAndHashcode() throws IOException {