-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Fix the rewrite method for MatchOnlyText field query #14154
Fix the rewrite method for MatchOnlyText field query #14154
Conversation
e911b4b
to
f31b3b0
Compare
❌ Gradle check result for e911b4b: 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? |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #14154 +/- ##
============================================
+ Coverage 71.42% 71.67% +0.25%
- Complexity 59978 62012 +2034
============================================
Files 4985 5117 +132
Lines 282275 291686 +9411
Branches 40946 42166 +1220
============================================
+ Hits 201603 209059 +7456
- Misses 63999 65356 +1357
- Partials 16673 17271 +598 ☔ View full report in Codecov by Sentry. |
server/src/main/java/org/opensearch/index/query/SourceFieldMatchQuery.java
Show resolved
Hide resolved
❌ Gradle check result for ffd19d0: 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? |
Signed-off-by: Rishabh Maurya <rishabhmaurya05@gmail.com>
ffd19d0
to
0fe4f72
Compare
❌ Gradle check result for 0fe4f72: 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 0fe4f72: 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? |
hitting another flaky test - #5329 |
No merges with failing checks please, @rishabhmaurya could you please add entry to https://github.com/opensearch-project/OpenSearch/blob/main/release-notes/opensearch.release-notes-2.15.0.md? (we could do it in separate pull request if checks are green for this one) |
❕ Gradle check result for 0fe4f72: 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. |
Signed-off-by: Rishabh Maurya <rishabhmaurya05@gmail.com> (cherry picked from commit 435af89) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
#14251 to add this change to 2.15 release notes. |
(cherry picked from commit 435af89) Signed-off-by: Rishabh Maurya <rishabhmaurya05@gmail.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>
…ect#14154) Signed-off-by: Rishabh Maurya <rishabhmaurya05@gmail.com>
…ect#14154) (opensearch-project#14250) (cherry picked from commit 435af89) Signed-off-by: Rishabh Maurya <rishabhmaurya05@gmail.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> Signed-off-by: kkewwei <kkewwei@163.com>
…ect#14154) Signed-off-by: Rishabh Maurya <rishabhmaurya05@gmail.com>
Description
Using
indexSearcher.rewrite(delegateQuery)
instead ofdelegateQuery.rewrite(indexSearcher);
can have side effects whenprofile
is set to true, as it can start reusing the profile objects created in parent query and can interfere with timers if they were already started. Whereas,query.rewrite(indexSearcher)
it just uses the lucene APIs and doesn't use the wrapped functionality in OpenSearch.In query phase, when rewrite is called
OpenSearch/server/src/main/java/org/opensearch/search/query/QueryPhase.java
Line 127 in 5ff50a7
the
ContextIndexSearch
will start the timer for the query and will start noting therewrite()
time for all clauses in the clauses for the complete query tree.But if we try to use
indexSearcher.rewrite(delegateQuery)
within rewrite of any of the query, it will try to initialize the profiler and timer again and will result in the assertion error here -OpenSearch/server/src/main/java/org/opensearch/search/profile/query/AbstractQueryProfileTree.java
Line 48 in 200ad5d
Related Issues
Fixes #14156
Check List
[ ] Functionality includes testing.There are already unit and integ tests present formatch_only_text
field.[ ] 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.