diff --git a/CHANGELOG.md b/CHANGELOG.md index 5309dd0d66b69..da20dc9d97f0d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Add pull-based ingestion error metrics and make internal queue size configurable ([#18088](https://github.com/opensearch-project/OpenSearch/pull/18088)) - Enabled Async Shard Batch Fetch by default ([#18139](https://github.com/opensearch-project/OpenSearch/pull/18139)) - Allow to get the search request from the QueryCoordinatorContext ([#17818](https://github.com/opensearch-project/OpenSearch/pull/17818)) +- Improve sort-query performance by retaining the default `totalHitsThreshold` for approximated `match_all` queries ([#18189](https://github.com/opensearch-project/OpenSearch/pull/18189)) ### Changed diff --git a/server/src/internalClusterTest/java/org/opensearch/search/sort/FieldSortIT.java b/server/src/internalClusterTest/java/org/opensearch/search/sort/FieldSortIT.java index 53692292e4169..e8675ff3a6e55 100644 --- a/server/src/internalClusterTest/java/org/opensearch/search/sort/FieldSortIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/search/sort/FieldSortIT.java @@ -1145,7 +1145,7 @@ public void testSortMissingNumbersMinMax() throws Exception { .get(); assertNoFailures(searchResponse); - assertThat(searchResponse.getHits().getTotalHits().value(), equalTo(2L)); + assertThat(searchResponse.getHits().getTotalHits().value(), equalTo(3L)); assertThat(searchResponse.getHits().getAt(0).getId(), equalTo("1")); // The order here could be unstable (depends on document order) since missing == field value assertThat(searchResponse.getHits().getAt(1).getId(), is(oneOf("3", "2"))); 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 ae3593537f674..13b8007201971 100644 --- a/server/src/main/java/org/opensearch/search/approximate/ApproximatePointRangeQuery.java +++ b/server/src/main/java/org/opensearch/search/approximate/ApproximatePointRangeQuery.java @@ -449,7 +449,7 @@ public boolean canApproximate(SearchContext context) { if (context.from() + context.size() == 0) { this.setSize(SearchContext.DEFAULT_TRACK_TOTAL_HITS_UP_TO); } else { - this.setSize(Math.max(context.from() + context.size(), context.trackTotalHitsUpTo() + 1)); + this.setSize(Math.max(context.from() + context.size(), context.trackTotalHitsUpTo())); } if (context.request() != null && context.request().source() != null) { FieldSortBuilder primarySortField = FieldSortBuilder.getPrimaryFieldSortOrNull(context.request().source()); diff --git a/server/src/main/java/org/opensearch/search/approximate/ApproximateScoreQuery.java b/server/src/main/java/org/opensearch/search/approximate/ApproximateScoreQuery.java index cf9ead3f088c2..be1b6eed5333d 100644 --- a/server/src/main/java/org/opensearch/search/approximate/ApproximateScoreQuery.java +++ b/server/src/main/java/org/opensearch/search/approximate/ApproximateScoreQuery.java @@ -12,6 +12,7 @@ import org.apache.lucene.search.IndexSearcher; import org.apache.lucene.search.Query; import org.apache.lucene.search.QueryVisitor; +import org.apache.lucene.search.ScoreMode; import org.apache.lucene.search.Weight; import org.opensearch.search.internal.SearchContext; @@ -47,7 +48,11 @@ public Query rewrite(IndexSearcher indexSearcher) throws IOException { // Default to the original query. This suggests that we were not called from ContextIndexSearcher. return originalQuery.rewrite(indexSearcher); } - return resolvedQuery.rewrite(indexSearcher); + Query rewritten = resolvedQuery.rewrite(indexSearcher); + if (rewritten != resolvedQuery) { + resolvedQuery = rewritten; + } + return this; } public void setContext(SearchContext context) { @@ -78,6 +83,15 @@ public boolean equals(Object o) { return true; } + @Override + public Weight createWeight(IndexSearcher indexSearcher, ScoreMode scoreMode, float boost) throws IOException { + if (resolvedQuery == null) { + // Default to the original query. + return originalQuery.createWeight(indexSearcher, scoreMode, boost); + } + return resolvedQuery.createWeight(indexSearcher, scoreMode, boost); + } + @Override public int hashCode() { int h = classHash(); diff --git a/server/src/main/java/org/opensearch/search/query/TopDocsCollectorContext.java b/server/src/main/java/org/opensearch/search/query/TopDocsCollectorContext.java index dab7f30db6a7b..5c8515337e3e6 100644 --- a/server/src/main/java/org/opensearch/search/query/TopDocsCollectorContext.java +++ b/server/src/main/java/org/opensearch/search/query/TopDocsCollectorContext.java @@ -74,6 +74,7 @@ import org.opensearch.common.util.CachedSupplier; import org.opensearch.index.search.OpenSearchToParentBlockJoinQuery; import org.opensearch.search.DocValueFormat; +import org.opensearch.search.approximate.ApproximateScoreQuery; import org.opensearch.search.collapse.CollapseContext; import org.opensearch.search.internal.ScrollContext; import org.opensearch.search.internal.SearchContext; @@ -724,6 +725,8 @@ static int shortcutTotalHitCount(IndexReader reader, Query query) throws IOExcep query = ((ConstantScoreQuery) query).getQuery(); } else if (query instanceof BoostQuery) { query = ((BoostQuery) query).getQuery(); + } else if (query instanceof ApproximateScoreQuery) { + query = ((ApproximateScoreQuery) query).getOriginalQuery(); } else { break; } diff --git a/server/src/test/java/org/opensearch/index/mapper/DateFieldTypeTests.java b/server/src/test/java/org/opensearch/index/mapper/DateFieldTypeTests.java index 3fda7bcb1616a..daa6e4bcb8e7c 100644 --- a/server/src/test/java/org/opensearch/index/mapper/DateFieldTypeTests.java +++ b/server/src/test/java/org/opensearch/index/mapper/DateFieldTypeTests.java @@ -287,13 +287,20 @@ public void testRangeQuery() throws IOException { String date2 = "2016-04-28T11:33:52"; long instant1 = DateFormatters.from(DateFieldMapper.getDefaultDateTimeFormatter().parse(date1)).toInstant().toEpochMilli(); long instant2 = DateFormatters.from(DateFieldMapper.getDefaultDateTimeFormatter().parse(date2)).toInstant().toEpochMilli() + 999; - Query expected = new ApproximatePointRangeQuery( + ApproximatePointRangeQuery approximatePointRangeQuery = new ApproximatePointRangeQuery( "field", pack(new long[] { instant1 }).bytes, pack(new long[] { instant2 }).bytes, new long[] { instant1 }.length, ApproximatePointRangeQuery.LONG_FORMAT ); + Query expected = new ApproximateScoreQuery( + new IndexOrDocValuesQuery( + LongPoint.newRangeQuery("field", instant1, instant2), + SortedNumericDocValuesField.newSlowRangeQuery("field", instant1, instant2) + ), + approximatePointRangeQuery + ); Query rangeQuery = ft.rangeQuery(date1, date2, true, true, null, null, null, context); assertTrue(rangeQuery instanceof ApproximateScoreQuery); ((ApproximateScoreQuery) rangeQuery).setContext(new TestSearchContext(context));