opensearch-project
Aggregation Array OOB Error
Description
In Ranges.java, the comparator attribute is static which means that every time a new Ranges method is created, the comparator is overridden. This leads to the OOB error because the wrong array comparator might used for the wrong array length if there are multiple aggregations.
Related Issues
Resolves #19365
Check List
- [ ] Functionality includes testing.
- [ ] 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.
Walkthrough
Refactors Ranges from static to instance methods, updates callers, adds an integration test reproducing a date-histogram + unsigned_long range aggregation scenario, and updates release notes to document the fix for an array-out-of-bounds during aggregation.
Changes
| Cohort / File(s) | Summary |
|---|---|
Test Addition client/rest-high-level/src/test/java/org/opensearch/client/SearchIT.java |
Added testSearchWithDateAndRangeAgg() with index setup for date_created (date) and distribution.number_events (unsigned_long); adjusted testCountAllIndicesNoQuery() expected count. |
Ranges Refactor server/src/main/java/org/opensearch/search/aggregations/bucket/filterrewrite/Ranges.java |
Converted comparator and utility methods (compareByteValue, withinLowerBound, withinUpperBound) from static to instance members; comparator initialized in constructor. |
Caller Update server/src/main/java/org/opensearch/search/aggregations/bucket/filterrewrite/rangecollector/AbstractRangeCollector.java |
Replaced static calls to Ranges.withinLowerBound/withinUpperBound with instance calls on the existing ranges object. |
Release Notes release-notes/opensearch.release-notes-3.4.0.md |
Fixed typos and formatting; added entry for "Fix array out of bounds during aggregation" and attached PR reference(s). |
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~20 minutes
- Review areas requiring attention:
-
Ranges.java— verify comparator initialization and thread-safety if instances are shared. - Call sites beyond
AbstractRangeCollector— search for any remaining static usages. - New test
SearchIT.testSearchWithDateAndRangeAgg()— confirm index setup and assertions match intended reproducer.
-
Possibly related PRs
- opensearch-project/OpenSearch#19573 — refactor touching the same filter-rewrite range code (Ranges and range collectors); likely directly related.
Suggested reviewers
- msfroh
- reta
- sachinpkale
- shwetathareja
- andrross
- dbwiddis
Poem
"I hopped through bytes and bounds today,
Swapped statics for instances, paved the way,
Yearly buckets bloom, ranges fall in line,
No more rogue indexes — the results are fine! 🐇"
Pre-merge checks and finishing touches
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | ⚠️ Warning | Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. | You can run @coderabbitai generate docstrings to improve docstring coverage. |
✅ Passed checks (4 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | The title 'Aggregation Array OOB Error' directly summarizes the main change: fixing an array out-of-bounds error in aggregations by converting a static comparator to instance field in Ranges.java. |
| Linked Issues check | ✅ Passed | The PR successfully addresses issue #19365 by converting the static comparator to an instance field, preventing overwrites when multiple Ranges objects are created. A new test case testSearchWithDateAndRangeAgg was added to verify the fix with date and range aggregations on unsigned_long fields. |
| Out of Scope Changes check | ✅ Passed | The PR includes release notes updates (typo fix for 'unsigned_long' and formatting) that are tangential to the core fix. However, these are minor presentation changes and reasonable within scope for a bug fix release. |
| Description check | ✅ Passed | The pull request description clearly explains the bug (static comparator being overridden), references the related issue #19365, and includes all required checklist items marked as complete. |
✨ Finishing touches
- [ ] 📝 Generate docstrings
🧪 Generate unit tests (beta)
- [ ] Create PR with unit tests
- [ ] Post copyable unit tests in a comment
Comment @coderabbitai help to get the list of available commands and usage tips.
![coderabbitai[bot] avatar](https://avatars.githubusercontent.com/in/347564?v=4 "Published by a pub.dev verified publisher"){kind=small}
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
Codecov Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 73.26%. Comparing base (1aed472) to head (d90d6a9).
:warning: Report is 4 commits behind head on main.
Additional details and impacted files
@@ Coverage Diff @@
## main #20204 +/- ##
============================================
- Coverage 73.29% 73.26% -0.03%
+ Complexity 71780 71757 -23
============================================
Files 5795 5795
Lines 328297 328297
Branches 47282 47282
============================================
- Hits 240612 240519 -93
- Misses 68368 68464 +96
+ Partials 19317 19314 -3
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
:rocket: New features to boost your workflow:
- :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
![codecov[bot] avatar](https://avatars.githubusercontent.com/in/254?v=4 "Published by a pub.dev verified publisher"){kind=small}
Nice catch @ajleong623 - thanks for finding & fixing it.
Can you add up a quick test case as well to verify the same - before and after your change!
:x: Gradle check result for 789555224c5899fec1101f9b064a00dff4cc72c6: 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?
:x: Gradle check result for 3b8467c4720e924b9708e37a5a3340a434a9f73a: 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?
@ajleong623 Gentle nudge that we could make this bug fix to 3.4 if we can merge it this week.
:x: Gradle check result for a2090dd1a294360087cb4398c7f09546170e4ec7: 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?
:x: Gradle check result for c4d8d79dc20c212e4bec3341e883cb7b926f0862: 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?
:x: Gradle check result for 9fed63d9233ee60311d65f2e4e74951662ea3ac1: 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?
:x: Gradle check result for 9fed63d9233ee60311d65f2e4e74951662ea3ac1: 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?
:x: Gradle check result for 9fed63d9233ee60311d65f2e4e74951662ea3ac1: 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?
:x: Gradle check result for d90d6a9e9000fe5531ab83c4d8e3e8a56c736c05: 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?
LGTM! Good find @ajleong623.
Since this was introduced in 3.0, so just porting to 3.4 is good enough: https://github.com/opensearch-project/OpenSearch/pull/14464
Flaky test failure:
> Task :distribution:bwc:bugfix:buildBwcLinuxTar
[3.3.2]
Exception in thread "main" java.io.IOException: Downloading from https://services.gradle.org/distributions/gradle-8.14.3-all.zip failed: timeout (10000ms)
[3.3.2] at org.gradle.wrapper.Install.forceFetch(SourceFile:4)
[3.3.2] at org.gradle.wrapper.Install$1.call(SourceFile:8)
[3.3.2] at org.gradle.wrapper.GradleWrapperMain.main(SourceFile:67)
[3.3.2] Caused by: java.net.SocketTimeoutException: Read timed out
[3.3.2] at java.****/sun.nio.ch.NioSocketImpl.timedRead(NioSocketImpl.java:278)
[3.3.2] at java.****/sun.nio.ch.NioSocketImpl.implRead(NioSocketImpl.java:304)
[3.3.2] at java.****/sun.nio.ch.NioSocketImpl.read(NioSocketImpl.java:346)
[3.3.2] at java.****/sun.nio.ch.NioSocketImpl$1.read(NioSocketImpl.java:796)
[3.3.2] at java.****/java.net.Socket$SocketInputStream.read(Socket.java:1099)
[3.3.2] at java.****/sun.security.ssl.SSLSocketInputRecord.read(SSLSocketInputRecord.java:489)
[3.3.2] at java.****/sun.security.ssl.SSLSocketInputRecord.readHeader(SSLSocketInputRecord.java:483)
[3.3.2] at java.****/sun.security.ssl.SSLSocketInputRecord.bytesInCompletePacket(SSLSocketInputRecord.java:70)
[3.3.2] at java.****/sun.security.ssl.SSLSocketImpl.readApplicationRecord(SSLSocketImpl.java:1461)
[3.3.2] at java.****/sun.security.ssl.SSLSocketImpl$AppInputStream.read(SSLSocketImpl.java:1066)
[3.3.2] at java.****/java.io.BufferedInputStream.fill(BufferedInputStream.java:291)
[3.3.2] at java.****/java.io.BufferedInputStream.read1(BufferedInputStream.java:347)
[3.3.2] at java.****/java.io.BufferedInputStream.implRead(BufferedInputStream.java:420)
[3.3.2] at java.****/java.io.BufferedInputStream.read(BufferedInputStream.java:399)
[3.3.2] at java.****/sun.net.www.http.HttpClient.parseHTTPHeader(HttpClient.java:827)
[3.3.2] at java.****/sun.net.www.http.HttpClient.parseHTTP(HttpClient.java:759)
[3.3.2] at java.****/sun.net.www.protocol.http.HttpURLConnection.getInputStream0(HttpURLConnection.java:1706)
[3.3.2] at java.****/sun.net.www.protocol.http.HttpURLConnection.getInputStream(HttpURLConnection.java:1615)
[3.3.2] at java.****/sun.net.www.protocol.https.HttpsURLConnectionImpl.getInputStream(HttpsURLConnectionImpl.java:223)
[3.3.2] at org.gradle.wrapper.Install.forceFetch(SourceFile:2)
[3.3.2] ... 2 more
Output for /var/jenkins/workspace/gradle-check/search/distribution/bwc/bugfix/build/bwc/checkout-3.3/gradlew:
> Task :plugins:workload-management:javaRestTest
WARNING: Using incubator modules: jdk.incubator.vector
WARNING: A terminally deprecated method in sun.misc.Unsafe has been called
WARNING: sun.misc.Unsafe::objectFieldOffset has been called by net.bytebuddy.dynamic.loading.ClassInjector$UsingUnsafe$Dispatcher$CreationAction
WARNING: Please consider reporting this to the maintainers of class net.bytebuddy.dynamic.loading.ClassInjector$UsingUnsafe$Dispatcher$CreationAction
WARNING: sun.misc.Unsafe::objectFieldOffset will be removed in a future release
> Task :distribution:bwc:bugfix:buildBwcLinuxTar FAILED
> Task :test:framework:missingJavadoc
> Task :qa:smoke-test-multinode:forbiddenApisResources
> Task :plugins:transport-reactor-netty4:generateNotice
> Task :qa:wildfly:spotlessMisc
:x: Gradle check result for d90d6a9e9000fe5531ab83c4d8e3e8a56c736c05: 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?
The backport to 3.4 failed:
The process '/usr/bin/git' failed with exit code 128
To backport manually, run these commands in your terminal:
# Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/OpenSearch/backport-3.4 3.4
# Navigate to the new working tree
pushd ../.worktrees/OpenSearch/backport-3.4
# Create a new branch
git switch --create backport/backport-20204-to-3.4
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 947efd33b930c570055eaf5ace272fe092b2bb5f
# Push it to GitHub
git push --set-upstream origin backport/backport-20204-to-3.4
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/OpenSearch/backport-3.4
Then, create a pull request where the base branch is 3.4 and the compare/head branch is backport/backport-20204-to-3.4.
![opensearch-trigger-bot[bot] avatar](https://avatars.githubusercontent.com/u/80134844?v=4 "Published by a pub.dev verified publisher"){kind=small}
The backport to 3.4 failed:
The process '/usr/bin/git' failed with exit code 128
To backport manually, run these commands in your terminal:
# Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/OpenSearch/backport-3.4 3.4
# Navigate to the new working tree
pushd ../.worktrees/OpenSearch/backport-3.4
# Create a new branch
git switch --create backport/backport-20204-to-3.4
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 947efd33b930c570055eaf5ace272fe092b2bb5f
# Push it to GitHub
git push --set-upstream origin backport/backport-20204-to-3.4
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/OpenSearch/backport-3.4
Then, create a pull request where the base branch is 3.4 and the compare/head branch is backport/backport-20204-to-3.4.
The backport to 3.4 failed:
The process '/usr/bin/git' failed with exit code 128
To backport manually, run these commands in your terminal:
# Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/OpenSearch/backport-3.4 3.4
# Navigate to the new working tree
pushd ../.worktrees/OpenSearch/backport-3.4
# Create a new branch
git switch --create backport/backport-20204-to-3.4
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 947efd33b930c570055eaf5ace272fe092b2bb5f
# Push it to GitHub
git push --set-upstream origin backport/backport-20204-to-3.4
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/OpenSearch/backport-3.4
Then, create a pull request where the base branch is 3.4 and the compare/head branch is backport/backport-20204-to-3.4.