dongjoon-hyun commented on code in PR #1412:
URL: https://github.com/apache/orc/pull/1412#discussion_r1110399200
##########
java/core/src/java/org/apache/orc/impl/DynamicByteArray.java:
##########
@@ -59,10 +60,16 @@ private void grow(int chunkIndex) {
int newSize = Math.max(chunkIndex + 1, 2 * data.length);
data = Arrays.copyOf(data, newSize);
}
- for(int i=initializedChunks; i <= chunkIndex; ++i) {
+ for (int i = initializedChunks; i <= chunkIndex; ++i) {
data[i] = new byte[chunkSize];
}
initializedChunks = chunkIndex + 1;
+ } else if (chunkIndex < 0) {
Review Comment:
If NPE happens always before, this looks like a precondition check for this
method.
- Shall we place this at the beginning of this method as an independent `if`
statement?
- Could you add your new logic to the function description too? The current
one looks vague.
https://github.com/apache/orc/blob/d403e11b7eab126259fb0683c0eb5c6131fd347b/java/core/src/java/org/apache/orc/impl/DynamicByteArray.java#L54
- Lastly, can we have a unit test to check the new exception message?
BTW, I revise the PR title, @cxzl25 . You can change back if you want.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
