lidavidm commented on code in PR #1465:
URL: https://github.com/apache/arrow-java/pull/1465#discussion_r4557929148
##########
vector/src/main/java/org/apache/arrow/vector/BaseFixedWidthVector.java:
##########
@@ -503,8 +503,11 @@ public void loadFieldBuffers(ArrowFieldNode fieldNode,
List<ArrowBuf> ownBuffers
ArrowBuf bitBuffer = ownBuffers.get(0);
ArrowBuf dataBuffer = ownBuffers.get(1);
+ // Load the new validity buffer first, so that a failure leaves the vector
unchanged
+ ArrowBuf newValidityBuffer =
+ BitVectorHelper.loadValidityBuffer(fieldNode, bitBuffer, allocator);
validityBuffer.getReferenceManager().release();
- validityBuffer = BitVectorHelper.loadValidityBuffer(fieldNode, bitBuffer,
allocator);
+ validityBuffer = newValidityBuffer;
valueBuffer.getReferenceManager().release();
valueBuffer = dataBuffer.getReferenceManager().retain(dataBuffer,
allocator);
Review Comment:
Shouldn't we be doing the same thing for the value buffer? Otherwise you
still have an inconsistent vector.
Also, this would mean that you temporarily have 2x memory usage instead of
1x as before. Is it necessarily wrong to say that failing a load means the
vector is cleared? After all, if you're loading data, presumably you didn't
care about the previous contents of the vector anymore.
--
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]
