ORC-1332: Avoid NegativeArraySizeException when using searchArgument #1340
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
cxzl25
commented
Dec 18, 2022
| @@ -237,7 +237,7 @@ protected RecordReaderImpl(ReaderImpl fileReader, | |||
| this.fileIncluded = evolution.getFileIncluded(); | |||
| SearchArgument sarg = options.getSearchArgument(); | |||
| boolean[] rowIndexCols = new boolean[evolution.getFileIncluded().length]; | |||
| if (sarg != null && rowIndexStride != 0) { | |||
| if (sarg != null && rowIndexStride > 0) { | |||
There was a problem hiding this comment.
orc/java/core/src/java/org/apache/orc/impl/WriterImpl.java
Lines 186 to 188 in becf5a5
|
Thank you @cxzl25. This is a good fix. The main problem is the the inconsistency of reading and writing implementation. |
deshanxiao
reviewed
Dec 19, 2022
java/core/src/test/org/apache/orc/impl/TestRecordReaderImpl.java
Outdated
Show resolved
Hide resolved
deshanxiao
approved these changes
Dec 19, 2022
|
Thank you for making a PR. I think it would be better to also make the writer reset the negative rowIndexStride to 0. This provides a wider range of compatibility. Allowing a negative rowIndexStride to be written to the file footer, then older versions of the ORC reader will still not be read correctly. |
williamhyun
pushed a commit
that referenced
this pull request
Dec 26, 2022
### What changes were proposed in this pull request? searchArgument takes effect only when rowIndexStride is greater than 0. ### Why are the changes needed? When `orc.row.index.stride` is set to a negative number, using searchArgument will throw NegativeArraySizeException. ```java Caused by: java.lang.NegativeArraySizeException at org.apache.orc.impl.RecordReaderImpl$SargApplier.pickRowGroups(RecordReaderImpl.java:1164) at org.apache.orc.impl.RecordReaderImpl.pickRowGroups(RecordReaderImpl.java:1273) at org.apache.orc.impl.RecordReaderImpl.readStripe(RecordReaderImpl.java:1293) at org.apache.orc.impl.RecordReaderImpl.advanceStripe(RecordReaderImpl.java:1345) at org.apache.orc.impl.RecordReaderImpl.advanceToNextRow(RecordReaderImpl.java:1388) at org.apache.orc.impl.RecordReaderImpl.<init>(RecordReaderImpl.java:367) ``` ### How was this patch tested? add UT Closes #1340 from cxzl25/ORC-1332. Authored-by: sychen <sychen@ctrip.com> Signed-off-by: William Hyun <william@apache.org> (cherry picked from commit ae84ad0) Signed-off-by: William Hyun <william@apache.org>
dongjoon-hyun
pushed a commit
that referenced
this pull request
Jan 6, 2023
### What changes were proposed in this pull request? searchArgument takes effect only when rowIndexStride is greater than 0. ### Why are the changes needed? When `orc.row.index.stride` is set to a negative number, using searchArgument will throw NegativeArraySizeException. ```java Caused by: java.lang.NegativeArraySizeException at org.apache.orc.impl.RecordReaderImpl$SargApplier.pickRowGroups(RecordReaderImpl.java:1164) at org.apache.orc.impl.RecordReaderImpl.pickRowGroups(RecordReaderImpl.java:1273) at org.apache.orc.impl.RecordReaderImpl.readStripe(RecordReaderImpl.java:1293) at org.apache.orc.impl.RecordReaderImpl.advanceStripe(RecordReaderImpl.java:1345) at org.apache.orc.impl.RecordReaderImpl.advanceToNextRow(RecordReaderImpl.java:1388) at org.apache.orc.impl.RecordReaderImpl.<init>(RecordReaderImpl.java:367) ``` ### How was this patch tested? add UT Closes #1340 from cxzl25/ORC-1332. Authored-by: sychen <sychen@ctrip.com> Signed-off-by: William Hyun <william@apache.org> (cherry picked from commit ae84ad0) Signed-off-by: William Hyun <william@apache.org> (cherry picked from commit 38aef73) Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
|
I backported this to branch-1.7 too. |
cxzl25
added a commit
to cxzl25/orc
that referenced
this pull request
Jan 11, 2024
### What changes were proposed in this pull request? searchArgument takes effect only when rowIndexStride is greater than 0. ### Why are the changes needed? When `orc.row.index.stride` is set to a negative number, using searchArgument will throw NegativeArraySizeException. ```java Caused by: java.lang.NegativeArraySizeException at org.apache.orc.impl.RecordReaderImpl$SargApplier.pickRowGroups(RecordReaderImpl.java:1164) at org.apache.orc.impl.RecordReaderImpl.pickRowGroups(RecordReaderImpl.java:1273) at org.apache.orc.impl.RecordReaderImpl.readStripe(RecordReaderImpl.java:1293) at org.apache.orc.impl.RecordReaderImpl.advanceStripe(RecordReaderImpl.java:1345) at org.apache.orc.impl.RecordReaderImpl.advanceToNextRow(RecordReaderImpl.java:1388) at org.apache.orc.impl.RecordReaderImpl.<init>(RecordReaderImpl.java:367) ``` ### How was this patch tested? add UT Closes apache#1340 from cxzl25/ORC-1332. Authored-by: sychen <sychen@ctrip.com> Signed-off-by: William Hyun <william@apache.org>
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?
searchArgument takes effect only when rowIndexStride is greater than 0.
Why are the changes needed?
When
orc.row.index.strideis set to a negative number, using searchArgument will throw NegativeArraySizeException.How was this patch tested?
add UT