-
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 document source and score field mismatch in sorted hybrid queries #1043
base: main
Are you sure you want to change the base?
Fixed document source and score field mismatch in sorted hybrid queries #1043
Conversation
…is enabled in hybrid query Signed-off-by: Martin Gaievski <[email protected]>
Fixed mismatch between document source and score fields when sorting is enabled in hybrid query Signed-off-by: Martin Gaievski <[email protected]>
093f199
to
3559f12
Compare
private FieldValueHitQueue.Entry bottom; | ||
@Getter(AccessLevel.PACKAGE) | ||
@VisibleForTesting | ||
private FieldValueHitQueue.Entry fieldValueLeafTrackers[]; |
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.
private FieldValueHitQueue.Entry fieldValueLeafTrackers[]; | |
private FieldValueHitQueue.Entry sortFieldValueTrackers[]; |
@@ -254,6 +259,9 @@ protected void initializePriorityQueuesWithComparators(LeafReaderContext context | |||
initializeLeafFieldComparators(context, i); | |||
} | |||
} | |||
if (Objects.isNull(fieldValueLeafTrackers)) { |
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.
QQ: Do you want this code to be executed
a. Once per shard?
b. Once per segment?
c. Once per entire search flow?
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.
I think this code can be shifted under above if(compoundScores==null) condition.
Fixed document source and score field mismatch in sorted hybrid queries.
Returned search hits identified with the help of min heap, it's used by sorting functionality to get top X docs. We do keep track of the heap leaf element and updating it when collecting doc ids. In current code we use only one element for this, but in case of hybrid query we do need separate element for each sub-query. With a single element our updates are incorrectly propagated to a results of different sub-queries, cause different types of inconsistency: doc id, score fields, score can be incorrect in final search result.
In this PR I'm changing the min heap leaf element from a single object to an array of objects, one for each sub-query.
Tested on the data set referred in the issue, got correct response where all field have consistent values in
_source
andsort
sections:Related Issues
#1044
Check List
[ ] New functionality has been documented.[ ] API changes companion pull request created.--signoff
.~~[ ] Public documentation issue/PR created.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.