John-W-Lewis commented on code in PR #1145:
URL: https://github.com/apache/arrow-java/pull/1145#discussion_r3255227248
##########
vector/src/main/java/org/apache/arrow/vector/VectorSchemaRoot.java:
##########
@@ -201,23 +205,33 @@ public VectorSchemaRoot addVector(int index, FieldVector
vector) {
Preconditions.checkNotNull(vector);
Preconditions.checkArgument(index >= 0 && index <= fieldVectors.size());
List<FieldVector> newVectors = new ArrayList<>();
- if (index == fieldVectors.size()) {
- newVectors.addAll(fieldVectors);
- newVectors.add(vector);
- } else {
- for (int i = 0; i < fieldVectors.size(); i++) {
- if (i == index) {
- newVectors.add(vector);
- }
- newVectors.add(fieldVectors.get(i));
+ for (int i = 0; i < fieldVectors.size(); i++) {
+ if (i == index) {
+ TransferPair addPair = vector.getTransferPair(vector.getAllocator());
+ addPair.transfer();
+ newVectors.add((FieldVector) addPair.getTo());
}
+ FieldVector v = fieldVectors.get(i);
+ TransferPair transferPair = v.getTransferPair(v.getAllocator());
+ transferPair.transfer();
+ newVectors.add((FieldVector) transferPair.getTo());
+ }
+ if (index == fieldVectors.size()) {
+ TransferPair addPair = vector.getTransferPair(vector.getAllocator());
+ addPair.transfer();
+ newVectors.add((FieldVector) addPair.getTo());
}
return new VectorSchemaRoot(newVectors);
}
/**
* Remove vector from the record batch, producing a new VectorSchemaRoot.
*
+ * <p>Buffer ownership is transferred to the returned root via {@link
TransferPair}. After this
+ * operation, the vectors in this root are left in a transferred (empty)
state. The removed
+ * vector's data is not transferred and is released. This root can be reused
by calling {@link
Review Comment:
Thank you for spotting this. You're right that "is released" was inaccurate.
Looking more closely, it was also a code issue. For the source vectors which
are not removed, transfer() creates new ArrowBufs in the corresponding target
vector, each referencing the same underlying allocation as its source
counterpart, while the source vector's ArrowBufs release their references. So,
after the operation, those source vectors' ArrowBufs no longer own any
allocated memory. But for the removed vector, we weren't doing anything -- its
ArrowBufs still held references to their underlying allocations, which would
only be freed when the original root was eventually closed.
I've added a clear() on the removed vector so that its buffers also release
their underlying allocations, making it consistent with the other vectors in
the source root after the operation.
--
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]
