fix: DH-20318: Fix bug in UnionSourceManager pushdown estimation that caused AIOOBE in UnionRedirection#7796
Merged
rcaudy merged 6 commits intodeephaven:mainfrom Mar 17, 2026
Merged
Conversation
…nstituent row keys have gaps When `constituentChangesPermitted=true`, removing then adding a constituent creates gaps in `constituentRows` row keys (e.g. remove key 8, add key 15 → max key 15 with only 15 entries). `initialize` was incorrectly using these row keys as 0-based slot positions for `UnionRedirection` lookups, causing `currLastRowKeyForSlot(15)` to access `currFirstRowKeys[16]` → `ArrayIndexOutOfBoundsException: Index 16 out of bounds for length 16`. Fix uses a separate `MutableInt slotPosition` counter (0-based position) for `unionRedirection` slot lookups, while keeping the actual `rowKey` for `constituentTables.get/getPrev`. Adds a regression test that reproduces the exact crash scenario. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… the fix The AIOBE in UnionSourcePushdownFilterContext.initialize propagates via errorUpdate -> notifyListenersOnError, silently setting filtered.isFailed()=true. Since filtered had no downstream listeners, RefreshingTableTestCase.reportUpdateError was never called and TestCase.fail() was never triggered. filtered.size() returned the stale count of N, so assertEquals(N, filtered.size()) passed even without the fix. Attach an ErrorListener to filtered before the update cycle to capture the exception delivered via notifyListenersOnError. Without the fix the assertThat on originalException() fails showing the actual AIOBE; with the fix it is null. Also assert !filtered.isFailed() as a belt-and-suspenders check. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
No docs changes detected for 03860ed |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes an ArrayIndexOutOfBoundsException in UnionSourcePushdownFilterContext.initialize that occurred when constituent changes created gaps in row keys. The bug was in the pushdown filter estimation logic, where row keys from constituentRows were incorrectly used as position-based slot indices for unionRedirection lookups.
Changes:
- Fixed
initialize()inUnionSourcePushdownFilterContextto use a 0-based position counter forunionRedirectionslot lookups while using actual row keys forconstituentTableslookups. - Added a regression test (
testMergeWhereWithConstituentChangeGap) that reproduces the AIOOBE by removing a constituent (creating a gap) and adding a new one beyond the previous max key.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
engine/table/src/main/java/io/deephaven/engine/table/impl/sources/UnionSourceManager.java |
Decouples position-based slot index from row key in initialize(), replacing TIntArrayList of row keys with a MutableInt position counter |
engine/table/src/test/java/io/deephaven/engine/table/impl/PartitionedTableTest.java |
Adds regression test reproducing the AIOOBE when constituent row keys have gaps |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…rContext.initialize; fix AIOOBE with constituent row-key gaps When constituentChangesPermitted=true, removing a constituent creates a gap in constituentRows row keys. The previous code incorrectly used those row keys as slot indices into unionRedirection (which is position-indexed), causing an ArrayIndexOutOfBoundsException in currLastRowKeyForSlot when the max row key exceeded currSize-1. Fix uses a 0-based slot counter alongside a ChunkedObjectColumnIterator over constituentTables to avoid serial get/getPrev calls, cleanly separating position-indexed slot lookups from row-key-indexed table lookups. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes an ArrayIndexOutOfBoundsException in UnionSourceManager's pushdown filter estimation. The bug occurred when constituentChangesPermitted=true and a constituent removal created a gap in constituentRows row keys, causing the initialize() method to incorrectly use row keys (which may have gaps) as position-based slot indices for unionRedirection lookups.
Changes:
- Fixed
UnionSourcePushdownFilterContext.initialize()to use a position-based slot counter with an iterator over constituent tables, instead of usingconstituentRowsrow keys directly as slot positions. - Added a regression test that reproduces the AIOOBE by removing a constituent (creating a gap) and adding a new one past the previous max key.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
engine/table/src/main/java/io/deephaven/engine/table/impl/sources/UnionSourceManager.java |
Replaced row-key-based slot lookup with iterator + 0-based slot counter in initialize() |
engine/table/src/test/java/io/deephaven/engine/table/impl/PartitionedTableTest.java |
Added regression test testMergeWhereWithConstituentChangeGap |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
cpwright
reviewed
Mar 17, 2026
cpwright
previously approved these changes
Mar 17, 2026
…itionedTableTest.java Co-authored-by: Charles P. Wright <cpwright@gmail.com>
cpwright
previously approved these changes
Mar 17, 2026
…ments Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
cpwright
approved these changes
Mar 17, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.