Skip to content

Commit c6b8ac4

Browse files
authored
Fixed single shard pagination issue of from (#1140)
Signed-off-by: Owais <[email protected]>
1 parent 8c743ec commit c6b8ac4

File tree

3 files changed

+78
-1
lines changed

3 files changed

+78
-1
lines changed

CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
3333
- Fix bug where embedding is missing when ingested document has "." in field name, and mismatches fieldMap config ([#1062](https://github.com/opensearch-project/neural-search/pull/1062))
3434
- Fix explain exception in hybrid queries with partial subquery matches ([#1123](https://github.com/opensearch-project/neural-search/pull/1123))
3535
- Handle pagination_depth when from =0 and removes default value of pagination_depth ([#1132](https://github.com/opensearch-project/neural-search/pull/1132))
36+
- Fix single shard pagination issue of from ([#1140](https://github.com/opensearch-project/neural-search/pull/1140))
3637
### Infrastructure
3738
### Documentation
3839
### Maintenance

src/main/java/org/opensearch/neuralsearch/processor/NormalizationProcessorWorkflow.java

+10-1
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,16 @@ private int getFromValueIfSingleShard(final NormalizationProcessorWorkflowExecut
113113
if (searchPhaseContext.getNumShards() > 1 || request.fetchSearchResultOptional.isEmpty()) {
114114
return -1;
115115
}
116-
return searchPhaseContext.getRequest().source().from();
116+
int from = searchPhaseContext.getRequest().source().from();
117+
// for the initial searchRequest, it creates a default search context which sets the value of
118+
// from to 0 if it's -1. That's not the case with SearchPhaseContext, that's why need to
119+
// explicitly set to 0 for the single shard case
120+
// Ref:
121+
// https://github.com/opensearch-project/OpenSearch/blob/2.18/server/src/main/java/org/opensearch/search/DefaultSearchContext.java#L288
122+
if (from == -1) {
123+
return 0;
124+
}
125+
return from;
117126
}
118127

119128
/**

src/test/java/org/opensearch/neuralsearch/processor/NormalizationProcessorWorkflowTests.java

+67
Original file line numberDiff line numberDiff line change
@@ -398,6 +398,73 @@ public void testFetchResultsAndCache_whenOneShardAndMultipleNodesAndMismatchResu
398398
TestUtils.assertFetchResultScores(fetchSearchResult, 4);
399399
}
400400

401+
public void testNormalization_whenOneShardAndFromIsNegativeOne_thenSuccess() {
402+
NormalizationProcessorWorkflow normalizationProcessorWorkflow = spy(
403+
new NormalizationProcessorWorkflow(new ScoreNormalizer(), new ScoreCombiner())
404+
);
405+
406+
int shardId = 0;
407+
SearchShardTarget searchShardTarget = new SearchShardTarget(
408+
"node",
409+
new ShardId("index", "uuid", shardId),
410+
null,
411+
OriginalIndices.NONE
412+
);
413+
414+
// Setup query search results
415+
List<QuerySearchResult> querySearchResults = new ArrayList<>();
416+
FetchSearchResult fetchSearchResult = new FetchSearchResult();
417+
QuerySearchResult querySearchResult = new QuerySearchResult();
418+
querySearchResult.topDocs(
419+
new TopDocsAndMaxScore(
420+
new TopDocs(
421+
new TotalHits(4, TotalHits.Relation.EQUAL_TO),
422+
new ScoreDoc[] {
423+
createStartStopElementForHybridSearchResults(0),
424+
createDelimiterElementForHybridSearchResults(0),
425+
new ScoreDoc(0, 0.5f),
426+
new ScoreDoc(2, 0.3f),
427+
new ScoreDoc(4, 0.25f),
428+
new ScoreDoc(10, 0.2f),
429+
createStartStopElementForHybridSearchResults(0) }
430+
),
431+
0.5f
432+
),
433+
null
434+
);
435+
querySearchResult.setSearchShardTarget(searchShardTarget);
436+
querySearchResult.setShardIndex(shardId);
437+
ShardSearchRequest shardSearchRequest = mock(ShardSearchRequest.class);
438+
when(shardSearchRequest.requestCache()).thenReturn(Boolean.TRUE);
439+
querySearchResult.setShardSearchRequest(shardSearchRequest);
440+
querySearchResults.add(querySearchResult);
441+
SearchHits searchHits = getSearchHits();
442+
fetchSearchResult.hits(searchHits);
443+
SearchPhaseContext searchPhaseContext = mock(SearchPhaseContext.class);
444+
SearchRequest searchRequest = mock(SearchRequest.class);
445+
SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder();
446+
// searchSourceBuilder.from(); if no from is defined here it would initialize it to -1
447+
when(searchPhaseContext.getRequest()).thenReturn(searchRequest);
448+
when(searchRequest.source()).thenReturn(searchSourceBuilder);
449+
when(searchPhaseContext.getNumShards()).thenReturn(1);
450+
NormalizationProcessorWorkflowExecuteRequest normalizationExecuteDTO = NormalizationProcessorWorkflowExecuteRequest.builder()
451+
.querySearchResults(querySearchResults)
452+
.fetchSearchResultOptional(Optional.of(fetchSearchResult))
453+
.normalizationTechnique(ScoreNormalizationFactory.DEFAULT_METHOD)
454+
.combinationTechnique(ScoreCombinationFactory.DEFAULT_METHOD)
455+
.searchPhaseContext(searchPhaseContext)
456+
.build();
457+
458+
// Setup fetch search result
459+
fetchSearchResult.hits(searchHits);
460+
461+
normalizationProcessorWorkflow.execute(normalizationExecuteDTO);
462+
463+
// Verify that the fetch result has been updated correctly
464+
TestUtils.assertQueryResultScores(querySearchResults);
465+
TestUtils.assertFetchResultScores(fetchSearchResult, 4);
466+
}
467+
401468
public void testNormalization_whenFromIsGreaterThanResultsSize_thenFail() {
402469
NormalizationProcessorWorkflow normalizationProcessorWorkflow = spy(
403470
new NormalizationProcessorWorkflow(new ScoreNormalizer(), new ScoreCombiner())

0 commit comments

Comments
 (0)