diff --git a/CHANGELOG.md b/CHANGELOG.md index bd9ec337e9449..5be3fa6af7665 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Pass index settings to system ingest processor factories. ([#18708](https://github.com/opensearch-project/OpenSearch/pull/18708)) - Include named queries from rescore contexts in matched_queries array ([#18697](https://github.com/opensearch-project/OpenSearch/pull/18697)) - Add the configurable limit on rule cardinality ([#18663](https://github.com/opensearch-project/OpenSearch/pull/18663)) +- Disable approximation framework when dealing with multiple sorts ([#18763](https://github.com/opensearch-project/OpenSearch/pull/18763)) - [Experimental] Start in "clusterless" mode if a clusterless ClusterPlugin is loaded ([#18479](https://github.com/opensearch-project/OpenSearch/pull/18479)) - [Star-Tree] Add star-tree search related stats ([#18707](https://github.com/opensearch-project/OpenSearch/pull/18707)) - Add support for plugins to profile information ([#18656](https://github.com/opensearch-project/OpenSearch/pull/18656)) diff --git a/server/src/main/java/org/opensearch/search/approximate/ApproximateMatchAllQuery.java b/server/src/main/java/org/opensearch/search/approximate/ApproximateMatchAllQuery.java index 62557bf21eee9..bcd40095dbc8f 100644 --- a/server/src/main/java/org/opensearch/search/approximate/ApproximateMatchAllQuery.java +++ b/server/src/main/java/org/opensearch/search/approximate/ApproximateMatchAllQuery.java @@ -42,6 +42,9 @@ protected boolean canApproximate(SearchContext context) { } if (context.request() != null && context.request().source() != null && context.innerHits().getInnerHits().isEmpty()) { + if (context.request().source().sorts() != null && context.request().source().sorts().size() > 1) { + return false; + } FieldSortBuilder primarySortField = FieldSortBuilder.getPrimaryFieldSortOrNull(context.request().source()); if (primarySortField != null && primarySortField.missing() == null diff --git a/server/src/main/java/org/opensearch/search/approximate/ApproximatePointRangeQuery.java b/server/src/main/java/org/opensearch/search/approximate/ApproximatePointRangeQuery.java index e6d11d02eacf6..adecb8c89ef82 100644 --- a/server/src/main/java/org/opensearch/search/approximate/ApproximatePointRangeQuery.java +++ b/server/src/main/java/org/opensearch/search/approximate/ApproximatePointRangeQuery.java @@ -447,6 +447,9 @@ public boolean canApproximate(SearchContext context) { this.setSize(Math.max(context.from() + context.size(), context.trackTotalHitsUpTo()) + 1); } if (context.request() != null && context.request().source() != null) { + if (context.request().source().sorts() != null && context.request().source().sorts().size() > 1) { + return false; + } FieldSortBuilder primarySortField = FieldSortBuilder.getPrimaryFieldSortOrNull(context.request().source()); if (primarySortField != null) { if (!primarySortField.fieldName().equals(pointRangeQuery.getField())) { diff --git a/server/src/test/java/org/opensearch/search/approximate/ApproximatePointRangeQueryTests.java b/server/src/test/java/org/opensearch/search/approximate/ApproximatePointRangeQueryTests.java index a62a5568cc0bb..e7ef69b2ad8c6 100644 --- a/server/src/test/java/org/opensearch/search/approximate/ApproximatePointRangeQueryTests.java +++ b/server/src/test/java/org/opensearch/search/approximate/ApproximatePointRangeQueryTests.java @@ -33,7 +33,10 @@ import org.opensearch.common.time.DateFormatter; import org.opensearch.common.time.DateMathParser; import org.opensearch.index.mapper.DateFieldMapper.DateFieldType; +import org.opensearch.search.builder.SearchSourceBuilder; import org.opensearch.search.internal.SearchContext; +import org.opensearch.search.internal.ShardSearchRequest; +import org.opensearch.search.sort.FieldSortBuilder; import org.opensearch.search.sort.SortOrder; import org.opensearch.test.OpenSearchTestCase; @@ -1142,4 +1145,47 @@ private void verifyRangeQueries( approxDocsAsc.scoreDocs.length ); } + + public void testApproximateWithSort() { + long lower = RandomNumbers.randomLongBetween(random(), 0, 100); + long upper = RandomNumbers.randomLongBetween(random(), lower + 10, lower + 1000); + ApproximatePointRangeQuery query = new ApproximatePointRangeQuery( + numericType.fieldName, + numericType.encode(lower), + numericType.encode(upper), + 1, + numericType.format + ); + // Test 1: Multiple sorts should prevent approximation + { + SearchContext mockContext = mock(SearchContext.class); + ShardSearchRequest mockRequest = mock(ShardSearchRequest.class); + SearchSourceBuilder source = new SearchSourceBuilder(); + source.sort(new FieldSortBuilder(numericType.fieldName).order(SortOrder.ASC)); + source.sort(new FieldSortBuilder("another_field").order(SortOrder.ASC)); + source.terminateAfter(SearchContext.DEFAULT_TERMINATE_AFTER); + when(mockContext.aggregations()).thenReturn(null); + when(mockContext.trackTotalHitsUpTo()).thenReturn(10000); + when(mockContext.from()).thenReturn(0); + when(mockContext.size()).thenReturn(10); + when(mockContext.request()).thenReturn(mockRequest); + when(mockRequest.source()).thenReturn(source); + assertFalse("Should not approximate with multiple sorts", query.canApproximate(mockContext)); + } + // Test 2: Single sort on the same field should allow approximation + { + SearchContext mockContext = mock(SearchContext.class); + ShardSearchRequest mockRequest = mock(ShardSearchRequest.class); + SearchSourceBuilder source = new SearchSourceBuilder(); + source.sort(new FieldSortBuilder(numericType.fieldName).order(SortOrder.ASC)); + source.terminateAfter(SearchContext.DEFAULT_TERMINATE_AFTER); + when(mockContext.aggregations()).thenReturn(null); + when(mockContext.trackTotalHitsUpTo()).thenReturn(10000); + when(mockContext.from()).thenReturn(0); + when(mockContext.size()).thenReturn(10); + when(mockContext.request()).thenReturn(mockRequest); + when(mockRequest.source()).thenReturn(source); + assertTrue("Should approximate with single sort on same field", query.canApproximate(mockContext)); + } + } }