From a1f977472c98307e5f71938941037ff45aeb6982 Mon Sep 17 00:00:00 2001 From: Martin Gaievski Date: Wed, 29 Nov 2023 15:47:37 -0800 Subject: [PATCH] Changing exception to log message is we reach max number of nested levels Signed-off-by: Martin Gaievski --- .../query/HybridQueryPhaseSearcher.java | 15 ++++++++--- .../query/HybridQueryPhaseSearcherTests.java | 27 ++++++++----------- 2 files changed, 22 insertions(+), 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 index cf0c9c85e..fd29aec29 100644 --- 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 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) { 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 index 52f81f4d4..c4f3f4a3e 100644 --- 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); }