pitrou commented on code in PR #13815:
URL: https://github.com/apache/arrow/pull/13815#discussion_r940252012
##########
java/vector/src/main/java/org/apache/arrow/vector/BaseVariableWidthVector.java:
##########
@@ -1240,7 +1241,7 @@ protected final void handleSafe(int index, int
dataLength) {
* So even though we may have setup an initial capacity of 1024
* elements in the vector, it is quite possible
* that we need to reAlloc() the data buffer when we are setting
- * the 5th element in the vector simply because previous
+ * the 1025th element in the vector simply because previous
Review Comment:
No, I don't think this change is right. You should read this example as:
* the binary/string vector is 1024 elements long
* the 4 first binary/string elements already occupy 1024 *bytes* in the data
buffer, so need to resize the data buffer as soon as the 5th binary/string
element is appended
##########
java/vector/src/main/java/org/apache/arrow/vector/BaseVariableWidthVector.java:
##########
@@ -1250,8 +1251,10 @@ protected final void handleSafe(int index, int
dataLength) {
while (index >= getValueCapacity()) {
reallocValidityAndOffsetBuffers();
}
- final int startOffset = lastSet < 0 ? 0 : getStartOffset(lastSet + 1);
- while (valueBuffer.capacity() < (startOffset + dataLength)) {
+ final long startOffset = lastSet < 0 ? 0 : getStartOffset(lastSet + 1);
+ final long targetCapacity = startOffset + dataLength;
+ checkDataBufferSize(targetCapacity);
+ while (valueBuffer.capacity() < targetCapacity) {
reallocDataBuffer();
}
Review Comment:
Slightly unrelated, but why is this using a `while` loop? Ideally it would
be more efficient to write:
```suggestion
reallocDataBuffer(targetCapacity);
```
--
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]
