[Backport 2.x] Optimize UnsignedLong range queries to convert to MatchNoDocsQuery when lower > upper bounds #14483
Conversation
Skyring100
requested review from
anasalkouz,
andrross,
Bukhtawar,
CEHENKLE,
dblock,
dbwiddis,
gbbafna,
jed326,
kotwanikunal,
mch2,
msfroh,
nknize,
owaiskazi19,
peternied,
reta,
Rishikesh1159,
sachinpkale,
saratvemulapalli,
shwetathareja,
sohami and
VachaShah
as code owners
github-actions
bot
added
enhancement
|
@Skyring100 looks like something went wrong here. Can you make sure the PR targets the |
|
❌ Gradle check result for 8148e4d: 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? |
|
@jed326 when I try to switch the branch I'm committing to '2.x', GitHub claims there are no new commits between it and my own branch. Was there a different branch I am supposed to commit to? |
|
@Skyring100 looking at the branch this PR is opened from, https://github.com/Skyring100/OpenSearch/commits/backport/backport-14416-to-2.x/, I don't see the commit you are trying to backport on the branch. Based on your comment #14416 (comment) it looks like the cherry-pick didn't go through for some reason. My guess is that the commit was squashed and then wasn't fetched to your fork yet, there should be an option in the GitHub UI on https://github.com/Skyring100/OpenSearch/tree/main to sync your fork. Can you make sure that the commit d2c08b3 is present on the |
jed326
changed the title
|
@Skyring100 here are the steps that can help resolving the problem in this PR:
note that |
|
@gaobinlong I followed these steps but I did realized I didn't use the "signoff" flag for some of the last commits, is there a fast way to fix this? |
|
❕ Gradle check result for d8397c1: 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. |
The commit in this PR should be merge into the 2.x branch, you chose the wrong branch which is |
reta
force-pushed
the
backport/backport-14416-to-2.x
branch
from
d8397c1 to
84a4a3b
Compare
…en lower > upper bounds (opensearch-project#14416) * Added check for lower > upper at end of function Signed-off-by: Skyring100 <[email protected]> * Fixed mistake of using < operator on BigInteger, now using compareTo Signed-off-by: Skyring100 <[email protected]> * Fixed simple mistake of flipping > operator Signed-off-by: Skyring100 <[email protected]> * Fixed space formatting Signed-off-by: Skyring100 <[email protected]> * Updated CHANGELOG.md Signed-off-by: Skyring100 <[email protected]> * Issue number linked in CHANGELOG.md Signed-off-by: Skyring100 <[email protected]> * doTestDocValueRangeQueries now accepts MatchNoDocsQuery alongside IndexOrDocValuesQuery Signed-off-by: Skyring100 <[email protected]> * dotestdoTestDocValueRangeQueries only checks indexQuery and randomAccessQuery only when query is type IndexIndexOrDocValuesQuery Signed-off-by: Skyring100 <[email protected]> * Ran gradlew spotlessApply to fix import formatting issues Signed-off-by: Skyring100 <[email protected]> * Imported Matchers.either method instead of entire Matchers class Signed-off-by: Skyring100 <[email protected]> --------- Signed-off-by: Skyring100 <[email protected]> Signed-off-by: Andriy Redko <[email protected]> Co-authored-by: Andriy Redko <[email protected]> (cherry picked from commit d2c08b3) Signed-off-by: Andriy Redko <[email protected]>
reta
force-pushed
the
backport/backport-14416-to-2.x
branch
from
84a4a3b to
98ae9ce
Compare
|
@Skyring100 fixed the pull request, thanks for help @gaobinlong & @jed326 ! |
|
❌ Gradle check result for 84a4a3b: 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 98ae9ce: null 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 98ae9ce: 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 98ae9ce: 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 98ae9ce: 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? |
|
Thank you all for helping me out so much, it's very much appreciated |
…en lower > upper bounds (opensearch-project#14416) (opensearch-project#14483) * Added check for lower > upper at end of function * Fixed mistake of using < operator on BigInteger, now using compareTo * Fixed simple mistake of flipping > operator * Fixed space formatting * Updated CHANGELOG.md * Issue number linked in CHANGELOG.md * doTestDocValueRangeQueries now accepts MatchNoDocsQuery alongside IndexOrDocValuesQuery * dotestdoTestDocValueRangeQueries only checks indexQuery and randomAccessQuery only when query is type IndexIndexOrDocValuesQuery * Ran gradlew spotlessApply to fix import formatting issues * Imported Matchers.either method instead of entire Matchers class --------- (cherry picked from commit d2c08b3) Signed-off-by: Skyring100 <[email protected]> Signed-off-by: Andriy Redko <[email protected]> Co-authored-by: Andriy Redko <[email protected]> Signed-off-by: kkewwei <[email protected]>
Description
Backport #14416 to 2.x
unsignedLongRangeQuery method now returns MatchNoDocsQuery when lower bound > upper bound
Related Issues
Resolves #14404
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.