You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@arrow.apache.org by ra...@apache.org on 2019/08/01 07:11:34 UTC

[arrow] branch master updated: ARROW-6021: [Java] Extract copyFrom and copyFromSafe methods to ValueVector interface

This is an automated email from the ASF dual-hosted git repository.

ravindra pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/arrow.git


The following commit(s) were added to refs/heads/master by this push:
     new 25efd82  ARROW-6021: [Java] Extract copyFrom and copyFromSafe methods to ValueVector interface
25efd82 is described below

commit 25efd825e0f523fe32bd2eabd6c2ba60921d031f
Author: liyafan82 <fa...@foxmail.com>
AuthorDate: Thu Aug 1 12:41:14 2019 +0530

    ARROW-6021: [Java] Extract copyFrom and copyFromSafe methods to ValueVector interface
    
    Currently we have copyFrom and copyFromSafe methods in fixed-width and variable-width vectors. Extracting them to the common super interface will make it much more convenient to use them, and avoid unnecessary if-else statements.
    
    Closes #4931 from liyafan82/fly_0724_copy and squashes the following commits:
    
    1ce95bb77 <liyafan82>  Extract copyFrom and copyFromSafe methods to ValueVector interface
    
    Authored-by: liyafan82 <fa...@foxmail.com>
    Signed-off-by: Pindikura Ravindra <ra...@dremio.com>
---
 .../src/main/codegen/templates/UnionVector.java     | 11 +++++++----
 .../apache/arrow/vector/BaseFixedWidthVector.java   | 11 +++++++----
 .../org/apache/arrow/vector/BaseValueVector.java    | 10 ++++++++++
 .../arrow/vector/BaseVariableWidthVector.java       | 21 ++++++++++++---------
 .../java/org/apache/arrow/vector/BitVector.java     | 13 ++++++++++---
 .../java/org/apache/arrow/vector/ValueVector.java   | 21 +++++++++++++++++++++
 .../java/org/apache/arrow/vector/ZeroVector.java    | 10 ++++++++++
 .../vector/complex/AbstractContainerVector.java     | 10 ++++++++++
 8 files changed, 87 insertions(+), 20 deletions(-)

diff --git a/java/vector/src/main/codegen/templates/UnionVector.java b/java/vector/src/main/codegen/templates/UnionVector.java
index c79dfd0..c4db607 100644
--- a/java/vector/src/main/codegen/templates/UnionVector.java
+++ b/java/vector/src/main/codegen/templates/UnionVector.java
@@ -364,13 +364,16 @@ public class UnionVector implements FieldVector {
     return new TransferImpl((UnionVector) target);
   }
 
-  public void copyFrom(int inIndex, int outIndex, UnionVector from) {
-    from.getReader().setPosition(inIndex);
+  @Override
+  public void copyFrom(int inIndex, int outIndex, ValueVector from) {
+    UnionVector fromCast = (UnionVector) from;
+    fromCast.getReader().setPosition(inIndex);
     getWriter().setPosition(outIndex);
-    ComplexCopier.copy(from.reader, writer);
+    ComplexCopier.copy(fromCast.reader, writer);
   }
 
-  public void copyFromSafe(int inIndex, int outIndex, UnionVector from) {
+  @Override
+  public void copyFromSafe(int inIndex, int outIndex, ValueVector from) {
     copyFrom(inIndex, outIndex, from);
   }
 
diff --git a/java/vector/src/main/java/org/apache/arrow/vector/BaseFixedWidthVector.java b/java/vector/src/main/java/org/apache/arrow/vector/BaseFixedWidthVector.java
index 76228b4..aca34af 100644
--- a/java/vector/src/main/java/org/apache/arrow/vector/BaseFixedWidthVector.java
+++ b/java/vector/src/main/java/org/apache/arrow/vector/BaseFixedWidthVector.java
@@ -25,6 +25,7 @@ import java.util.List;
 import org.apache.arrow.memory.BufferAllocator;
 import org.apache.arrow.memory.util.ArrowBufPointer;
 import org.apache.arrow.memory.util.ByteFunctionHelpers;
+import org.apache.arrow.util.Preconditions;
 import org.apache.arrow.vector.ipc.message.ArrowFieldNode;
 import org.apache.arrow.vector.types.pojo.Field;
 import org.apache.arrow.vector.util.CallBack;
@@ -810,13 +811,14 @@ public abstract class BaseFixedWidthVector extends BaseValueVector
 
   /**
    * Copy a cell value from a particular index in source vector to a particular
-   * position in this vector.
+   * position in this vector. The source vector should be of the same type as this one.
    *
    * @param fromIndex position to copy from in source vector
    * @param thisIndex position to copy to in this vector
    * @param from      source vector
    */
-  public void copyFrom(int fromIndex, int thisIndex, BaseFixedWidthVector from) {
+  public void copyFrom(int fromIndex, int thisIndex, ValueVector from) {
+    Preconditions.checkArgument(this.getMinorType() == from.getMinorType());
     if (from.isNull(fromIndex)) {
       BitVectorHelper.setValidityBit(this.getValidityBuffer(), thisIndex, 0);
     } else {
@@ -827,7 +829,7 @@ public abstract class BaseFixedWidthVector extends BaseValueVector
   }
 
   /**
-   * Same as {@link #copyFrom(int, int, BaseFixedWidthVector)} except that
+   * Same as {@link #copyFrom(int, int, ValueVector)} except that
    * it handles the case when the capacity of the vector needs to be expanded
    * before copy.
    *
@@ -835,7 +837,8 @@ public abstract class BaseFixedWidthVector extends BaseValueVector
    * @param thisIndex position to copy to in this vector
    * @param from      source vector
    */
-  public void copyFromSafe(int fromIndex, int thisIndex, BaseFixedWidthVector from) {
+  public void copyFromSafe(int fromIndex, int thisIndex, ValueVector from) {
+    Preconditions.checkArgument(this.getMinorType() == from.getMinorType());
     handleSafe(thisIndex);
     copyFrom(fromIndex, thisIndex, from);
   }
diff --git a/java/vector/src/main/java/org/apache/arrow/vector/BaseValueVector.java b/java/vector/src/main/java/org/apache/arrow/vector/BaseValueVector.java
index fc8e2e7..166af07 100644
--- a/java/vector/src/main/java/org/apache/arrow/vector/BaseValueVector.java
+++ b/java/vector/src/main/java/org/apache/arrow/vector/BaseValueVector.java
@@ -213,5 +213,15 @@ public abstract class BaseValueVector implements ValueVector {
     final ReferenceManager referenceManager = srcBuffer.getReferenceManager();
     return referenceManager.transferOwnership(srcBuffer, targetAllocator).getTransferredBuffer();
   }
+
+  @Override
+  public void copyFrom(int fromIndex, int thisIndex, ValueVector from) {
+    throw new UnsupportedOperationException();
+  }
+
+  @Override
+  public void copyFromSafe(int fromIndex, int thisIndex, ValueVector from) {
+    throw new UnsupportedOperationException();
+  }
 }
 
diff --git a/java/vector/src/main/java/org/apache/arrow/vector/BaseVariableWidthVector.java b/java/vector/src/main/java/org/apache/arrow/vector/BaseVariableWidthVector.java
index 4386e3b..5bb54b9 100644
--- a/java/vector/src/main/java/org/apache/arrow/vector/BaseVariableWidthVector.java
+++ b/java/vector/src/main/java/org/apache/arrow/vector/BaseVariableWidthVector.java
@@ -27,6 +27,7 @@ import org.apache.arrow.memory.BufferAllocator;
 import org.apache.arrow.memory.OutOfMemoryException;
 import org.apache.arrow.memory.util.ArrowBufPointer;
 import org.apache.arrow.memory.util.ByteFunctionHelpers;
+import org.apache.arrow.util.Preconditions;
 import org.apache.arrow.vector.ipc.message.ArrowFieldNode;
 import org.apache.arrow.vector.types.pojo.Field;
 import org.apache.arrow.vector.util.CallBack;
@@ -1288,27 +1289,28 @@ public abstract class BaseVariableWidthVector extends BaseValueVector
    * @param thisIndex position to copy to in this vector
    * @param from source vector
    */
-  public void copyFrom(int fromIndex, int thisIndex, BaseVariableWidthVector from) {
+  public void copyFrom(int fromIndex, int thisIndex, ValueVector from) {
+    Preconditions.checkArgument(this.getMinorType() == from.getMinorType());
     if (from.isNull(fromIndex)) {
       fillHoles(thisIndex);
       BitVectorHelper.setValidityBit(this.validityBuffer, thisIndex, 0);
       final int copyStart = offsetBuffer.getInt(thisIndex * OFFSET_WIDTH);
       offsetBuffer.setInt((thisIndex + 1) * OFFSET_WIDTH, copyStart);
     } else {
-      final int start = from.offsetBuffer.getInt(fromIndex * OFFSET_WIDTH);
-      final int end = from.offsetBuffer.getInt((fromIndex + 1) * OFFSET_WIDTH);
+      final int start = from.getOffsetBuffer().getInt(fromIndex * OFFSET_WIDTH);
+      final int end = from.getOffsetBuffer().getInt((fromIndex + 1) * OFFSET_WIDTH);
       final int length = end - start;
       fillHoles(thisIndex);
       BitVectorHelper.setValidityBit(this.validityBuffer, thisIndex, 1);
       final int copyStart = offsetBuffer.getInt(thisIndex * OFFSET_WIDTH);
-      from.valueBuffer.getBytes(start, this.valueBuffer, copyStart, length);
+      from.getDataBuffer().getBytes(start, this.valueBuffer, copyStart, length);
       offsetBuffer.setInt((thisIndex + 1) * OFFSET_WIDTH, copyStart + length);
     }
     lastSet = thisIndex;
   }
 
   /**
-   * Same as {@link #copyFrom(int, int, BaseVariableWidthVector)} except that
+   * Same as {@link #copyFrom(int, int, ValueVector)} except that
    * it handles the case when the capacity of the vector needs to be expanded
    * before copy.
    *
@@ -1316,7 +1318,8 @@ public abstract class BaseVariableWidthVector extends BaseValueVector
    * @param thisIndex position to copy to in this vector
    * @param from source vector
    */
-  public void copyFromSafe(int fromIndex, int thisIndex, BaseVariableWidthVector from) {
+  public void copyFromSafe(int fromIndex, int thisIndex, ValueVector from) {
+    Preconditions.checkArgument(this.getMinorType() == from.getMinorType());
     if (from.isNull(fromIndex)) {
       handleSafe(thisIndex, 0);
       fillHoles(thisIndex);
@@ -1324,14 +1327,14 @@ public abstract class BaseVariableWidthVector extends BaseValueVector
       final int copyStart = offsetBuffer.getInt(thisIndex * OFFSET_WIDTH);
       offsetBuffer.setInt((thisIndex + 1) * OFFSET_WIDTH, copyStart);
     } else {
-      final int start = from.offsetBuffer.getInt(fromIndex * OFFSET_WIDTH);
-      final int end = from.offsetBuffer.getInt((fromIndex + 1) * OFFSET_WIDTH);
+      final int start = from.getOffsetBuffer().getInt(fromIndex * OFFSET_WIDTH);
+      final int end = from.getOffsetBuffer().getInt((fromIndex + 1) * OFFSET_WIDTH);
       final int length = end - start;
       handleSafe(thisIndex, length);
       fillHoles(thisIndex);
       BitVectorHelper.setValidityBit(this.validityBuffer, thisIndex, 1);
       final int copyStart = offsetBuffer.getInt(thisIndex * OFFSET_WIDTH);
-      from.valueBuffer.getBytes(start, this.valueBuffer, copyStart, length);
+      from.getDataBuffer().getBytes(start, this.valueBuffer, copyStart, length);
       offsetBuffer.setInt((thisIndex + 1) * OFFSET_WIDTH, copyStart + length);
     }
     lastSet = thisIndex;
diff --git a/java/vector/src/main/java/org/apache/arrow/vector/BitVector.java b/java/vector/src/main/java/org/apache/arrow/vector/BitVector.java
index ff4504f..b1b10f9 100644
--- a/java/vector/src/main/java/org/apache/arrow/vector/BitVector.java
+++ b/java/vector/src/main/java/org/apache/arrow/vector/BitVector.java
@@ -20,6 +20,7 @@ package org.apache.arrow.vector;
 import static org.apache.arrow.vector.NullCheckingForGet.NULL_CHECKING_ENABLED;
 
 import org.apache.arrow.memory.BufferAllocator;
+import org.apache.arrow.util.Preconditions;
 import org.apache.arrow.vector.complex.impl.BitReaderImpl;
 import org.apache.arrow.vector.complex.reader.FieldReader;
 import org.apache.arrow.vector.holders.BitHolder;
@@ -297,9 +298,15 @@ public class BitVector extends BaseFixedWidthVector {
    * @param from      source vector
    */
   @Override
-  public void copyFrom(int fromIndex, int thisIndex, BaseFixedWidthVector from) {
-    BitVectorHelper.setValidityBit(validityBuffer, thisIndex, from.isSet(fromIndex));
-    BitVectorHelper.setValidityBit(valueBuffer, thisIndex, ((BitVector) from).getBit(fromIndex));
+  public void copyFrom(int fromIndex, int thisIndex, ValueVector from) {
+    Preconditions.checkArgument(this.getMinorType() == from.getMinorType());
+    boolean fromIsSet = BitVectorHelper.get(from.getValidityBuffer(), fromIndex) != 0;
+    if (fromIsSet) {
+      BitVectorHelper.setValidityBit(validityBuffer, thisIndex, 1);
+      BitVectorHelper.setValidityBit(valueBuffer, thisIndex, ((BitVector) from).getBit(fromIndex));
+    } else {
+      BitVectorHelper.setValidityBit(validityBuffer, thisIndex, 0);
+    }
   }
 
   /*----------------------------------------------------------------*
diff --git a/java/vector/src/main/java/org/apache/arrow/vector/ValueVector.java b/java/vector/src/main/java/org/apache/arrow/vector/ValueVector.java
index 795493a..4d98397 100644
--- a/java/vector/src/main/java/org/apache/arrow/vector/ValueVector.java
+++ b/java/vector/src/main/java/org/apache/arrow/vector/ValueVector.java
@@ -251,4 +251,25 @@ public interface ValueVector extends Closeable, Iterable<ValueVector> {
    * @return true if equals, otherwise false.
    */
   boolean equals(int index, ValueVector target, int targetIndex);
+
+  /**
+   * Copy a cell value from a particular index in source vector to a particular
+   * position in this vector.
+   *
+   * @param fromIndex position to copy from in source vector
+   * @param thisIndex position to copy to in this vector
+   * @param from      source vector
+   */
+  void copyFrom(int fromIndex, int thisIndex, ValueVector from);
+
+  /**
+   * Same as {@link #copyFrom(int, int, ValueVector)} except that
+   * it handles the case when the capacity of the vector needs to be expanded
+   * before copy.
+   *
+   * @param fromIndex position to copy from in source vector
+   * @param thisIndex position to copy to in this vector
+   * @param from      source vector
+   */
+  void copyFromSafe(int fromIndex, int thisIndex, ValueVector from);
 }
diff --git a/java/vector/src/main/java/org/apache/arrow/vector/ZeroVector.java b/java/vector/src/main/java/org/apache/arrow/vector/ZeroVector.java
index b34aa4d..2a6e21c 100644
--- a/java/vector/src/main/java/org/apache/arrow/vector/ZeroVector.java
+++ b/java/vector/src/main/java/org/apache/arrow/vector/ZeroVector.java
@@ -254,4 +254,14 @@ public class ZeroVector implements FieldVector {
   public boolean equals(int index, ValueVector to, int toIndex) {
     return false;
   }
+
+  @Override
+  public void copyFrom(int fromIndex, int thisIndex, ValueVector from) {
+    throw new UnsupportedOperationException();
+  }
+
+  @Override
+  public void copyFromSafe(int fromIndex, int thisIndex, ValueVector from) {
+    throw new UnsupportedOperationException();
+  }
 }
diff --git a/java/vector/src/main/java/org/apache/arrow/vector/complex/AbstractContainerVector.java b/java/vector/src/main/java/org/apache/arrow/vector/complex/AbstractContainerVector.java
index 00c1b6d..c9e1e27 100644
--- a/java/vector/src/main/java/org/apache/arrow/vector/complex/AbstractContainerVector.java
+++ b/java/vector/src/main/java/org/apache/arrow/vector/complex/AbstractContainerVector.java
@@ -112,4 +112,14 @@ public abstract class AbstractContainerVector implements ValueVector, DensityAwa
   public UnionVector addOrGetUnion(String name) {
     return addOrGet(name, FieldType.nullable(MinorType.UNION.getType()), UnionVector.class);
   }
+
+  @Override
+  public void copyFrom(int fromIndex, int thisIndex, ValueVector from) {
+    throw new UnsupportedOperationException();
+  }
+
+  @Override
+  public void copyFromSafe(int fromIndex, int thisIndex, ValueVector from) {
+    throw new UnsupportedOperationException();
+  }
 }