diff --git a/server/src/main/java/org/opensearch/search/SearchService.java b/server/src/main/java/org/opensearch/search/SearchService.java index a53a7198c366f..728ecd8f1d16d 100644 --- a/server/src/main/java/org/opensearch/search/SearchService.java +++ b/server/src/main/java/org/opensearch/search/SearchService.java @@ -36,7 +36,9 @@ import org.apache.logging.log4j.Logger; import org.apache.lucene.search.FieldDoc; import org.apache.lucene.search.IndexSearcher; +import org.apache.lucene.search.SortField; import org.apache.lucene.search.TopDocs; +import org.apache.lucene.util.BytesRef; import org.opensearch.OpenSearchException; import org.opensearch.action.ActionRunnable; import org.opensearch.action.OriginalIndices; @@ -132,10 +134,10 @@ import org.opensearch.search.rescore.RescorerBuilder; import org.opensearch.search.searchafter.SearchAfterBuilder; import org.opensearch.search.sort.FieldSortBuilder; +import org.opensearch.search.sort.FieldStats; import org.opensearch.search.sort.MinAndMax; import org.opensearch.search.sort.SortAndFormats; import org.opensearch.search.sort.SortBuilder; -import org.opensearch.search.sort.SortOrder; import org.opensearch.search.suggest.Suggest; import org.opensearch.search.suggest.completion.CompletionSuggestion; import org.opensearch.tasks.TaskResourceTrackingService; @@ -1622,8 +1624,11 @@ private CanMatchResponse canMatch(ShardSearchRequest request, boolean checkRefre ); Rewriteable.rewrite(request.getRewriteable(), context, false); final boolean aliasFilterCanMatch = request.getAliasFilter().getQueryBuilder() instanceof MatchNoneQueryBuilder == false; - FieldSortBuilder sortBuilder = FieldSortBuilder.getPrimaryFieldSortOrNull(request.source()); - MinAndMax minMax = sortBuilder != null ? FieldSortBuilder.getMinMaxOrNull(context, sortBuilder) : null; + final FieldSortBuilder sortBuilder = FieldSortBuilder.getPrimaryFieldSortOrNull(request.source()); + final SortAndFormats primarySort = sortBuilder != null + ? SortBuilder.buildSort(Collections.singletonList(sortBuilder), context).get() + : null; + FieldStats stats = sortBuilder != null ? FieldSortBuilder.getFieldStatsForShard(context, sortBuilder) : FieldStats.UNKNOWN; boolean canMatch; if (canRewriteToMatchNone(request.source())) { QueryBuilder queryBuilder = request.source().query(); @@ -1634,9 +1639,16 @@ private CanMatchResponse canMatch(ShardSearchRequest request, boolean checkRefre } final FieldDoc searchAfterFieldDoc = getSearchAfterFieldDoc(request, context); final Integer trackTotalHitsUpto = request.source() == null ? null : request.source().trackTotalHitsUpTo(); - canMatch = canMatch && canMatchSearchAfter(searchAfterFieldDoc, minMax, sortBuilder, trackTotalHitsUpto); + canMatch = canMatch + && canMatchSearchAfter( + searchAfterFieldDoc, + stats.getMinAndMax(), + primarySort, + trackTotalHitsUpto, + stats.allDocsNonMissing() + ); - return new CanMatchResponse(canMatch || hasRefreshPending, minMax); + return new CanMatchResponse(canMatch || hasRefreshPending, stats.getMinAndMax()); } } } @@ -1644,34 +1656,66 @@ private CanMatchResponse canMatch(ShardSearchRequest request, boolean checkRefre public static boolean canMatchSearchAfter( FieldDoc searchAfter, MinAndMax minMax, - FieldSortBuilder primarySortField, - Integer trackTotalHitsUpto + SortAndFormats primarySort, + Integer trackTotalHitsUpto, + boolean allDocsNonMissing ) { // Check for sort.missing == null, since in case of missing values sort queries, if segment/shard's min/max // is out of search_after range, it still should be printed and hence we should not skip segment/shard. // Skipping search on shard/segment entirely can cause mismatch on total_tracking_hits, hence skip only if // track_total_hits is false. if (searchAfter != null + && searchAfter.fields[0] != null && minMax != null - && primarySortField != null - && primarySortField.missing() == null + && primarySort != null && Objects.equals(trackTotalHitsUpto, SearchContext.TRACK_TOTAL_HITS_DISABLED)) { final Object searchAfterPrimary = searchAfter.fields[0]; - if (primarySortField.order() == SortOrder.DESC) { + SortField primarySortField = primarySort.sort.getSort()[0]; + if (primarySortField.getReverse()) { if (minMax.compareMin(searchAfterPrimary) > 0) { // In Desc order, if segment/shard minimum is gt search_after, the segment/shard won't be competitive - return false; + return allDocsNonMissing == false && canMatchMissingValue(primarySortField, searchAfterPrimary); } } else { if (minMax.compareMax(searchAfterPrimary) < 0) { // In ASC order, if segment/shard maximum is lt search_after, the segment/shard won't be competitive - return false; + return allDocsNonMissing == false && canMatchMissingValue(primarySortField, searchAfterPrimary); } } } return true; } + private static boolean canMatchMissingValue(SortField primarySortField, Object primarySearchAfter) { + final Object missingValue = primarySortField.getMissingValue(); + if (primarySortField.getReverse()) { + // the missing value of Type.STRING can only be SortField.STRING_FIRS or SortField.STRING_LAST + if (primarySearchAfter instanceof BytesRef) { + return missingValue == SortField.STRING_FIRST; + } + return compare(primarySearchAfter, missingValue) >= 0; + } else { + if (primarySearchAfter instanceof BytesRef) { + return missingValue == SortField.STRING_LAST; + } + return compare(primarySearchAfter, missingValue) <= 0; + } + } + + private static int compare(Object one, Object two) { + if (one instanceof Long) { + return Long.compare((Long) one, (Long) two); + } else if (one instanceof Integer) { + return Integer.compare((Integer) one, (Integer) two); + } else if (one instanceof Float) { + return Float.compare((Float) one, (Float) two); + } else if (one instanceof Double) { + return Double.compare((Double) one, (Double) two); + } else { + throw new UnsupportedOperationException("compare type not supported : " + one.getClass()); + } + } + private static FieldDoc getSearchAfterFieldDoc(ShardSearchRequest request, QueryShardContext context) throws IOException { if (context != null && request != null && request.source() != null && request.source().sorts() != null) { final List> sorts = request.source().sorts(); diff --git a/server/src/main/java/org/opensearch/search/internal/ContextIndexSearcher.java b/server/src/main/java/org/opensearch/search/internal/ContextIndexSearcher.java index ec3ed2332d0b8..0452da975cd47 100644 --- a/server/src/main/java/org/opensearch/search/internal/ContextIndexSearcher.java +++ b/server/src/main/java/org/opensearch/search/internal/ContextIndexSearcher.java @@ -78,7 +78,7 @@ import org.opensearch.search.query.QueryPhase; import org.opensearch.search.query.QuerySearchResult; import org.opensearch.search.sort.FieldSortBuilder; -import org.opensearch.search.sort.MinAndMax; +import org.opensearch.search.sort.FieldStats; import java.io.IOException; import java.util.ArrayList; @@ -517,17 +517,19 @@ private boolean canMatchSearchAfter(LeafReaderContext ctx) throws IOException { // Only applied on primary sort field and primary search_after. FieldSortBuilder primarySortField = FieldSortBuilder.getPrimaryFieldSortOrNull(searchContext.request().source()); if (primarySortField != null) { - MinAndMax minMax = FieldSortBuilder.getMinMaxOrNullForSegment( + FieldStats stats = FieldSortBuilder.getFieldStatsForSegment( this.searchContext.getQueryShardContext(), ctx, primarySortField, searchContext.sort() ); + assert stats != null; return SearchService.canMatchSearchAfter( searchContext.searchAfter(), - minMax, - primarySortField, - searchContext.trackTotalHitsUpTo() + stats.getMinAndMax(), + searchContext.sort(), + searchContext.trackTotalHitsUpTo(), + stats.allDocsNonMissing() ); } } diff --git a/server/src/main/java/org/opensearch/search/searchafter/SearchAfterBuilder.java b/server/src/main/java/org/opensearch/search/searchafter/SearchAfterBuilder.java index a45b2bd40c03d..e033756ef6af8 100644 --- a/server/src/main/java/org/opensearch/search/searchafter/SearchAfterBuilder.java +++ b/server/src/main/java/org/opensearch/search/searchafter/SearchAfterBuilder.java @@ -138,6 +138,10 @@ public static FieldDoc buildFieldDoc(SortAndFormats sort, Object[] values) { if (values[i] != null) { fieldValues[i] = convertValueFromSortField(values[i], sortField, format); } else { + SortField.Type sortType = extractSortType(sortField); + if (sortType != SortField.Type.STRING && sortType != SortField.Type.STRING_VAL) { + throw new IllegalArgumentException("search after value of type [" + sortType + "] cannot be null"); + } fieldValues[i] = null; } } diff --git a/server/src/main/java/org/opensearch/search/sort/FieldSortBuilder.java b/server/src/main/java/org/opensearch/search/sort/FieldSortBuilder.java index 5cecda1346b90..cbcdd6393af04 100644 --- a/server/src/main/java/org/opensearch/search/sort/FieldSortBuilder.java +++ b/server/src/main/java/org/opensearch/search/sort/FieldSortBuilder.java @@ -606,30 +606,30 @@ public static FieldSortBuilder getPrimaryFieldSortOrNull(SearchSourceBuilder sou } /** - * Return the {@link MinAndMax} indexed value for shard from the provided {@link FieldSortBuilder} or null if unknown. + * Return the {@link FieldStats} indexed value for shard from the provided {@link FieldSortBuilder} or {@link FieldStats#UNKNOWN} if unknown. * The value can be extracted on non-nested indexed mapped fields of type keyword, numeric or date, other fields - * and configurations return null. + * and configurations return {@link FieldStats#UNKNOWN}. */ - public static MinAndMax getMinMaxOrNull(QueryShardContext context, FieldSortBuilder sortBuilder) throws IOException { + public static FieldStats getFieldStatsForShard(QueryShardContext context, FieldSortBuilder sortBuilder) throws IOException { final SortAndFormats sort = SortBuilder.buildSort(Collections.singletonList(sortBuilder), context).get(); - return getMinMaxOrNullInternal(context.getIndexReader(), context, sortBuilder, sort); + return getFieldStatsInternal(context.getIndexReader(), context, sortBuilder, sort); } /** - * Return the {@link MinAndMax} indexed value for segment from the provided {@link FieldSortBuilder} or null if unknown. + * Return the {@link FieldStats} indexed value for segment from the provided {@link FieldSortBuilder} or {@link FieldStats#UNKNOWN} if unknown. * The value can be extracted on non-nested indexed mapped fields of type keyword, numeric or date, other fields - * and configurations return null. + * and configurations return {@link FieldStats#UNKNOWN}. */ - public static MinAndMax getMinMaxOrNullForSegment( + public static FieldStats getFieldStatsForSegment( QueryShardContext context, LeafReaderContext ctx, FieldSortBuilder sortBuilder, SortAndFormats sort ) throws IOException { - return getMinMaxOrNullInternal(ctx.reader(), context, sortBuilder, sort); + return getFieldStatsInternal(ctx.reader(), context, sortBuilder, sort); } - private static MinAndMax getMinMaxOrNullInternal( + private static FieldStats getFieldStatsInternal( IndexReader reader, QueryShardContext context, FieldSortBuilder sortBuilder, @@ -637,42 +637,46 @@ private static MinAndMax getMinMaxOrNullInternal( ) throws IOException { SortField sortField = sort.sort.getSort()[0]; if (sortField.getField() == null) { - return null; + return FieldStats.UNKNOWN; } MappedFieldType fieldType = context.fieldMapper(sortField.getField()); if (reader == null || (fieldType == null || fieldType.isSearchable() == false)) { - return null; + return FieldStats.UNKNOWN; } switch (IndexSortConfig.getSortFieldType(sortField)) { case LONG: case INT: case DOUBLE: case FLOAT: - return extractNumericMinAndMax(reader, sortField, fieldType, sortBuilder); + return extractNumericFieldStats(reader, sortField, fieldType, sortBuilder); case STRING: case STRING_VAL: if (fieldType instanceof KeywordFieldMapper.KeywordFieldType) { Terms terms = MultiTerms.getTerms(reader, fieldType.name()); if (terms == null) { - return null; + return FieldStats.UNKNOWN; } - return terms.getMin() != null ? new MinAndMax<>(terms.getMin(), terms.getMax()) : null; + MinAndMax minAndMax = terms.getMin() != null ? new MinAndMax<>(terms.getMin(), terms.getMax()) : null; + return new FieldStats(minAndMax, terms.getDocCount() == reader.maxDoc()); } break; } - return null; + return FieldStats.UNKNOWN; } - private static MinAndMax extractNumericMinAndMax( + private static FieldStats extractNumericFieldStats( IndexReader reader, SortField sortField, MappedFieldType fieldType, FieldSortBuilder sortBuilder ) throws IOException { String fieldName = fieldType.name(); - if (PointValues.size(reader, fieldName) == 0) { - return null; + final int docCount = PointValues.getDocCount(reader, fieldName); + if (docCount == 0) { + return FieldStats.UNKNOWN; } + final boolean allDocsNonMissing = docCount == reader.maxDoc(); + MinAndMax minAndMax = null; if (fieldType instanceof NumberFieldType) { NumberFieldType numberFieldType = (NumberFieldType) fieldType; Number minPoint = numberFieldType.parsePoint(PointValues.getMinPackedValue(reader, fieldName)); @@ -681,27 +685,31 @@ private static MinAndMax extractNumericMinAndMax( case LONG: if (numberFieldType.numericType() == NumericType.UNSIGNED_LONG) { // The min and max are expected to be BigInteger numbers - return new MinAndMax<>((BigInteger) minPoint, (BigInteger) maxPoint); + minAndMax = new MinAndMax<>((BigInteger) minPoint, (BigInteger) maxPoint); } else { - return new MinAndMax<>(minPoint.longValue(), maxPoint.longValue()); + minAndMax = new MinAndMax<>(minPoint.longValue(), maxPoint.longValue()); } + break; case INT: - return new MinAndMax<>(minPoint.intValue(), maxPoint.intValue()); + minAndMax = new MinAndMax<>(minPoint.intValue(), maxPoint.intValue()); + break; case DOUBLE: - return new MinAndMax<>(minPoint.doubleValue(), maxPoint.doubleValue()); + minAndMax = new MinAndMax<>(minPoint.doubleValue(), maxPoint.doubleValue()); + break; case FLOAT: - return new MinAndMax<>(minPoint.floatValue(), maxPoint.floatValue()); + minAndMax = new MinAndMax<>(minPoint.floatValue(), maxPoint.floatValue()); + break; default: - return null; + // no-op } } else if (fieldType instanceof DateFieldType) { DateFieldType dateFieldType = (DateFieldType) fieldType; Function dateConverter = createDateConverter(sortBuilder, dateFieldType); Long min = dateConverter.apply(PointValues.getMinPackedValue(reader, fieldName)); Long max = dateConverter.apply(PointValues.getMaxPackedValue(reader, fieldName)); - return new MinAndMax<>(min, max); + minAndMax = new MinAndMax<>(min, max); } - return null; + return new FieldStats(minAndMax, allDocsNonMissing); } private static Function createDateConverter(FieldSortBuilder sortBuilder, DateFieldType dateFieldType) { diff --git a/server/src/main/java/org/opensearch/search/sort/FieldStats.java b/server/src/main/java/org/opensearch/search/sort/FieldStats.java new file mode 100644 index 0000000000000..4c0efebd05288 --- /dev/null +++ b/server/src/main/java/org/opensearch/search/sort/FieldStats.java @@ -0,0 +1,40 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.search.sort; + +/** + * A class that encapsulates some stats about a field, including min/max etc. + * + * @opensearch.internal + */ +public class FieldStats { + public static final FieldStats UNKNOWN = new FieldStats(null, false); + + private final MinAndMax minAndMax; + private final boolean allDocsNonMissing; + + public FieldStats(MinAndMax minAndMax, boolean allDocsNonMissing) { + this.minAndMax = minAndMax; + this.allDocsNonMissing = allDocsNonMissing; + } + + /** + * Return the minimum and maximum value. + */ + public MinAndMax getMinAndMax() { + return minAndMax; + } + + /** + * Indicates whether all docs have values for corresponding field + */ + public boolean allDocsNonMissing() { + return allDocsNonMissing; + } +} diff --git a/server/src/test/java/org/opensearch/search/SearchServiceTests.java b/server/src/test/java/org/opensearch/search/SearchServiceTests.java index 1caa2c99fc3b8..213089433d658 100644 --- a/server/src/test/java/org/opensearch/search/SearchServiceTests.java +++ b/server/src/test/java/org/opensearch/search/SearchServiceTests.java @@ -36,9 +36,12 @@ import org.apache.lucene.index.LeafReader; import org.apache.lucene.search.FieldDoc; import org.apache.lucene.search.Query; +import org.apache.lucene.search.Sort; +import org.apache.lucene.search.SortField; import org.apache.lucene.store.AlreadyClosedException; import org.opensearch.OpenSearchException; import org.opensearch.action.OriginalIndices; +import org.opensearch.action.bulk.BulkRequestBuilder; import org.opensearch.action.index.IndexResponse; import org.opensearch.action.search.ClearScrollRequest; import org.opensearch.action.search.DeletePitResponse; @@ -54,10 +57,13 @@ import org.opensearch.action.support.IndicesOptions; import org.opensearch.action.support.PlainActionFuture; import org.opensearch.action.support.WriteRequest; +import org.opensearch.common.CheckedTriFunction; +import org.opensearch.common.Numbers; import org.opensearch.common.UUIDs; import org.opensearch.common.settings.Settings; import org.opensearch.common.settings.SettingsException; import org.opensearch.common.unit.TimeValue; +import org.opensearch.common.xcontent.XContentFactory; import org.opensearch.core.action.ActionListener; import org.opensearch.core.common.Strings; import org.opensearch.core.common.io.stream.StreamInput; @@ -66,6 +72,7 @@ import org.opensearch.core.index.Index; import org.opensearch.core.index.shard.ShardId; import org.opensearch.core.rest.RestStatus; +import org.opensearch.core.xcontent.MediaTypeRegistry; import org.opensearch.core.xcontent.XContentBuilder; import org.opensearch.index.IndexModule; import org.opensearch.index.IndexNotFoundException; @@ -73,6 +80,8 @@ import org.opensearch.index.IndexSettings; import org.opensearch.index.engine.Engine; import org.opensearch.index.mapper.DerivedFieldType; +import org.opensearch.index.mapper.MapperService; +import org.opensearch.index.mapper.NumberFieldMapper; import org.opensearch.index.query.AbstractQueryBuilder; import org.opensearch.index.query.MatchAllQueryBuilder; import org.opensearch.index.query.MatchNoneQueryBuilder; @@ -106,8 +115,9 @@ import org.opensearch.search.internal.ShardSearchContextId; import org.opensearch.search.internal.ShardSearchRequest; import org.opensearch.search.query.QuerySearchResult; -import org.opensearch.search.sort.FieldSortBuilder; import org.opensearch.search.sort.MinAndMax; +import org.opensearch.search.sort.SortAndFormats; +import org.opensearch.search.sort.SortBuilders; import org.opensearch.search.sort.SortOrder; import org.opensearch.search.suggest.SuggestBuilder; import org.opensearch.test.OpenSearchSingleNodeTestCase; @@ -115,7 +125,9 @@ import org.junit.Before; import java.io.IOException; +import java.math.BigInteger; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.LinkedList; @@ -1169,6 +1181,161 @@ public void testCanRewriteToMatchNone() { ); } + private Number randomNumber(NumberFieldMapper.NumberType type) { + switch (type) { + case BYTE: + return randomByte(); + case SHORT: + return randomShort(); + case INTEGER: + return randomInt(10000); + case LONG: + return randomLongBetween(0L, 10000L); + case DOUBLE: + return randomDoubleBetween(0, 10000, false); + case FLOAT: + case HALF_FLOAT: + return (float) randomDoubleBetween(0, 10000, false); + case UNSIGNED_LONG: + BigInteger ul = randomUnsignedLong(); + while (ul.compareTo(Numbers.MIN_UNSIGNED_LONG_VALUE) == 0 || ul.compareTo(Numbers.MAX_UNSIGNED_LONG_VALUE) == 0) { + ul = randomUnsignedLong(); + } + return ul; + default: + throw new AssertionError(); + } + } + + private Number incDecNumber(Number number, NumberFieldMapper.NumberType type, boolean inc) { + switch (type) { + case BYTE: + case SHORT: + case INTEGER: + return number.intValue() + (inc ? 1 : -1); + case LONG: + return number.longValue() + (inc ? 1 : -1); + case DOUBLE: + return number.doubleValue() + (inc ? 1 : -1); + case FLOAT: + case HALF_FLOAT: + return number.floatValue() + (inc ? 1 : -1); + case UNSIGNED_LONG: + return ((BigInteger) number).add(inc ? BigInteger.valueOf(1) : BigInteger.valueOf(-1)); + default: + throw new AssertionError(); + } + } + + private void canMatchSearchAfterNumericTestCase(NumberFieldMapper.NumberType type) throws Exception { + XContentBuilder mapping = XContentFactory.jsonBuilder() + .startObject() + .startObject("properties") + .startObject("num1") + .field("type", type.typeName()) + .endObject() + .startObject("num2") + .field("type", type.typeName()) + .endObject() + .endObject() + .endObject(); + IndexService indexService = createIndex("test", Settings.EMPTY, MapperService.SINGLE_MAPPING_NAME, mapping); + ensureGreen(); + + final boolean allDocsNonMissing = randomBoolean(); + final BulkRequestBuilder bulkRequestBuilder = client().prepareBulk(); + final int numDocs = randomIntBetween(10, 20); + final Number[] nums1 = new Number[allDocsNonMissing ? numDocs : numDocs - 1]; + final Number[] nums2 = new Number[numDocs]; + for (int i = 0; i < numDocs; i++) { + String source; + if (i < numDocs - 1 || allDocsNonMissing) { + nums1[i] = randomNumber(type); + nums2[i] = randomNumber(type); + source = String.format(Locale.ROOT, "{\"num1\": %s, \"num2\": %s}", nums1[i].toString(), nums2[i].toString()); + } else { + nums2[i] = randomNumber(type); + source = String.format(Locale.ROOT, "{\"num2\": %s}", nums2[i].toString()); + } + bulkRequestBuilder.add(client().prepareIndex("test").setId(String.valueOf(i)).setSource(source, MediaTypeRegistry.JSON)); + } + bulkRequestBuilder.get(); + client().admin().indices().prepareRefresh().get(); + Arrays.sort(nums1); + Arrays.sort(nums2); + + final IndexShard shard = indexService.getShard(0); + final SearchService service = getInstanceFromNode(SearchService.class); + final ShardSearchRequest shardRequest = new ShardSearchRequest( + OriginalIndices.NONE, + new SearchRequest().allowPartialSearchResults(true), + shard.shardId(), + 1, + new AliasFilter(null, Strings.EMPTY_ARRAY), + 1f, + -1, + null, + null + ); + assertFalse(shard.hasRefreshPending()); + + final CheckedTriFunction outOfRangeTester = (outOfRange, missingMatch, reverse) -> { + Number searchAfter; + Object missing; + if (outOfRange) { + searchAfter = reverse ? incDecNumber(nums1[0], type, false) : incDecNumber(nums1[nums1.length - 1], type, true); + } else { + searchAfter = randomFrom(nums1); + } + if (missingMatch) { + missing = reverse + ? (randomBoolean() ? "_last" : incDecNumber(searchAfter, type, false)) + : (randomBoolean() ? "_last" : incDecNumber(searchAfter, type, true)); + } else { + missing = reverse + ? (randomBoolean() ? "_first" : incDecNumber(searchAfter, type, true)) + : (randomBoolean() ? "_first" : incDecNumber(searchAfter, type, false)); + } + shardRequest.source(new SearchSourceBuilder().query(new MatchAllQueryBuilder()).trackTotalHits(false)); + shardRequest.source().sort(SortBuilders.fieldSort("num1").missing(missing).order(reverse ? SortOrder.DESC : SortOrder.ASC)); + List searchAfterFields = new ArrayList<>(); + searchAfterFields.add(searchAfter); + if (randomBoolean()) { + shardRequest.source().sort("num2"); + searchAfterFields.add(randomFrom(nums2)); + } + shardRequest.source().searchAfter(searchAfterFields.toArray()); + SearchService.CanMatchResponse response = service.canMatch(shardRequest); + + if (type == NumberFieldMapper.NumberType.HALF_FLOAT || type == NumberFieldMapper.NumberType.UNSIGNED_LONG) { + assertTrue(response.canMatch()); + return null; + } + + if (outOfRange == false || (allDocsNonMissing == false && missingMatch)) { + assertTrue(response.canMatch()); + } else { + assertFalse(response.canMatch()); + } + return null; + }; + for (boolean outOfRange : new boolean[] { true, false }) { + for (boolean missingMatch : new boolean[] { true, false }) { + for (boolean reverse : new boolean[] { true, false }) { + outOfRangeTester.apply(outOfRange, missingMatch, reverse); + } + } + } + client().admin().indices().prepareDelete("test").get(); + ensureGreen(); + } + + public void testNumericCanMatch() throws Exception { + for (var type : NumberFieldMapper.NumberType.values()) { + canMatchSearchAfterNumericTestCase(type); + } + } + public void testSetSearchThrottled() { createIndex("throttled_threadpool_index"); client().execute( @@ -1869,111 +2036,132 @@ public void validatePitStats(String index, long expectedPitCurrent, long expecte /** * Test for ASC order search_after query. - * Min = 0L, Max = 9L, search_after = 10L + * Min = 0L, Max = 9L, search_after = 10L, missing = Long.MIN_VALUE * Expected result is canMatch = false */ - public void testCanMatchSearchAfterAscGreaterThanMax() throws IOException { + public void testCanMatchSearchAfterAscGreaterThanMaxAndMissing() throws IOException { FieldDoc searchAfter = new FieldDoc(0, 0, new Long[] { 10L }); MinAndMax minMax = new MinAndMax(0L, 9L); - FieldSortBuilder primarySort = new FieldSortBuilder("test"); - primarySort.order(SortOrder.ASC); - assertEquals(SearchService.canMatchSearchAfter(searchAfter, minMax, primarySort, SearchContext.TRACK_TOTAL_HITS_DISABLED), false); + SortField sortField = new SortField("test", SortField.Type.LONG); + sortField.setMissingValue(Long.MIN_VALUE); + final SortAndFormats primarySort = new SortAndFormats(new Sort(sortField), new DocValueFormat[] { DocValueFormat.RAW }); + assertFalse(SearchService.canMatchSearchAfter(searchAfter, minMax, primarySort, SearchContext.TRACK_TOTAL_HITS_DISABLED, false)); } /** * Test for ASC order search_after query. - * Min = 0L, Max = 9L, search_after = 7L + * Min = 0L, Max = 9L, search_after = 10L, missing = 11L + * Expected result is canMatch = true + */ + public void testCanMatchSearchAfterAscGreaterThanMaxAndLessThanMissing() throws IOException { + FieldDoc searchAfter = new FieldDoc(0, 0, new Long[] { 10L }); + MinAndMax minMax = new MinAndMax(0L, 9L); + SortField sortField = new SortField("test", SortField.Type.LONG); + sortField.setMissingValue(11L); + final SortAndFormats primarySort = new SortAndFormats(new Sort(sortField), new DocValueFormat[] { DocValueFormat.RAW }); + assertTrue(SearchService.canMatchSearchAfter(searchAfter, minMax, primarySort, SearchContext.TRACK_TOTAL_HITS_DISABLED, false)); + } + + /** + * Test for ASC order search_after query. + * Min = 0L, Max = 9L, search_after = 7L, missing = Long.MIN_VALUE/10L * Expected result is canMatch = true */ public void testCanMatchSearchAfterAscLessThanMax() throws IOException { FieldDoc searchAfter = new FieldDoc(0, 0, new Long[] { 7L }); MinAndMax minMax = new MinAndMax(0L, 9L); - FieldSortBuilder primarySort = new FieldSortBuilder("test"); - primarySort.order(SortOrder.ASC); - assertEquals(SearchService.canMatchSearchAfter(searchAfter, minMax, primarySort, SearchContext.TRACK_TOTAL_HITS_DISABLED), true); + SortField sortField = new SortField("test", SortField.Type.LONG); + sortField.setMissingValue(randomFrom(Long.MIN_VALUE, 10L)); + final SortAndFormats primarySort = new SortAndFormats(new Sort(sortField), new DocValueFormat[] { DocValueFormat.RAW }); + assertTrue(SearchService.canMatchSearchAfter(searchAfter, minMax, primarySort, SearchContext.TRACK_TOTAL_HITS_DISABLED, false)); } /** * Test for ASC order search_after query. - * Min = 0L, Max = 9L, search_after = 9L + * Min = 0L, Max = 9L, search_after = 9L, missing = Long.MIN_VALUE/10L * Expected result is canMatch = true */ public void testCanMatchSearchAfterAscEqualMax() throws IOException { FieldDoc searchAfter = new FieldDoc(0, 0, new Long[] { 9L }); MinAndMax minMax = new MinAndMax(0L, 9L); - FieldSortBuilder primarySort = new FieldSortBuilder("test"); - primarySort.order(SortOrder.ASC); - assertEquals(SearchService.canMatchSearchAfter(searchAfter, minMax, primarySort, SearchContext.TRACK_TOTAL_HITS_DISABLED), true); + SortField sortField = new SortField("test", SortField.Type.LONG); + sortField.setMissingValue(randomFrom(Long.MIN_VALUE, 10L)); + final SortAndFormats primarySort = new SortAndFormats(new Sort(sortField), new DocValueFormat[] { DocValueFormat.RAW }); + assertTrue(SearchService.canMatchSearchAfter(searchAfter, minMax, primarySort, SearchContext.TRACK_TOTAL_HITS_DISABLED, false)); } /** * Test for DESC order search_after query. - * Min = 0L, Max = 9L, search_after = 10L + * Min = 0L, Max = 9L, search_after = 10L, missing = Long.MAX_VALUE/Long.MIN_VALUE * Expected result is canMatch = true */ public void testCanMatchSearchAfterDescGreaterThanMin() throws IOException { FieldDoc searchAfter = new FieldDoc(0, 0, new Long[] { 10L }); MinAndMax minMax = new MinAndMax(0L, 9L); - FieldSortBuilder primarySort = new FieldSortBuilder("test"); - primarySort.order(SortOrder.DESC); - assertEquals(SearchService.canMatchSearchAfter(searchAfter, minMax, primarySort, SearchContext.TRACK_TOTAL_HITS_DISABLED), true); + SortField sortField = new SortField("test", SortField.Type.LONG, true); + sortField.setMissingValue(randomFrom(Long.MAX_VALUE, Long.MIN_VALUE)); + final SortAndFormats primarySort = new SortAndFormats(new Sort(sortField), new DocValueFormat[] { DocValueFormat.RAW }); + assertTrue(SearchService.canMatchSearchAfter(searchAfter, minMax, primarySort, SearchContext.TRACK_TOTAL_HITS_DISABLED, false)); } /** * Test for DESC order search_after query. - * Min = 0L, Max = 9L, search_after = -1L + * Min = 0L, Max = 9L, search_after = -1L, missing > search_after * Expected result is canMatch = false */ - public void testCanMatchSearchAfterDescLessThanMin() throws IOException { + public void testCanMatchSearchAfterDescLessThanMinAndMissing() throws IOException { FieldDoc searchAfter = new FieldDoc(0, 0, new Long[] { -1L }); MinAndMax minMax = new MinAndMax(0L, 9L); - FieldSortBuilder primarySort = new FieldSortBuilder("test"); - primarySort.order(SortOrder.DESC); - assertEquals(SearchService.canMatchSearchAfter(searchAfter, minMax, primarySort, SearchContext.TRACK_TOTAL_HITS_DISABLED), false); + SortField sortField = new SortField("test", SortField.Type.LONG, true); + sortField.setMissingValue(randomLongBetween(0L, Long.MAX_VALUE)); + final SortAndFormats primarySort = new SortAndFormats(new Sort(sortField), new DocValueFormat[] { DocValueFormat.RAW }); + assertFalse(SearchService.canMatchSearchAfter(searchAfter, minMax, primarySort, SearchContext.TRACK_TOTAL_HITS_DISABLED, false)); } /** * Test for DESC order search_after query. - * Min = 0L, Max = 9L, search_after = 0L + * Min = 0L, Max = 9L, search_after = 0L, missing > search_after * Expected result is canMatch = true */ - public void testCanMatchSearchAfterDescEqualMin() throws IOException { + public void testCanMatchSearchAfterDescEqualMinAndLessThanMissing() throws IOException { FieldDoc searchAfter = new FieldDoc(0, 0, new Long[] { 0L }); MinAndMax minMax = new MinAndMax(0L, 9L); - FieldSortBuilder primarySort = new FieldSortBuilder("test"); - primarySort.order(SortOrder.DESC); - assertEquals(SearchService.canMatchSearchAfter(searchAfter, minMax, primarySort, SearchContext.TRACK_TOTAL_HITS_DISABLED), true); + SortField sortField = new SortField("test", SortField.Type.LONG, true); + sortField.setMissingValue(randomLongBetween(1L, Long.MAX_VALUE)); + final SortAndFormats primarySort = new SortAndFormats(new Sort(sortField), new DocValueFormat[] { DocValueFormat.RAW }); + assertTrue(SearchService.canMatchSearchAfter(searchAfter, minMax, primarySort, SearchContext.TRACK_TOTAL_HITS_DISABLED, false)); } /** * Test canMatchSearchAfter with missing value, even if min/max is out of range * Min = 0L, Max = 9L, search_after = -1L - * Expected result is canMatch = true */ - public void testCanMatchSearchAfterWithMissing() throws IOException { + public void testCanMatchSearchAfterWithMinMaxOutOfRangeAndDifferentMissing() throws IOException { FieldDoc searchAfter = new FieldDoc(0, 0, new Long[] { -1L }); MinAndMax minMax = new MinAndMax(0L, 9L); - FieldSortBuilder primarySort = new FieldSortBuilder("test"); - primarySort.order(SortOrder.DESC); - // Should be false without missing values - assertEquals(SearchService.canMatchSearchAfter(searchAfter, minMax, primarySort, SearchContext.TRACK_TOTAL_HITS_DISABLED), false); - primarySort.missing("_last"); - // Should be true with missing values - assertEquals(SearchService.canMatchSearchAfter(searchAfter, minMax, primarySort, SearchContext.TRACK_TOTAL_HITS_DISABLED), true); + SortField sortField = new SortField("test", SortField.Type.LONG, true); + sortField.setMissingValue(randomLongBetween(0L, Long.MAX_VALUE)); + final SortAndFormats primarySort = new SortAndFormats(new Sort(sortField), new DocValueFormat[] { DocValueFormat.RAW }); + // Should be false with missing values is larger than search_after + assertFalse(SearchService.canMatchSearchAfter(searchAfter, minMax, primarySort, SearchContext.TRACK_TOTAL_HITS_DISABLED, false)); + sortField.setMissingValue(Long.MIN_VALUE); + // Should be true with missing values is less than search_after + assertTrue(SearchService.canMatchSearchAfter(searchAfter, minMax, primarySort, SearchContext.TRACK_TOTAL_HITS_DISABLED, false)); } /** * Test for DESC order search_after query with track_total_hits=true. - * Min = 0L, Max = 9L, search_after = -1L + * Min = 0L, Max = 9L, search_after = -1L, missing > search_after * With above min/max and search_after, it should not match, but since * track_total_hits = true, * Expected result is canMatch = true */ - public void testCanMatchSearchAfterDescLessThanMinWithTrackTotalhits() throws IOException { + public void testCanMatchSearchAfterDescLessThanMinAndMissingWithTrackTotalHits() throws IOException { FieldDoc searchAfter = new FieldDoc(0, 0, new Long[] { -1L }); MinAndMax minMax = new MinAndMax(0L, 9L); - FieldSortBuilder primarySort = new FieldSortBuilder("test"); - primarySort.order(SortOrder.DESC); - assertEquals(SearchService.canMatchSearchAfter(searchAfter, minMax, primarySort, 1000), true); + SortField sortField = new SortField("test", SortField.Type.LONG, true); + sortField.setMissingValue(randomLongBetween(0L, Long.MAX_VALUE)); + final SortAndFormats primarySort = new SortAndFormats(new Sort(sortField), new DocValueFormat[] { DocValueFormat.RAW }); + assertTrue(SearchService.canMatchSearchAfter(searchAfter, minMax, primarySort, 1000, false)); } } diff --git a/server/src/test/java/org/opensearch/search/sort/FieldSortBuilderTests.java b/server/src/test/java/org/opensearch/search/sort/FieldSortBuilderTests.java index 9b8cd1b5f1ce0..546b870811b9e 100644 --- a/server/src/test/java/org/opensearch/search/sort/FieldSortBuilderTests.java +++ b/server/src/test/java/org/opensearch/search/sort/FieldSortBuilderTests.java @@ -87,7 +87,7 @@ import java.util.Locale; import java.util.Set; -import static org.opensearch.search.sort.FieldSortBuilder.getMinMaxOrNull; +import static org.opensearch.search.sort.FieldSortBuilder.getFieldStatsForShard; import static org.opensearch.search.sort.FieldSortBuilder.getPrimaryFieldSortOrNull; import static org.opensearch.search.sort.NestedSortBuilderTests.createRandomNestedSort; import static org.hamcrest.Matchers.instanceOf; @@ -488,8 +488,8 @@ public void testGetMaxNumericSortValue() throws IOException { QueryShardContext context = createMockShardContext(); for (NumberFieldMapper.NumberType numberType : NumberFieldMapper.NumberType.values()) { String fieldName = "custom-" + numberType.numericType(); - assertNull(getMinMaxOrNull(context, SortBuilders.fieldSort(fieldName))); - assertNull(getMinMaxOrNull(context, SortBuilders.fieldSort(fieldName + "-ni"))); + assertEquals(FieldStats.UNKNOWN, getFieldStatsForShard(context, SortBuilders.fieldSort(fieldName))); + assertEquals(FieldStats.UNKNOWN, getFieldStatsForShard(context, SortBuilders.fieldSort(fieldName + "-ni"))); try (Directory dir = newDirectory()) { int numDocs = randomIntBetween(10, 30); @@ -556,12 +556,18 @@ public void testGetMaxNumericSortValue() throws IOException { QueryShardContext newContext = createMockShardContext(new AssertingIndexSearcher(random(), reader)); if (numberType == NumberFieldMapper.NumberType.HALF_FLOAT || numberType == NumberFieldMapper.NumberType.UNSIGNED_LONG) { - assertNull(getMinMaxOrNull(newContext, SortBuilders.fieldSort(fieldName + "-ni"))); - assertNull(getMinMaxOrNull(newContext, SortBuilders.fieldSort(fieldName))); + assertEquals(FieldStats.UNKNOWN, getFieldStatsForShard(newContext, SortBuilders.fieldSort(fieldName + "-ni"))); + assertEquals(FieldStats.UNKNOWN, getFieldStatsForShard(newContext, SortBuilders.fieldSort(fieldName))); } else { - assertNull(getMinMaxOrNull(newContext, SortBuilders.fieldSort(fieldName + "-ni"))); - assertEquals(values[numDocs - 1], getMinMaxOrNull(newContext, SortBuilders.fieldSort(fieldName)).getMax()); - assertEquals(values[0], getMinMaxOrNull(newContext, SortBuilders.fieldSort(fieldName)).getMin()); + assertEquals(FieldStats.UNKNOWN, getFieldStatsForShard(newContext, SortBuilders.fieldSort(fieldName + "-ni"))); + assertEquals( + values[numDocs - 1], + getFieldStatsForShard(newContext, SortBuilders.fieldSort(fieldName)).getMinAndMax().getMax() + ); + assertEquals( + values[0], + getFieldStatsForShard(newContext, SortBuilders.fieldSort(fieldName)).getMinAndMax().getMin() + ); } } } @@ -572,8 +578,8 @@ public void testGetMaxNumericSortValue() throws IOException { public void testGetMaxNumericDateValue() throws IOException { QueryShardContext context = createMockShardContext(); String fieldName = "custom-date"; - assertNull(getMinMaxOrNull(context, SortBuilders.fieldSort(fieldName))); - assertNull(getMinMaxOrNull(context, SortBuilders.fieldSort(fieldName + "-ni"))); + assertEquals(FieldStats.UNKNOWN, getFieldStatsForShard(context, SortBuilders.fieldSort(fieldName))); + assertEquals(FieldStats.UNKNOWN, getFieldStatsForShard(context, SortBuilders.fieldSort(fieldName + "-ni"))); try (Directory dir = newDirectory()) { int numDocs = randomIntBetween(10, 30); final long[] values = new long[numDocs]; @@ -587,8 +593,11 @@ public void testGetMaxNumericDateValue() throws IOException { Arrays.sort(values); try (DirectoryReader reader = writer.getReader()) { QueryShardContext newContext = createMockShardContext(new AssertingIndexSearcher(random(), reader)); - assertEquals(values[numDocs - 1], getMinMaxOrNull(newContext, SortBuilders.fieldSort(fieldName)).getMax()); - assertEquals(values[0], getMinMaxOrNull(newContext, SortBuilders.fieldSort(fieldName)).getMin()); + assertEquals( + values[numDocs - 1], + getFieldStatsForShard(newContext, SortBuilders.fieldSort(fieldName)).getMinAndMax().getMax() + ); + assertEquals(values[0], getFieldStatsForShard(newContext, SortBuilders.fieldSort(fieldName)).getMinAndMax().getMin()); } } } @@ -597,8 +606,8 @@ public void testGetMaxNumericDateValue() throws IOException { public void testGetMaxKeywordValue() throws IOException { QueryShardContext context = createMockShardContext(); String fieldName = "custom-keyword"; - assertNull(getMinMaxOrNull(context, SortBuilders.fieldSort(fieldName))); - assertNull(getMinMaxOrNull(context, SortBuilders.fieldSort(fieldName + "-ni"))); + assertEquals(FieldStats.UNKNOWN, getFieldStatsForShard(context, SortBuilders.fieldSort(fieldName))); + assertEquals(FieldStats.UNKNOWN, getFieldStatsForShard(context, SortBuilders.fieldSort(fieldName + "-ni"))); try (Directory dir = newDirectory()) { int numDocs = randomIntBetween(10, 30); final BytesRef[] values = new BytesRef[numDocs]; @@ -612,8 +621,11 @@ public void testGetMaxKeywordValue() throws IOException { Arrays.sort(values); try (DirectoryReader reader = writer.getReader()) { QueryShardContext newContext = createMockShardContext(new AssertingIndexSearcher(random(), reader)); - assertEquals(values[numDocs - 1], getMinMaxOrNull(newContext, SortBuilders.fieldSort(fieldName)).getMax()); - assertEquals(values[0], getMinMaxOrNull(newContext, SortBuilders.fieldSort(fieldName)).getMin()); + assertEquals( + values[numDocs - 1], + getFieldStatsForShard(newContext, SortBuilders.fieldSort(fieldName)).getMinAndMax().getMax() + ); + assertEquals(values[0], getFieldStatsForShard(newContext, SortBuilders.fieldSort(fieldName)).getMinAndMax().getMin()); } } }