Support-itecz commented on code in PR #1376:
URL: https://github.com/apache/arrow-java/pull/1376#discussion_r4490400152
##########
vector/src/main/java/org/apache/arrow/vector/util/VectorAppender.java:
##########
@@ -99,6 +99,9 @@ public ValueVector visit(BaseFixedWidthVector deltaVector,
Void value) {
deltaVector.getValueCount(),
targetVector.getDataBuffer());
} else {
+ deltaVector
+ .getDataBuffer()
+ .checkBytes(0, (long) deltaVector.getTypeWidth() *
deltaVector.getValueCount());
Review Comment:
Agreed, the target shouldn't be reallocated or have bits written for a delta
we then reject. Moved the validity and data checks to right after the
`getValueCount() == 0` return using `checkCapacity`, with the `BitVector` case
sized from the validity size as you suggested, and dropped the late
`checkBytes`. Union and dense union now check the type buffer (`valueCount *
TYPE_WIDTH`) before the capacity loop too. The two appender tests also assert
the target's value count, capacity and values are unchanged after the throw.
Pushed in 67b5780.
##########
vector/src/main/java/org/apache/arrow/vector/util/VectorAppender.java:
##########
@@ -366,6 +369,10 @@ public ValueVector visit(BaseVariableWidthViewVector
deltaVector, Void value) {
// append view buffer
ArrowBuf targetViewBuffer = targetVector.getDataBuffer();
+ deltaVector
+ .getDataBuffer()
+ .checkBytes(
+ 0, (long) BaseVariableWidthViewVector.ELEMENT_SIZE *
deltaVector.getValueCount());
Review Comment:
Right, a throw there left retained buffers in the target's `dataBuffers`.
The view and validity checks now run with `checkCapacity` before the capacity
loop, so nothing is retained or added unless the delta passes. Same commit.
--
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]
