ORC-672: FIX ORC type conversion within arrays where array.length > 1… #568
Conversation
This file contains 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
added 2 commits
November 6, 2020 16:22
…024 (the default CV size) Change-Id: I26426e6146ec05cbc1f96d79ec7960494cbc3669
Change-Id: Id73642d6b398554091cf9230e2870feb1b11cb64
|
Thank you, @pgaref . According to the JIRA, is this a bug for 1.5/1.6/1.7? |
Hey @dongjoon-hyun -- thats correct. |
|
Got it. Thanks, @pgaref . |
|
I backported ORC-598 to branch-1.6/1.5 for next releases. |
java/core/src/test/org/apache/orc/impl/TestConvertTreeReaderFactory.java
Show resolved
Hide resolved
java/core/src/test/org/apache/orc/impl/TestConvertTreeReaderFactory.java
Outdated
Show resolved
Hide resolved
java/core/src/test/org/apache/orc/impl/TestConvertTreeReaderFactory.java
Outdated
Show resolved
Hide resolved
java/core/src/test/org/apache/orc/impl/TestConvertTreeReaderFactory.java
Outdated
Show resolved
Hide resolved
Change-Id: Id739b2f01a465d641e72eafba4f04576cb7c44ca
java/core/src/test/org/apache/orc/impl/TestConvertTreeReaderFactory.java
Outdated
Show resolved
Hide resolved
| @@ -1396,7 +1396,7 @@ public void nextVector(ColumnVector previousVector, | |||
| FilterContext filterContext) throws IOException { | |||
| if (inBytesColVector == null) { | |||
| // Allocate column vector for file; cast column vector for reader. | |||
| inBytesColVector = new BytesColumnVector(); | |||
| inBytesColVector = new BytesColumnVector(batchSize); | |||
There was a problem hiding this comment.
@pgaref . Could you add a test coverage for this change? It seems that we are missing Varchar to Binary conversion test case.
There was a problem hiding this comment.
Good catch @dongjoon-hyun ! test added
dongjoon-hyun
approved these changes
Nov 9, 2020
Change-Id: I057613382422270e6dcd540ae07d89b0e4be6f30
dongjoon-hyun
pushed a commit
that referenced
this pull request
Nov 9, 2020
…024 (#568) ### What changes were proposed in this pull request? Orc type conversion is throwing ArrayIndexOutOfBoundsException when using ColumnVectors larger than 1024. The issue is originating from the ConvertTreeReaderFactory where we convert types always using the default CV length instead of the specific batch size. ### Why are the changes needed? This PR takes into account batchSize when converting types and adds tests for all possible type conversions (with large batchSize) as part of TestConvertTreeReaderFactory. ### How was this patch tested? TestConvertTreeReaderFactory (cherry picked from commit ec644f8) Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
|
Merged to master/1.6. Could you make a backporting PR to branch-1.5, too? |
pgaref
pushed a commit
to pgaref/orc
that referenced
this pull request
Nov 9, 2020
…024 (apache#568) ### What changes were proposed in this pull request? Orc type conversion is throwing ArrayIndexOutOfBoundsException when using ColumnVectors larger than 1024. The issue is originating from the ConvertTreeReaderFactory where we convert types always using the default CV length instead of the specific batch size. ### Why are the changes needed? This PR takes into account batchSize when converting types and adds tests for all possible type conversions (with large batchSize) as part of TestConvertTreeReaderFactory. ### How was this patch tested? TestConvertTreeReaderFactory (cherry picked from commit ec644f8) Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
@dongjoon-hyun Thanks for taking care of this -- 1.5 backport available at #569 |
|
Thank you always! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What changes were proposed in this pull request?
Orc type conversion is throwing ArrayIndexOutOfBoundsException when using ColumnVectors larger than 1024.
The issue is originating from the ConvertTreeReaderFactory where we convert types always using the default CV length instead of the specific batch size.
Why are the changes needed?
This PR takes into account batchSize when converting types and adds tests for all possible type conversions (with large batchSize) as part of TestConvertTreeReaderFactory.
How was this patch tested?
TestConvertTreeReaderFactory