-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
[Sort] Sending missing values in comparator instead sending null #11196
Conversation
Signed-off-by: Chaitanya Gohel <[email protected]>
Signed-off-by: Chaitanya Gohel <[email protected]>
@reta @backslasht @kkmr |
Compatibility status:Checks if related components are compatible with change 9b0af16 Incompatible componentsIncompatible components: [https://github.com/opensearch-project/performance-analyzer.git] Skipped componentsCompatible componentsCompatible components: [https://github.com/opensearch-project/asynchronous-search.git, https://github.com/opensearch-project/security-analytics.git, https://github.com/opensearch-project/observability.git, https://github.com/opensearch-project/reporting.git, https://github.com/opensearch-project/notifications.git, https://github.com/opensearch-project/job-scheduler.git, https://github.com/opensearch-project/neural-search.git, https://github.com/opensearch-project/common-utils.git, https://github.com/opensearch-project/opensearch-oci-object-storage.git, https://github.com/opensearch-project/custom-codecs.git, https://github.com/opensearch-project/ml-commons.git, https://github.com/opensearch-project/geospatial.git, https://github.com/opensearch-project/anomaly-detection.git, https://github.com/opensearch-project/index-management.git, https://github.com/opensearch-project/security.git, https://github.com/opensearch-project/cross-cluster-replication.git, https://github.com/opensearch-project/alerting.git, https://github.com/opensearch-project/k-nn.git, https://github.com/opensearch-project/performance-analyzer-rca.git, https://github.com/opensearch-project/sql.git] |
❕ Gradle check result for 9b0af16: UNSTABLE
Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure. |
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #11196 +/- ##
============================================
+ Coverage 71.12% 71.17% +0.04%
- Complexity 58788 58805 +17
============================================
Files 4883 4885 +2
Lines 277152 277199 +47
Branches 40281 40285 +4
============================================
+ Hits 197134 197287 +153
+ Misses 63545 63462 -83
+ Partials 16473 16450 -23 ☔ View full report in Codecov by Sentry. |
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.
Looks great, nice and clean.
Backport manually with the proper version number checks in YAML tests? |
…nsearch-project#11196) * [Sort] Sending missing values in comparator instead sending null Signed-off-by: Chaitanya Gohel <[email protected]> * Removing missing value outdated comments Signed-off-by: Chaitanya Gohel <[email protected]> --------- Signed-off-by: Chaitanya Gohel <[email protected]>
Thanks @dblock @backslasht !! |
…nsearch-project#11196) * [Sort] Sending missing values in comparator instead sending null Signed-off-by: Chaitanya Gohel <[email protected]> * Removing missing value outdated comments Signed-off-by: Chaitanya Gohel <[email protected]> --------- Signed-off-by: Chaitanya Gohel <[email protected]>
…nsearch-project#11196) * [Sort] Sending missing values in comparator instead sending null Signed-off-by: Chaitanya Gohel <[email protected]> * Removing missing value outdated comments Signed-off-by: Chaitanya Gohel <[email protected]> --------- Signed-off-by: Chaitanya Gohel <[email protected]>
…nsearch-project#11196) * [Sort] Sending missing values in comparator instead sending null Signed-off-by: Chaitanya Gohel <[email protected]> * Removing missing value outdated comments Signed-off-by: Chaitanya Gohel <[email protected]> --------- Signed-off-by: Chaitanya Gohel <[email protected]> Signed-off-by: Shivansh Arora <[email protected]>
Description
NumericComparator uses
missingValue
to determine if to apply BKD based skipping logic in casemissingValue
is not competitive. IfmissingValue
is competitive, we cant apply this optimization as per the comment.If we dont pass missing value here, it might hit the corner case like here #9537. We fixed this in Lucene, apache/lucene#12520, but didnt add integ tests here in OpenSearch.
Related Issues
#10998
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.