-
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
Remove ApproximateIndexOrDocValuesQuery #16273
Remove ApproximateIndexOrDocValuesQuery #16273
Conversation
3078190
to
dc2c63a
Compare
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.
Nice cleanup, thanks @msfroh !
❌ Gradle check result for 21c5400: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
❌ Gradle check result for 21c5400: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
Looking into the query approximation framework, I realized that we don't actually need ApproximateIndexOrDocValuesQuery. We already have ApproximateScoreQuery that is able to choose between the approximate query and the original query. Assuming the range query is over an indexed field, it is (potentially) possible to approximate. If there are doc values, then the original query will be an IndexOrDocValuesQuery. Otherwise, it will just be a PointRangeQuery. Either way, we can wrap the original query into a generic ApproximateScoreQuery along with the approximate query. The approximation logic doesn't need to be specialized to the IndexOrDocValue case. Signed-off-by: Michael Froh <froh@amazon.com>
Signed-off-by: Michael Froh <froh@amazon.com>
Signed-off-by: Michael Froh <froh@amazon.com>
Using IndexSortSortedNumericDocValuesRangeQuery should take precedence over IndexOrDocValuesQuery. My previous commit accidentally put it under the IndexOrDocValuesQuery (wrapped around the dvQuery). Signed-off-by: Michael Froh <froh@amazon.com>
21c5400
to
207ffa7
Compare
❕ Gradle check result for 207ffa7: 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: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #16273 +/- ##
============================================
- Coverage 72.06% 71.97% -0.10%
+ Complexity 64822 64739 -83
============================================
Files 5308 5307 -1
Lines 302574 302540 -34
Branches 43710 43708 -2
============================================
- Hits 218048 217746 -302
- Misses 66648 66905 +257
- Partials 17878 17889 +11 ☔ View full report in Codecov by Sentry. |
server/src/main/java/org/opensearch/search/approximate/ApproximateScoreQuery.java
Show resolved
Hide resolved
Looking into the query approximation framework, I realized that we don't actually need ApproximateIndexOrDocValuesQuery. We already have ApproximateScoreQuery that is able to choose between the approximate query and the original query. Assuming the range query is over an indexed field, it is (potentially) possible to approximate. If there are doc values, then the original query will be an IndexOrDocValuesQuery. Otherwise, it will just be a PointRangeQuery. Either way, we can wrap the original query into a generic ApproximateScoreQuery along with the approximate query. The approximation logic doesn't need to be specialized to the IndexOrDocValue case. --------- Signed-off-by: Michael Froh <froh@amazon.com> (cherry picked from commit 53c9ddf) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Looking into the query approximation framework, I realized that we don't actually need ApproximateIndexOrDocValuesQuery. We already have ApproximateScoreQuery that is able to choose between the approximate query and the original query. Assuming the range query is over an indexed field, it is (potentially) possible to approximate. If there are doc values, then the original query will be an IndexOrDocValuesQuery. Otherwise, it will just be a PointRangeQuery. Either way, we can wrap the original query into a generic ApproximateScoreQuery along with the approximate query. The approximation logic doesn't need to be specialized to the IndexOrDocValue case. --------- (cherry picked from commit 53c9ddf) Signed-off-by: Michael Froh <froh@amazon.com> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Looking into the query approximation framework, I realized that we don't actually need ApproximateIndexOrDocValuesQuery. We already have ApproximateScoreQuery that is able to choose between the approximate query and the original query. Assuming the range query is over an indexed field, it is (potentially) possible to approximate. If there are doc values, then the original query will be an IndexOrDocValuesQuery. Otherwise, it will just be a PointRangeQuery. Either way, we can wrap the original query into a generic ApproximateScoreQuery along with the approximate query. The approximation logic doesn't need to be specialized to the IndexOrDocValue case. --------- Signed-off-by: Michael Froh <froh@amazon.com>
Looking into the query approximation framework, I realized that we don't actually need ApproximateIndexOrDocValuesQuery. We already have ApproximateScoreQuery that is able to choose between the approximate query and the original query. Assuming the range query is over an indexed field, it is (potentially) possible to approximate. If there are doc values, then the original query will be an IndexOrDocValuesQuery. Otherwise, it will just be a PointRangeQuery. Either way, we can wrap the original query into a generic ApproximateScoreQuery along with the approximate query. The approximation logic doesn't need to be specialized to the IndexOrDocValue case. --------- Signed-off-by: Michael Froh <froh@amazon.com>
Looking into the query approximation framework, I realized that we don't actually need ApproximateIndexOrDocValuesQuery. We already have ApproximateScoreQuery that is able to choose between the approximate query and the original query. Assuming the range query is over an indexed field, it is (potentially) possible to approximate. If there are doc values, then the original query will be an IndexOrDocValuesQuery. Otherwise, it will just be a PointRangeQuery. Either way, we can wrap the original query into a generic ApproximateScoreQuery along with the approximate query. The approximation logic doesn't need to be specialized to the IndexOrDocValue case. --------- Signed-off-by: Michael Froh <froh@amazon.com>
Description
Looking into the query approximation framework, I realized that we don't actually need ApproximateIndexOrDocValuesQuery. We already have ApproximateScoreQuery that is able to choose between the approximate query and the original query.
Assuming the range query is over an indexed field, it is (potentially) possible to approximate. If there are doc values, then the original query will be an IndexOrDocValuesQuery. Otherwise, it will just be a PointRangeQuery. Either way, we can wrap the original query into a generic ApproximateScoreQuery along with the approximate query. The approximation logic doesn't need to be specialized to the IndexOrDocValue case.
Related Issues
N/A
Check List
API changes companion pull request created, if applicable.Public documentation issue/PR created, if applicable.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.