Browse Source

Throw exception in scroll requests using `from` (#26235)

The `from` search parameter cannot really be used in scrolled searches. This
commit adds a check for this case to the SearchRequest#validate() method so we
can reported it as an error rather than silently ignoring it.

Closes #9373
Christoph Büscher 8 years ago
parent
commit
4ff12c9a0b

+ 4 - 0
core/src/main/java/org/elasticsearch/action/search/SearchRequest.java

@@ -116,6 +116,10 @@ public final class SearchRequest extends ActionRequest implements IndicesRequest
             validationException =
                 addValidationError("disabling [track_total_hits] is not allowed in a scroll context", validationException);
         }
+        if (source != null && source.from() > 0 &&  scroll() != null) {
+            validationException =
+                addValidationError("using [from] is not allowed in a scroll context", validationException);
+        }
         return validationException;
     }
 

+ 33 - 0
core/src/test/java/org/elasticsearch/search/SearchRequestTests.java

@@ -19,6 +19,7 @@
 
 package org.elasticsearch.search;
 
+import org.elasticsearch.action.ActionRequestValidationException;
 import org.elasticsearch.action.search.SearchRequest;
 import org.elasticsearch.action.search.SearchType;
 import org.elasticsearch.action.support.IndicesOptions;
@@ -27,6 +28,7 @@ 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.search.builder.SearchSourceBuilder;
 
 import java.io.IOException;
 import java.util.ArrayList;
@@ -80,6 +82,37 @@ public class SearchRequestTests extends AbstractSearchTestCase {
         assertEquals("keepAlive must not be null", e.getMessage());
     }
 
+    public void testValidate() throws IOException {
+
+        {
+            // if scroll isn't set, validate should never add errors
+            SearchRequest searchRequest = createSearchRequest().source(new SearchSourceBuilder());
+            searchRequest.scroll((Scroll) null);
+            ActionRequestValidationException validationErrors = searchRequest.validate();
+            assertNull(validationErrors);
+        }
+        {
+        // disabeling `track_total_hits` isn't valid in scroll context
+            SearchRequest searchRequest = createSearchRequest().source(new SearchSourceBuilder());
+            searchRequest.scroll(new TimeValue(1000));
+            searchRequest.source().trackTotalHits(false);
+            ActionRequestValidationException validationErrors = searchRequest.validate();
+            assertNotNull(validationErrors);
+            assertEquals(1, validationErrors.validationErrors().size());
+            assertEquals("disabling [track_total_hits] is not allowed in a scroll context", validationErrors.validationErrors().get(0));
+        }
+        {
+            // scroll and `from` isn't valid
+            SearchRequest searchRequest = createSearchRequest().source(new SearchSourceBuilder());
+            searchRequest.scroll(new TimeValue(1000));
+            searchRequest.source().from(10);
+            ActionRequestValidationException validationErrors = searchRequest.validate();
+            assertNotNull(validationErrors);
+            assertEquals(1, validationErrors.validationErrors().size());
+            assertEquals("using [from] is not allowed in a scroll context", validationErrors.validationErrors().get(0));
+        }
+    }
+
     public void testEqualsAndHashcode() throws IOException {
         checkEqualsAndHashCode(createSearchRequest(), SearchRequestTests::copyRequest, this::mutate);
     }

+ 1 - 1
rest-api-spec/src/main/resources/rest-api-spec/test/search/30_limits.yml

@@ -32,7 +32,7 @@ setup:
       search:
         index: test_1
         scroll: 5m
-        from: 10000
+        size: 10010
 
 ---
 "Rescore window limits":