Refactor implementations of query hpase searcher by adding overloaded…

… searchWith method

Signed-off-by: Martin Gaievski <gaievski@amazon.com>

CHANGELOG.md

@@ -70,6 +70,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - Refactoring globMatch using simpleMatchWithNormalizedStrings from Regex ([#13104](https://github.com/opensearch-project/OpenSearch/pull/13104))
 - [BWC and API enforcement] Reconsider the breaking changes check policy to detect breaking changes against released versions ([#13292](https://github.com/opensearch-project/OpenSearch/pull/13292))
 - Switch to macos-13 runner for precommit and assemble github actions due to macos-latest is now arm64 ([#13412](https://github.com/opensearch-project/OpenSearch/pull/13412))
+- Refactor implementations of query phase searcher, allow QueryCollectorContext to have zero collectors ([#XXX](https://github.com/opensearch-project/OpenSearch/pull/XXX))
 
 ### Deprecated
 

server/src/main/java/org/opensearch/search/query/ConcurrentQueryPhaseSearcher.java

@@ -23,6 +23,7 @@
 
 import java.io.IOException;
 import java.util.LinkedList;
+import java.util.Objects;
 import java.util.concurrent.ExecutionException;
 
 import static org.opensearch.search.query.TopDocsCollectorContext.createTopDocsCollectorContext;
@@ -49,21 +50,56 @@ protected boolean searchWithCollector(
         boolean hasFilterCollector,
         boolean hasTimeout
     ) throws IOException {
-        return searchWithCollectorManager(searchContext, searcher, query, collectors, hasFilterCollector, hasTimeout);
+        // create the top docs collector
+        final TopDocsCollectorContext topDocsFactory = createTopDocsCollectorContext(searchContext, hasFilterCollector);
+        return searchWithCollector(
+            searchContext,
+            searcher,
+            query,
+            collectors,
+            topDocsFactory,
+            hasFilterCollector,
+            hasTimeout,
+            topDocsFactory.shouldRescore()
+        );
+    }
+
+    protected boolean searchWithCollector(
+        SearchContext searchContext,
+        ContextIndexSearcher searcher,
+        Query query,
+        LinkedList<QueryCollectorContext> collectors,
+        QueryCollectorContext queryCollectorContext,
+        boolean hasFilterCollector,
+        boolean hasTimeout,
+        boolean shouldRescore
+    ) throws IOException {
+        return searchWithCollectorManager(
+            searchContext,
+            searcher,
+            query,
+            collectors,
+            queryCollectorContext,
+            hasFilterCollector,
+            hasTimeout,
+            shouldRescore
+        );
     }
 
     private static boolean searchWithCollectorManager(
         SearchContext searchContext,
         ContextIndexSearcher searcher,
         Query query,
         LinkedList<QueryCollectorContext> collectorContexts,
+        QueryCollectorContext queryCollectorContext,
         boolean hasFilterCollector,
-        boolean timeoutSet
+        boolean timeoutSet,
+        boolean shouldRescore
     ) throws IOException {
-        // create the top docs collector last when the other collectors are known
-        final TopDocsCollectorContext topDocsFactory = createTopDocsCollectorContext(searchContext, hasFilterCollector);
-        // add the top docs collector, the first collector context in the chain
-        collectorContexts.addFirst(topDocsFactory);
+        if (Objects.nonNull(queryCollectorContext)) {
+            // add the passed collector, the first collector context in the chain
+            collectorContexts.addFirst(queryCollectorContext);
+        }
 
         final QuerySearchResult queryResult = searchContext.queryResult();
         final CollectorManager<?, ReduceableSearchResult> collectorManager;
@@ -95,7 +131,7 @@ private static boolean searchWithCollectorManager(
             queryResult.terminatedEarly(false);
         }
 
-        return topDocsFactory.shouldRescore();
+        return shouldRescore;
     }
 
     @Override

server/src/main/java/org/opensearch/search/query/QueryCollectorContext.java

@@ -54,6 +54,7 @@
 import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
+import java.util.Objects;
 
 import static org.opensearch.search.profile.query.CollectorResult.REASON_SEARCH_MIN_SCORE;
 import static org.opensearch.search.profile.query.CollectorResult.REASON_SEARCH_MULTI;
@@ -248,7 +249,9 @@ CollectorManager<? extends Collector, ReduceableSearchResult> createManager(
                 CollectorManager<? extends Collector, ReduceableSearchResult> in
             ) throws IOException {
                 final List<CollectorManager<?, ReduceableSearchResult>> managers = new ArrayList<>();
-                managers.add(in);
+                if (Objects.nonNull(in)) {
+                    managers.add(in);
+                }
                 managers.addAll(subs);
                 return QueryCollectorManagerContext.createMultiCollectorManager(managers);
             }

server/src/main/java/org/opensearch/search/query/QueryPhase.java

@@ -335,13 +335,15 @@ private static boolean searchWithCollector(
         ContextIndexSearcher searcher,
         Query query,
         LinkedList<QueryCollectorContext> collectors,
+        QueryCollectorContext queryCollectorContext,
         boolean hasFilterCollector,
-        boolean timeoutSet
+        boolean timeoutSet,
+        boolean shouldRescore
     ) throws IOException {
-        // create the top docs collector last when the other collectors are known
-        final TopDocsCollectorContext topDocsFactory = createTopDocsCollectorContext(searchContext, hasFilterCollector);
-        // add the top docs collector, the first collector context in the chain
-        collectors.addFirst(topDocsFactory);
+        if (Objects.nonNull(queryCollectorContext)) {
+            // add passed collector, the first collector context in the chain
+            collectors.addFirst(queryCollectorContext);
+        }
 
         final Collector queryCollector;
         if (searchContext.getProfilers() != null) {
@@ -370,7 +372,7 @@ private static boolean searchWithCollector(
         for (QueryCollectorContext ctx : collectors) {
             ctx.postProcess(queryResult);
         }
-        return topDocsFactory.shouldRescore();
+        return shouldRescore;
     }
 
     /**
@@ -440,7 +442,40 @@ protected boolean searchWithCollector(
             boolean hasFilterCollector,
             boolean hasTimeout
         ) throws IOException {
-            return QueryPhase.searchWithCollector(searchContext, searcher, query, collectors, hasFilterCollector, hasTimeout);
+            // create the top docs collector last when the other collectors are known
+            final TopDocsCollectorContext topDocsFactory = createTopDocsCollectorContext(searchContext, hasFilterCollector);
+            return searchWithCollector(
+                searchContext,
+                searcher,
+                query,
+                collectors,
+                topDocsFactory,
+                hasFilterCollector,
+                hasTimeout,
+                topDocsFactory.shouldRescore()
+            );
+        }
+
+        protected boolean searchWithCollector(
+            SearchContext searchContext,
+            ContextIndexSearcher searcher,
+            Query query,
+            LinkedList<QueryCollectorContext> collectors,
+            QueryCollectorContext queryCollectorContext,
+            boolean hasFilterCollector,
+            boolean hasTimeout,
+            boolean shouldRescore
+        ) throws IOException {
+            return QueryPhase.searchWithCollector(
+                searchContext,
+                searcher,
+                query,
+                collectors,
+                queryCollectorContext,
+                hasFilterCollector,
+                hasTimeout,
+                shouldRescore
+            );
         }
     }
 }