[common] Stop nesting row-tracking vector wrappers per batch - #9634
Open
LuciferYang wants to merge 1 commit into
Open
[common] Stop nesting row-tracking vector wrappers per batch#9634LuciferYang wants to merge 1 commit into
LuciferYang wants to merge 1 commit into
Conversation
assignRowTracking wraps the _ROW_ID and _SEQUENCE_NUMBER vectors, and it runs once per batch while the wrapped vector stays in the file's single ColumnarBatch, so batch N wrapped batch N-1's wrapper. Reading a row then walked one delegation level per batch already read, and enough batches ended in a StackOverflowError. Name the wrapper and strip an existing one before installing a new one. Also leave both columns alone when firstRowId or snapshotId is null, which the row-based branch of the same method already did.
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Purpose
close #9633
ColumnarRowIterator.assignRowTrackingwraps the_ROW_IDand_SEQUENCE_NUMBERvectors in a delegatingLongColumnVector, and it runs once per batch while the wrapped vector lives on in the file's singleColumnarBatch. Batch N therefore wrapped batch N-1's wrapper, so reading one row walked one level of delegation per batch already read. Over a file that is quadratic, and with enough batches it ends in aStackOverflowError.The wrapper is now a named
TrackingLongColumnVectorholding its base, andassignRowTrackingstrips an existing one before installing a new one, so the depth stays at one however many times it runs.The same method also unboxed a nullable
firstRowId:RawFileSplitReadsays a file may carry no stored row id, and when the row's_rowidis null as well,firstRowId + returnedPosition()threw aNullPointerException. Both columns are now left untouched when their value is null, which is what the row-based branch of this method already did withfirstRowId != null.Tests
ColumnarRowIteratorTest.testAssignRowTrackingSkipsNullFirstRowIdpasses a nullfirstRowIdand asserts the vector is the original instance, not a wrapper.ColumnarRowIteratorTest.testRepeatedAssignRowTrackingDoesNotNestassigns 100000 times and then reads all four rows, covering both the stored-id and the fallback-to-firstRowId + positionpaths. The count is deliberate: nesting keeps returning correct values, so only a read deep enough to overflow the stack distinguishes the two versions.Against the unfixed iterator the first fails on the vector identity and the second errors with
StackOverflowError.mvn -pl paimon-common -Dtest=ColumnarRowIteratorTest teston JDK 8: 3 tests, 0 failures.spotless:checkandcheckstyle:checkon paimon-common are clean.