[GitHub] [arrow] scampi commented on a change in pull request #6402: ARROW-7831: [Java] do not allocate a new offset buffer if the slice starts at 0 since the relative offset pointer would be unchanged

[GitBox]

scampi commented on a change in pull request #6402:
URL: https://github.com/apache/arrow/pull/6402#discussion_r425610823



##########
File path: 
java/vector/src/main/java/org/apache/arrow/vector/BaseVariableWidthVector.java
##########
@@ -740,10 +740,16 @@ private void splitAndTransferOffsetBuffer(int startIndex, 
int length, BaseVariab
     final int start = offsetBuffer.getInt(startIndex * OFFSET_WIDTH);
     final int end = offsetBuffer.getInt((startIndex + length) * OFFSET_WIDTH);
     final int dataLength = end - start;
-    target.allocateOffsetBuffer((length + 1) * OFFSET_WIDTH);
-    for (int i = 0; i < length + 1; i++) {
-      final int relativeSourceOffset = offsetBuffer.getInt((startIndex + i) * 
OFFSET_WIDTH) - start;
-      target.offsetBuffer.setInt(i * OFFSET_WIDTH, relativeSourceOffset);
+
+    if (startIndex == 0) {
+      target.offsetBuffer = offsetBuffer.slice(0, (1 + length) * OFFSET_WIDTH);
+      target.offsetBuffer.getReferenceManager().retain();

Review comment:
       @emkornfield I based the logic of that branch (relying on `retain()`) on 
the same case below for splitting the vailidty buffer:
   
   
https://github.com/apache/arrow/blob/29545bcdef35f4151379e72e69ef7ce619f1a517/java/vector/src/main/java/org/apache/arrow/vector/BaseVariableWidthVector.java#L772
   
   However, for the value buffer, `transferOwnership` is used, which is called 
from `transferBuffer`:
   
   
https://github.com/apache/arrow/blob/29545bcdef35f4151379e72e69ef7ce619f1a517/java/vector/src/main/java/org/apache/arrow/vector/BaseVariableWidthVector.java#L752
   
   I am not sure about the difference, but from the documentation I understand 
that the ownership is not transferred to the new allocator when using `retain`. 
Does that mean the validity buffer case needs to be updated and use 
`transferOwnership` as well ? 




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org