axreldable commented on code in PR #1145:
URL: https://github.com/apache/arrow-java/pull/1145#discussion_r3254785710
##########
vector/src/main/java/org/apache/arrow/vector/VectorSchemaRoot.java:
##########
@@ -226,7 +240,10 @@ public VectorSchemaRoot removeVector(int index) {
List<FieldVector> newVectors = new ArrayList<>();
for (int i = 0; i < fieldVectors.size(); i++) {
if (i != index) {
- newVectors.add(fieldVectors.get(i));
+ FieldVector v = fieldVectors.get(i);
+ TransferPair transferPair = v.getTransferPair(v.getAllocator());
+ transferPair.transfer();
+ newVectors.add((FieldVector) transferPair.getTo());
Review Comment:
nit: Consider extracting the transferring logic into a separate method to
avoid code duplication.
e.g.
```
private static FieldVector transferVector(FieldVector vector) {
TransferPair transferPair =
vector.getTransferPair(vector.getAllocator());
transferPair.transfer();
return (FieldVector) transferPair.getTo();
}
```
##########
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:
> The removed vector's data is not transferred and is released.
`is released` is not 100% accurate and can be confusing. The vector remains
in the original schema root until it's closed/cleared.
--
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]
