Skip to content

Commit b8bbcc7

Browse files
bugmakerrrrrrbugmakerrrrrr
committed
search_after query optimization with missing value comparison
Signed-off-by: panguixin <panguixin@bytedance.com>
1 parent a2cef8f commit b8bbcc7

5 files changed

Lines changed: 172 additions & 53 deletions

File tree

‎rest-api-spec/src/main/resources/rest-api-spec/test/search/90_search_after.yml‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -785,3 +785,61 @@
785785
- match: { hits.hits.2._index: test }
786786
- match: { hits.hits.2._source.unsignedlong: null }
787787
- match: { hits.hits.2._source.id: 22 }
788+
789+
---
790+
"search with null search after value":
791+
- do:
792+
indices.create:
793+
index: test
794+
body:
795+
mappings:
796+
properties:
797+
doc:
798+
type: keyword
799+
name:
800+
type: keyword
801+
802+
- do:
803+
bulk:
804+
refresh: true
805+
index: test
806+
body: |
807+
{"index":{}}
808+
{ "doc": "doc1", "name": "bob"}
809+
{"index":{}}
810+
{ "doc": "doc2", "name": null}
811+
{"index":{}}
812+
{ "doc": "doc3", "name": null}
813+
814+
- do:
815+
search:
816+
rest_total_hits_as_int: true
817+
index: test
818+
body:
819+
size: 2
820+
track_total_hits: false
821+
sort: [{ name: desc }, { doc: desc }]
822+
823+
- match: {hits.total: -1 }
824+
- length: {hits.hits: 2 }
825+
- match: {hits.hits.0._index: test }
826+
- match: {hits.hits.0._source.doc: doc1 }
827+
- match: {hits.hits.1._index: test }
828+
- match: {hits.hits.1._source.doc: doc3 }
829+
- match: {hits.hits.1.sort: [null, "doc3"] }
830+
831+
- do:
832+
search:
833+
rest_total_hits_as_int: true
834+
index: test
835+
body:
836+
size: 1
837+
track_total_hits: false
838+
sort: [{ name: desc }, { doc: desc }]
839+
search_after: [null, "doc3"]
840+
841+
- match: {hits.total: -1 }
842+
- length: {hits.hits: 1 }
843+
- match: {hits.hits.0._index: test }
844+
- match: {hits.hits.0._source.doc: doc2 }
845+
- match: {hits.hits.0.sort: [null, "doc2"] }

‎server/src/main/java/org/opensearch/search/SearchService.java‎

Lines changed: 44 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,9 @@
3636
import org.apache.logging.log4j.Logger;
3737
import org.apache.lucene.search.FieldDoc;
3838
import org.apache.lucene.search.IndexSearcher;
39+
import org.apache.lucene.search.SortField;
3940
import org.apache.lucene.search.TopDocs;
41+
import org.apache.lucene.util.BytesRef;
4042
import org.opensearch.OpenSearchException;
4143
import org.opensearch.action.ActionRunnable;
4244
import org.opensearch.action.OriginalIndices;
@@ -135,7 +137,6 @@
135137
import org.opensearch.search.sort.MinAndMax;
136138
import org.opensearch.search.sort.SortAndFormats;
137139
import org.opensearch.search.sort.SortBuilder;
138-
import org.opensearch.search.sort.SortOrder;
139140
import org.opensearch.search.suggest.Suggest;
140141
import org.opensearch.search.suggest.completion.CompletionSuggestion;
141142
import org.opensearch.tasks.TaskResourceTrackingService;
@@ -1622,7 +1623,10 @@ private CanMatchResponse canMatch(ShardSearchRequest request, boolean checkRefre
16221623
);
16231624
Rewriteable.rewrite(request.getRewriteable(), context, false);
16241625
final boolean aliasFilterCanMatch = request.getAliasFilter().getQueryBuilder() instanceof MatchNoneQueryBuilder == false;
1625-
FieldSortBuilder sortBuilder = FieldSortBuilder.getPrimaryFieldSortOrNull(request.source());
1626+
final FieldSortBuilder sortBuilder = FieldSortBuilder.getPrimaryFieldSortOrNull(request.source());
1627+
final SortAndFormats primarySort = sortBuilder != null
1628+
? SortBuilder.buildSort(Collections.singletonList(sortBuilder), context).get()
1629+
: null;
16261630
MinAndMax<?> minMax = sortBuilder != null ? FieldSortBuilder.getMinMaxOrNull(context, sortBuilder) : null;
16271631
boolean canMatch;
16281632
if (canRewriteToMatchNone(request.source())) {
@@ -1634,7 +1638,7 @@ private CanMatchResponse canMatch(ShardSearchRequest request, boolean checkRefre
16341638
}
16351639
final FieldDoc searchAfterFieldDoc = getSearchAfterFieldDoc(request, context);
16361640
final Integer trackTotalHitsUpto = request.source() == null ? null : request.source().trackTotalHitsUpTo();
1637-
canMatch = canMatch && canMatchSearchAfter(searchAfterFieldDoc, minMax, sortBuilder, trackTotalHitsUpto);
1641+
canMatch = canMatch && canMatchSearchAfter(searchAfterFieldDoc, minMax, primarySort, trackTotalHitsUpto);
16381642

16391643
return new CanMatchResponse(canMatch || hasRefreshPending, minMax);
16401644
}
@@ -1644,34 +1648,65 @@ private CanMatchResponse canMatch(ShardSearchRequest request, boolean checkRefre
16441648
public static boolean canMatchSearchAfter(
16451649
FieldDoc searchAfter,
16461650
MinAndMax<?> minMax,
1647-
FieldSortBuilder primarySortField,
1651+
SortAndFormats primarySort,
16481652
Integer trackTotalHitsUpto
16491653
) {
16501654
// Check for sort.missing == null, since in case of missing values sort queries, if segment/shard's min/max
16511655
// is out of search_after range, it still should be printed and hence we should not skip segment/shard.
16521656
// Skipping search on shard/segment entirely can cause mismatch on total_tracking_hits, hence skip only if
16531657
// track_total_hits is false.
16541658
if (searchAfter != null
1659+
&& searchAfter.fields[0] != null
16551660
&& minMax != null
1656-
&& primarySortField != null
1657-
&& primarySortField.missing() == null
1661+
&& primarySort != null
16581662
&& Objects.equals(trackTotalHitsUpto, SearchContext.TRACK_TOTAL_HITS_DISABLED)) {
16591663
final Object searchAfterPrimary = searchAfter.fields[0];
1660-
if (primarySortField.order() == SortOrder.DESC) {
1664+
SortField primarySortField = primarySort.sort.getSort()[0];
1665+
if (primarySortField.getReverse()) {
16611666
if (minMax.compareMin(searchAfterPrimary) > 0) {
16621667
// In Desc order, if segment/shard minimum is gt search_after, the segment/shard won't be competitive
1663-
return false;
1668+
return canMatchMissingValue(primarySortField, searchAfterPrimary);
16641669
}
16651670
} else {
16661671
if (minMax.compareMax(searchAfterPrimary) < 0) {
16671672
// In ASC order, if segment/shard maximum is lt search_after, the segment/shard won't be competitive
1668-
return false;
1673+
return canMatchMissingValue(primarySortField, searchAfterPrimary);
16691674
}
16701675
}
16711676
}
16721677
return true;
16731678
}
16741679

1680+
private static boolean canMatchMissingValue(SortField primarySortField, Object primarySearchAfter) {
1681+
final Object missingValue = primarySortField.getMissingValue();
1682+
if (primarySortField.getReverse()) {
1683+
// the missing value of Type.STRING can only be SortField.STRING_FIRS or SortField.STRING_LAST
1684+
if (primarySearchAfter instanceof BytesRef) {
1685+
return missingValue == SortField.STRING_FIRST;
1686+
}
1687+
return compare(primarySearchAfter, missingValue) >= 0;
1688+
} else {
1689+
if (primarySearchAfter instanceof BytesRef) {
1690+
return missingValue == SortField.STRING_LAST;
1691+
}
1692+
return compare(primarySearchAfter, missingValue) <= 0;
1693+
}
1694+
}
1695+
1696+
private static int compare(Object one, Object two) {
1697+
if (one instanceof Long) {
1698+
return Long.compare((Long) one, (Long) two);
1699+
} else if (one instanceof Integer) {
1700+
return Integer.compare((Integer) one, (Integer) two);
1701+
} else if (one instanceof Float) {
1702+
return Float.compare((Float) one, (Float) two);
1703+
} else if (one instanceof Double) {
1704+
return Double.compare((Double) one, (Double) two);
1705+
} else {
1706+
throw new UnsupportedOperationException("compare type not supported : " + one.getClass());
1707+
}
1708+
}
1709+
16751710
private static FieldDoc getSearchAfterFieldDoc(ShardSearchRequest request, QueryShardContext context) throws IOException {
16761711
if (context != null && request != null && request.source() != null && request.source().sorts() != null) {
16771712
final List<SortBuilder<?>> sorts = request.source().sorts();

‎server/src/main/java/org/opensearch/search/internal/ContextIndexSearcher.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -526,7 +526,7 @@ private boolean canMatchSearchAfter(LeafReaderContext ctx) throws IOException {
526526
return SearchService.canMatchSearchAfter(
527527
searchContext.searchAfter(),
528528
minMax,
529-
primarySortField,
529+
searchContext.sort(),
530530
searchContext.trackTotalHitsUpTo()
531531
);
532532
}

‎server/src/main/java/org/opensearch/search/searchafter/SearchAfterBuilder.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,10 @@ public static FieldDoc buildFieldDoc(SortAndFormats sort, Object[] values) {
138138
if (values[i] != null) {
139139
fieldValues[i] = convertValueFromSortField(values[i], sortField, format);
140140
} else {
141+
SortField.Type sortType = extractSortType(sortField);
142+
if (sortType != SortField.Type.STRING && sortType != SortField.Type.STRING_VAL) {
143+
throw new IllegalArgumentException("search after value of type [" + sortType + "] cannot be null");
144+
}
141145
fieldValues[i] = null;
142146
}
143147
}

0 commit comments

Comments
 (0)