-
Notifications
You must be signed in to change notification settings - Fork 72
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixed exception in Hybrid Query for one shard and multiple node #396
Fixed exception in Hybrid Query for one shard and multiple node #396
Conversation
ec4ca62
to
10b4c00
Compare
Codecov Report
@@ Coverage Diff @@
## main #396 +/- ##
============================================
- Coverage 84.63% 84.56% -0.07%
- Complexity 445 449 +4
============================================
Files 38 38
Lines 1334 1348 +14
Branches 199 201 +2
============================================
+ Hits 1129 1140 +11
- Misses 119 120 +1
- Partials 86 88 +2
|
Signed-off-by: Martin Gaievski <[email protected]>
10b4c00
to
b2d6c38
Compare
@@ -179,4 +179,117 @@ public void testFetchResults_whenOneShardAndQueryAndFetchResultsPresent_thenDoNo | |||
TestUtils.assertQueryResultScores(querySearchResults); | |||
TestUtils.assertFetchResultScores(fetchSearchResult, 4); | |||
} | |||
|
|||
public void testFetchResults_whenOneShardAndMultipleNodes_thenDoNormalizationCombination() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this test case would fail with old code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for adding a negative case.
src/main/java/org/opensearch/neuralsearch/processor/NormalizationProcessorWorkflow.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/neuralsearch/processor/NormalizationProcessorWorkflow.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Martin Gaievski <[email protected]>
8e98167
into
opensearch-project:main
* Use list of original doc ids for fetch results Signed-off-by: Martin Gaievski <[email protected]> (cherry picked from commit 8e98167)
* Use list of original doc ids for fetch results Signed-off-by: Martin Gaievski <[email protected]> (cherry picked from commit 8e98167)
#400) * Use list of original doc ids for fetch results Signed-off-by: Martin Gaievski <[email protected]> (cherry picked from commit 8e98167) Co-authored-by: Martin Gaievski <[email protected]>
#399) * Use list of original doc ids for fetch results Signed-off-by: Martin Gaievski <[email protected]> (cherry picked from commit 8e98167) Co-authored-by: Martin Gaievski <[email protected]>
Description
Fixed case when there is one shard and multiple nodes. In such case Fetch phase all results have doc ids as "-1", and it's not possible to merge them with query results using doc id. Result is a runtime exception in search request.
To fix te problem we're storing doc ids before the normalization and combination is done, and them use docs ids to merge with fetch phase results.
Issues Resolved
#393
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.