From 5cf65816d523f800008ee3eafc43011d732c28ab Mon Sep 17 00:00:00 2001 From: Panagiotis Bailis Date: Fri, 18 Oct 2024 17:17:21 +0300 Subject: [PATCH] adding validation for retrievers and scroll --- .../search/builder/SearchSourceBuilder.java | 2 +- .../retriever/CompoundRetrieverBuilder.java | 10 +++++++--- .../search/retriever/RetrieverBuilder.java | 1 + .../action/search/SearchRequestTests.java | 15 ++++++++++++++- 4 files changed, 23 insertions(+), 5 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/search/builder/SearchSourceBuilder.java b/server/src/main/java/org/elasticsearch/search/builder/SearchSourceBuilder.java index 6ceb02f0e797f..9c96319136007 100644 --- a/server/src/main/java/org/elasticsearch/search/builder/SearchSourceBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/builder/SearchSourceBuilder.java @@ -2179,7 +2179,7 @@ public ActionRequestValidationException validate( boolean allowPartialSearchResults ) { if (retriever() != null) { - validationException = retriever().validate(this, validationException, allowPartialSearchResults); + validationException = retriever().validate(this, validationException, isScroll, allowPartialSearchResults); List specified = new ArrayList<>(); if (subSearches().isEmpty() == false) { specified.add(QUERY_FIELD.getPreferredName()); diff --git a/server/src/main/java/org/elasticsearch/search/retriever/CompoundRetrieverBuilder.java b/server/src/main/java/org/elasticsearch/search/retriever/CompoundRetrieverBuilder.java index e994c55e43452..28f18429629ed 100644 --- a/server/src/main/java/org/elasticsearch/search/retriever/CompoundRetrieverBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/retriever/CompoundRetrieverBuilder.java @@ -172,9 +172,10 @@ public final void extractToSearchSourceBuilder(SearchSourceBuilder searchSourceB public ActionRequestValidationException validate( SearchSourceBuilder source, ActionRequestValidationException validationException, + boolean isScroll, boolean allowPartialSearchResults ) { - validationException = super.validate(source, validationException, allowPartialSearchResults); + validationException = super.validate(source, validationException, isScroll, allowPartialSearchResults); if (source.size() > rankWindowSize) { validationException = addValidationError( "[" @@ -190,12 +191,15 @@ public ActionRequestValidationException validate( } if (allowPartialSearchResults) { validationException = addValidationError( - "cannot specify a compound retriever and [allow_partial_search_results]", + "cannot specify [" + getName() + "] and [allow_partial_search_results]", validationException ); } + if (isScroll) { + validationException = addValidationError("cannot specify [" + getName() + "] and [scroll]", validationException); + } for (RetrieverSource innerRetriever : innerRetrievers) { - validationException = innerRetriever.retriever().validate(source, validationException, allowPartialSearchResults); + validationException = innerRetriever.retriever().validate(source, validationException, isScroll, allowPartialSearchResults); } return validationException; } diff --git a/server/src/main/java/org/elasticsearch/search/retriever/RetrieverBuilder.java b/server/src/main/java/org/elasticsearch/search/retriever/RetrieverBuilder.java index 882d44adb79c3..5e36ad0fd4fd6 100644 --- a/server/src/main/java/org/elasticsearch/search/retriever/RetrieverBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/retriever/RetrieverBuilder.java @@ -239,6 +239,7 @@ public RetrieverBuilder rewrite(QueryRewriteContext ctx) throws IOException { public ActionRequestValidationException validate( SearchSourceBuilder source, ActionRequestValidationException validationException, + boolean isScroll, boolean allowPartialSearchResults ) { return validationException; diff --git a/server/src/test/java/org/elasticsearch/action/search/SearchRequestTests.java b/server/src/test/java/org/elasticsearch/action/search/SearchRequestTests.java index c6ca97fd5694a..526961d74bf52 100644 --- a/server/src/test/java/org/elasticsearch/action/search/SearchRequestTests.java +++ b/server/src/test/java/org/elasticsearch/action/search/SearchRequestTests.java @@ -291,10 +291,23 @@ public void testValidate() throws IOException { assertNotNull(validationErrors); assertEquals(1, validationErrors.validationErrors().size()); assertEquals( - "cannot specify a compound retriever and [allow_partial_search_results]", + "cannot specify [test_compound_retriever_builder] and [allow_partial_search_results]", validationErrors.validationErrors().get(0) ); } + { + // scroll and compound retriever + SearchRequest searchRequest = createSearchRequest().source( + new SearchSourceBuilder().retriever(new TestCompoundRetrieverBuilder(randomIntBetween(1, 10))) + ); + searchRequest.allowPartialSearchResults(false); + searchRequest.scroll(TimeValue.timeValueMinutes(1)); + searchRequest.requestCache(false); + ActionRequestValidationException validationErrors = searchRequest.validate(); + assertNotNull(validationErrors); + assertEquals(1, validationErrors.validationErrors().size()); + assertEquals("cannot specify [test_compound_retriever_builder] and [scroll]", validationErrors.validationErrors().get(0)); + } { // allow_partial_results and non-compound retriever SearchRequest searchRequest = createSearchRequest().source(new SearchSourceBuilder().retriever(new RetrieverBuilder() {