Changing exception to log message is we reach max number of nested le…

…vels Signed-off-by: Martin Gaievski <[email protected]>
opensearch-project · Nov 29, 2023 · a1f9774 · a1f9774
1 parent e33de9b
commit a1f9774
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 20 deletions.
diff --git a/src/main/java/org/opensearch/neuralsearch/search/query/HybridQueryPhaseSearcher.java b/src/main/java/org/opensearch/neuralsearch/search/query/HybridQueryPhaseSearcher.java
@@ -158,11 +158,9 @@ && mightBeWrappedHybridQuery(query)
      */
     private void validateQuery(final SearchContext searchContext, final Query query) {
         if (query instanceof BooleanQuery) {
-            Settings indexSettings = searchContext.getQueryShardContext().getIndexSettings().getSettings();
-            int maxDepthLimit = MapperService.INDEX_MAPPING_DEPTH_LIMIT_SETTING.get(indexSettings).intValue();
             List<BooleanClause> booleanClauses = ((BooleanQuery) query).clauses();
             for (BooleanClause booleanClause : booleanClauses) {
-                validateNestedBooleanQuery(booleanClause.getQuery(), maxDepthLimit);
+                validateNestedBooleanQuery(booleanClause.getQuery(), getMaxDepthLimit(searchContext));
             }
         }
     }
@@ -172,7 +170,11 @@ private void validateNestedBooleanQuery(final Query query, int level) {
             throw new IllegalArgumentException("hybrid query must be a top level query and cannot be wrapped into other queries");
         }
         if (level <= 0) {
-            throw new IllegalStateException("reached max nested query limit, cannot process query");
+            // ideally we should throw an error here but this code is on the main search workflow path and that might block
+            // execution of some queries. Instead, we're silently exit and allow such query to execute and potentially produce incorrect
+            // results in case hybrid query is wrapped into such bool query
+            log.error("reached max nested query limit, cannot process bool query with that many nested clauses");
+            return;
         }
         if (query instanceof BooleanQuery) {
             for (BooleanClause booleanClause : ((BooleanQuery) query).clauses()) {
@@ -324,4 +326,9 @@ private float getMaxScore(final List<TopDocs> topDocs) {
     private DocValueFormat[] getSortValueFormats(final SortAndFormats sortAndFormats) {
         return sortAndFormats == null ? null : sortAndFormats.formats;
     }
+
+    private int getMaxDepthLimit(final SearchContext searchContext) {
+        Settings indexSettings = searchContext.getQueryShardContext().getIndexSettings().getSettings();
+        return MapperService.INDEX_MAPPING_DEPTH_LIMIT_SETTING.get(indexSettings).intValue();
+    }
 }
diff --git a/src/test/java/org/opensearch/neuralsearch/search/query/HybridQueryPhaseSearcherTests.java b/src/test/java/org/opensearch/neuralsearch/search/query/HybridQueryPhaseSearcherTests.java
@@ -746,7 +746,7 @@ public void testWrappedHybridQuery_whenHybridWrappedIntoBoolBecauseOfNested_then
     }
 
     @SneakyThrows
-    public void testBoolQuery_whenTooManyNestedLevels_thenFail() {
+    public void testBoolQuery_whenTooManyNestedLevels_thenSuccess() {
         HybridQueryPhaseSearcher hybridQueryPhaseSearcher = new HybridQueryPhaseSearcher();
         QueryShardContext mockQueryShardContext = mock(QueryShardContext.class);
         when(mockQueryShardContext.index()).thenReturn(dummyIndex);
@@ -817,22 +817,17 @@ public void testBoolQuery_whenTooManyNestedLevels_thenFail() {
 
         when(searchContext.query()).thenReturn(query);
 
-        IllegalStateException exception = expectThrows(
-            IllegalStateException.class,
-            () -> hybridQueryPhaseSearcher.searchWith(
-                searchContext,
-                contextIndexSearcher,
-                query,
-                collectors,
-                hasFilterCollector,
-                hasTimeout
-            )
-        );
+        hybridQueryPhaseSearcher.searchWith(searchContext, contextIndexSearcher, query, collectors, hasFilterCollector, hasTimeout);
 
-        org.hamcrest.MatcherAssert.assertThat(
-            exception.getMessage(),
-            containsString("reached max nested query limit, cannot process quer")
-        );
+        assertNotNull(querySearchResult.topDocs());
+        TopDocsAndMaxScore topDocsAndMaxScore = querySearchResult.topDocs();
+        TopDocs topDocs = topDocsAndMaxScore.topDocs;
+        assertTrue(topDocs.totalHits.value > 0);
+        ScoreDoc[] scoreDocs = topDocs.scoreDocs;
+        assertNotNull(scoreDocs);
+        assertTrue(scoreDocs.length > 0);
+        assertFalse(isHybridQueryStartStopElement(scoreDocs[0]));
+        assertFalse(isHybridQueryStartStopElement(scoreDocs[scoreDocs.length - 1]));
 
         releaseResources(directory, w, reader);
     }