lidavidm commented on code in PR #884:
URL: https://github.com/apache/arrow-java/pull/884#discussion_r2463177254


##########
vector/src/main/java/org/apache/arrow/vector/util/VectorAppender.java:
##########
@@ -698,4 +703,70 @@ public ValueVector visit(ExtensionTypeVector<?> 
deltaVector, Void value) {
     deltaVector.getUnderlyingVector().accept(underlyingAppender, null);
     return targetVector;
   }
+
+  @Override
+  public ValueVector visit(RunEndEncodedVector deltaVector, Void value) {
+    Preconditions.checkArgument(
+        typeVisitor.equals(deltaVector),
+        "The vector to append must have the same type as the targetVector 
being appended");

Review Comment:
   ```suggestion
           "The vector to append must have the same type as the deltaVector 
being appended");
   ```



##########
vector/src/main/java/org/apache/arrow/vector/util/VectorAppender.java:
##########
@@ -698,4 +703,70 @@ public ValueVector visit(ExtensionTypeVector<?> 
deltaVector, Void value) {
     deltaVector.getUnderlyingVector().accept(underlyingAppender, null);
     return targetVector;
   }
+
+  @Override
+  public ValueVector visit(RunEndEncodedVector deltaVector, Void value) {
+    Preconditions.checkArgument(
+        typeVisitor.equals(deltaVector),
+        "The vector to append must have the same type as the targetVector 
being appended");
+
+    if (deltaVector.getValueCount() == 0) {
+      return targetVector; // optimization, nothing to append, return
+    }
+
+    RunEndEncodedVector targetEncodedVector = (RunEndEncodedVector) 
targetVector;
+
+    final int targetLogicalValueCount = targetEncodedVector.getValueCount();
+
+    // Append the values vector first.
+    VectorAppender valueAppender = new 
VectorAppender(targetEncodedVector.getValuesVector());
+    deltaVector.getValuesVector().accept(valueAppender, null);
+
+    // Then append the run-ends vector.
+    BaseIntVector targetRunEndsVector = (BaseIntVector) 
targetEncodedVector.getRunEndsVector();
+    BaseIntVector deltaRunEndsVector = (BaseIntVector) 
deltaVector.getRunEndsVector();
+
+    // Shift the delta run-ends vector in-place before appending.
+    shiftRunEndsVector(
+        deltaRunEndsVector,
+        deltaRunEndsVector.getDataBuffer(),
+        targetLogicalValueCount,
+        deltaRunEndsVector.getValueCount());
+
+    // Append the now-shifted delta run-ends vector to the target.
+    new VectorAppender(targetRunEndsVector).visit((BaseFixedWidthVector) 
deltaRunEndsVector, null);
+
+    targetEncodedVector.setValueCount(targetLogicalValueCount + 
deltaVector.getValueCount());
+
+    return targetVector;
+  }
+
+  private void shiftRunEndsVector(
+      ValueVector toRunEndVector, ArrowBuf fromRunEndBuffer, int offset, int 
physicalLength) {

Review Comment:
   It's also very confusing to frame this as a 'to vector' and 'from buffer' 
when really it is in an in-place change.



##########
vector/src/main/java/org/apache/arrow/vector/util/VectorAppender.java:
##########
@@ -698,4 +703,70 @@ public ValueVector visit(ExtensionTypeVector<?> 
deltaVector, Void value) {
     deltaVector.getUnderlyingVector().accept(underlyingAppender, null);
     return targetVector;
   }
+
+  @Override
+  public ValueVector visit(RunEndEncodedVector deltaVector, Void value) {
+    Preconditions.checkArgument(
+        typeVisitor.equals(deltaVector),
+        "The vector to append must have the same type as the targetVector 
being appended");
+
+    if (deltaVector.getValueCount() == 0) {
+      return targetVector; // optimization, nothing to append, return
+    }
+
+    RunEndEncodedVector targetEncodedVector = (RunEndEncodedVector) 
targetVector;
+
+    final int targetLogicalValueCount = targetEncodedVector.getValueCount();
+
+    // Append the values vector first.
+    VectorAppender valueAppender = new 
VectorAppender(targetEncodedVector.getValuesVector());
+    deltaVector.getValuesVector().accept(valueAppender, null);
+
+    // Then append the run-ends vector.
+    BaseIntVector targetRunEndsVector = (BaseIntVector) 
targetEncodedVector.getRunEndsVector();
+    BaseIntVector deltaRunEndsVector = (BaseIntVector) 
deltaVector.getRunEndsVector();
+
+    // Shift the delta run-ends vector in-place before appending.
+    shiftRunEndsVector(
+        deltaRunEndsVector,
+        deltaRunEndsVector.getDataBuffer(),
+        targetLogicalValueCount,
+        deltaRunEndsVector.getValueCount());

Review Comment:
   I don't think we ever defined it clearly, but I'm not sure that we want to 
modify `deltaVector`. It would be safer IMO to do this to a copy, or even just 
do the shift+append and never modify the delta run-ends vector.



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

Reply via email to