ARROW-8239: [Java] fix param checks in splitAndTransfer method

Closes #6737 from pprudhvi/precondfix and squashes the following commits: fd439c7 <Prudhvi Porandla> add tests for all types d242827 <Krisztián Szűcs> ARROW-8239: fix param checks in splitAndTransfer method Lead-authored-by: Prudhvi <pprudhvi@dremio.com> Co-authored-by: Krisztián Szűcs <szucs.krisztian@gmail.com> Co-authored-by: Prudhvi Porandla <prudhvi.porandla@icloud.com> Signed-off-by: Praveen <praveen@dremio.com>
apache · Mar 27, 2020 · 81d61a6 · 81d61a6
1 parent 21bf3d8
commit 81d61a6
Show file tree

Hide file tree

Showing 8 changed files with 174 additions and 30 deletions.
diff --git a/java/vector/src/main/codegen/templates/UnionVector.java b/java/vector/src/main/codegen/templates/UnionVector.java
@@ -482,10 +482,8 @@ public void transfer() {
 
     @Override
     public void splitAndTransfer(int startIndex, int length) {
-      Preconditions.checkArgument(startIndex >= 0 && startIndex < valueCount,
-          "Invalid startIndex: %s", startIndex);
-      Preconditions.checkArgument(startIndex + length <= valueCount,
-          "Invalid length: %s", length);
+      Preconditions.checkArgument(startIndex >= 0 && length >= 0 && startIndex + length <= valueCount,
+          "Invalid parameters startIndex: %s, length: %s for valueCount: %s", startIndex, length, valueCount);
       to.clear();
       internalStructVectorTransferPair.splitAndTransfer(startIndex, length);
       final int startPoint = startIndex * TYPE_WIDTH;

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
@@ -584,10 +584,8 @@ public void transferTo(BaseFixedWidthVector target) {
    */
   public void splitAndTransferTo(int startIndex, int length,
                                  BaseFixedWidthVector target) {
-    Preconditions.checkArgument(startIndex >= 0 && startIndex < valueCount,
-        "Invalid startIndex: %s", startIndex);
-    Preconditions.checkArgument(startIndex + length <= valueCount,
-        "Invalid length: %s", length);
+    Preconditions.checkArgument(startIndex >= 0 && length >= 0 && startIndex + length <= valueCount,
+        "Invalid parameters startIndex: %s, length: %s for valueCount: %s", startIndex, length, valueCount);
     compareTypes(target, "splitAndTransferTo");
     target.clear();
     splitAndTransferValidityBuffer(startIndex, length, target);

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
@@ -714,10 +714,8 @@ public void transferTo(BaseVariableWidthVector target) {
    */
   public void splitAndTransferTo(int startIndex, int length,
                                  BaseVariableWidthVector target) {
-    Preconditions.checkArgument(startIndex >= 0 && startIndex < valueCount,
-        "Invalid startIndex: %s", startIndex);
-    Preconditions.checkArgument(startIndex + length <= valueCount,
-        "Invalid length: %s", length);
+    Preconditions.checkArgument(startIndex >= 0 && length >= 0 && startIndex + length <= valueCount,
+        "Invalid parameters startIndex: %s, length: %s for valueCount: %s", startIndex, length, valueCount);
     compareTypes(target, "splitAndTransferTo");
     target.clear();
     splitAndTransferValidityBuffer(startIndex, length, target);

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
@@ -164,10 +164,8 @@ public int getBufferSize() {
    * @param target     destination vector
    */
   public void splitAndTransferTo(int startIndex, int length, BaseFixedWidthVector target) {
-    Preconditions.checkArgument(startIndex >= 0 && startIndex < valueCount,
-        "Invalid startIndex: %s", startIndex);
-    Preconditions.checkArgument(startIndex + length <= valueCount,
-        "Invalid length: %s", length);
+    Preconditions.checkArgument(startIndex >= 0 && length >= 0 && startIndex + length <= valueCount,
+        "Invalid parameters startIndex: %s, length: %s for valueCount: %s", startIndex, length, valueCount);
     compareTypes(target, "splitAndTransferTo");
     target.clear();
     target.validityBuffer = splitAndTransferBuffer(startIndex, length, target,

diff --git a/java/vector/src/main/java/org/apache/arrow/vector/complex/FixedSizeListVector.java b/java/vector/src/main/java/org/apache/arrow/vector/complex/FixedSizeListVector.java
@@ -590,10 +590,8 @@ public void transfer() {
 
     @Override
     public void splitAndTransfer(int startIndex, int length) {
-      Preconditions.checkArgument(startIndex >= 0 && startIndex < valueCount,
-          "Invalid startIndex: %s", startIndex);
-      Preconditions.checkArgument(startIndex + length <= valueCount,
-          "Invalid length: %s", length);
+      Preconditions.checkArgument(startIndex >= 0 && length >= 0 && startIndex + length <= valueCount,
+          "Invalid parameters startIndex: %s, length: %s for valueCount: %s", startIndex, length, valueCount);
       final int startPoint = listSize * startIndex;
       final int sliceLength = listSize * length;
       to.clear();

diff --git a/java/vector/src/main/java/org/apache/arrow/vector/complex/ListVector.java b/java/vector/src/main/java/org/apache/arrow/vector/complex/ListVector.java
@@ -493,10 +493,8 @@ public void transfer() {
      */
     @Override
     public void splitAndTransfer(int startIndex, int length) {
-      Preconditions.checkArgument(startIndex >= 0 && startIndex < valueCount,
-          "Invalid startIndex: %s", startIndex);
-      Preconditions.checkArgument(startIndex + length <= valueCount,
-          "Invalid length: %s", length);
+      Preconditions.checkArgument(startIndex >= 0 && length >= 0 && startIndex + length <= valueCount,
+          "Invalid parameters startIndex: %s, length: %s for valueCount: %s", startIndex, length, valueCount);
       final int startPoint = offsetBuffer.getInt(startIndex * OFFSET_WIDTH);
       final int sliceLength = offsetBuffer.getInt((startIndex + length) * OFFSET_WIDTH) - startPoint;
       to.clear();

diff --git a/java/vector/src/main/java/org/apache/arrow/vector/complex/StructVector.java b/java/vector/src/main/java/org/apache/arrow/vector/complex/StructVector.java
@@ -195,10 +195,8 @@ public void copyValueSafe(int fromIndex, int toIndex) {
 
     @Override
     public void splitAndTransfer(int startIndex, int length) {
-      Preconditions.checkArgument(startIndex >= 0 && startIndex < valueCount,
-          "Invalid startIndex: %s", startIndex);
-      Preconditions.checkArgument(startIndex + length <= valueCount,
-          "Invalid length: %s", length);
+      Preconditions.checkArgument(startIndex >= 0 && length >= 0 && startIndex + length <= valueCount,
+          "Invalid parameters startIndex: %s, length: %s for valueCount: %s", startIndex, length, valueCount);
       target.clear();
       splitAndTransferValidityBuffer(startIndex, length, target);
       super.splitAndTransfer(startIndex, length);

diff --git a/java/vector/src/test/java/org/apache/arrow/vector/TestSplitAndTransfer.java b/java/vector/src/test/java/org/apache/arrow/vector/TestSplitAndTransfer.java
@@ -22,8 +22,17 @@
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 
+import java.util.HashMap;
+import java.util.Map;
+
 import org.apache.arrow.memory.BufferAllocator;
 import org.apache.arrow.memory.RootAllocator;
+import org.apache.arrow.vector.complex.FixedSizeListVector;
+import org.apache.arrow.vector.complex.ListVector;
+import org.apache.arrow.vector.complex.StructVector;
+import org.apache.arrow.vector.complex.UnionVector;
+import org.apache.arrow.vector.types.pojo.ArrowType.Struct;
+import org.apache.arrow.vector.types.pojo.FieldType;
 import org.apache.arrow.vector.util.TransferPair;
 import org.junit.After;
 import org.junit.Before;
@@ -223,7 +232,7 @@ public void testInvalidStartIndex() {
           IllegalArgumentException.class,
           () -> tp.splitAndTransfer(valueCount, 10));
 
-      assertEquals("Invalid startIndex: 500", e.getMessage());
+      assertEquals("Invalid parameters startIndex: 500, length: 10 for valueCount: 500", e.getMessage());
 
       newVarCharVector.clear();
     }
@@ -244,9 +253,158 @@ public void testInvalidLength() {
           IllegalArgumentException.class,
           () -> tp.splitAndTransfer(0, valueCount * 2));
 
-      assertEquals("Invalid length: 1000", e.getMessage());
+      assertEquals("Invalid parameters startIndex: 0, length: 1000 for valueCount: 500", e.getMessage());
+
+      newVarCharVector.clear();
+    }
+  }
+
+  @Test
+  public void testZeroStartIndexAndLength() {
+    try (final VarCharVector varCharVector = new VarCharVector("myvector", allocator);
+         final VarCharVector newVarCharVector = new VarCharVector("newvector", allocator)) {
+
+      varCharVector.allocateNew(0, 0);
+      final int valueCount = 0;
+      populateVarcharVector(varCharVector, valueCount, null);
+
+      final TransferPair tp = varCharVector.makeTransferPair(newVarCharVector);
+
+      tp.splitAndTransfer(0, 0);
+      assertEquals(valueCount, newVarCharVector.getValueCount());
+
+      newVarCharVector.clear();
+    }
+  }
+
+  @Test
+  public void testZeroLength() {
+    try (final VarCharVector varCharVector = new VarCharVector("myvector", allocator);
+         final VarCharVector newVarCharVector = new VarCharVector("newvector", allocator)) {
+
+      varCharVector.allocateNew(10000, 1000);
+      final int valueCount = 500;
+      populateVarcharVector(varCharVector, valueCount, null);
+
+      final TransferPair tp = varCharVector.makeTransferPair(newVarCharVector);
+
+      tp.splitAndTransfer(500, 0);
+      assertEquals(0, newVarCharVector.getValueCount());
 
       newVarCharVector.clear();
     }
   }
+
+  @Test
+  public void testUnionVectorZeroStartIndexAndLength() {
+    try (final UnionVector unionVector = UnionVector.empty("myvector", allocator);
+         final UnionVector newUnionVector = UnionVector.empty("newvector", allocator)) {
+
+      unionVector.allocateNew();
+      final int valueCount = 0;
+      unionVector.setValueCount(valueCount);
+
+      final TransferPair tp = unionVector.makeTransferPair(newUnionVector);
+
+      tp.splitAndTransfer(0, 0);
+      assertEquals(valueCount, newUnionVector.getValueCount());
+
+      newUnionVector.clear();
+    }
+  }
+
+  @Test
+  public void testFixedWidthVectorZeroStartIndexAndLength() {
+    try (final IntVector intVector = new IntVector("myvector", allocator);
+         final IntVector newIntVector = new IntVector("newvector", allocator)) {
+
+      intVector.allocateNew(0);
+      final int valueCount = 0;
+      intVector.setValueCount(valueCount);
+
+      final TransferPair tp = intVector.makeTransferPair(newIntVector);
+
+      tp.splitAndTransfer(0, 0);
+      assertEquals(valueCount, newIntVector.getValueCount());
+
+      newIntVector.clear();
+    }
+  }
+
+  @Test
+  public void testBitVectorZeroStartIndexAndLength() {
+    try (final BitVector bitVector = new BitVector("myvector", allocator);
+         final BitVector newBitVector = new BitVector("newvector", allocator)) {
+
+      bitVector.allocateNew(0);
+      final int valueCount = 0;
+      bitVector.setValueCount(valueCount);
+
+      final TransferPair tp = bitVector.makeTransferPair(newBitVector);
+
+      tp.splitAndTransfer(0, 0);
+      assertEquals(valueCount, newBitVector.getValueCount());
+
+      newBitVector.clear();
+    }
+  }
+
+  @Test
+  public void testFixedSizeListVectorZeroStartIndexAndLength() {
+    try (final FixedSizeListVector listVector = FixedSizeListVector.empty("list", 4, allocator);
+         final FixedSizeListVector newListVector = FixedSizeListVector.empty("newList", 4, allocator)) {
+
+      listVector.allocateNew();
+      final int valueCount = 0;
+      listVector.setValueCount(valueCount);
+
+      final TransferPair tp = listVector.makeTransferPair(newListVector);
+
+      tp.splitAndTransfer(0, 0);
+      assertEquals(valueCount, newListVector.getValueCount());
+
+      newListVector.clear();
+    }
+  }
+
+  @Test
+  public void testListVectorZeroStartIndexAndLength() {
+    try (final ListVector listVector = ListVector.empty("list", allocator);
+         final ListVector newListVector = ListVector.empty("newList", allocator)) {
+
+      listVector.allocateNew();
+      final int valueCount = 0;
+      listVector.setValueCount(valueCount);
+
+      final TransferPair tp = listVector.makeTransferPair(newListVector);
+
+      tp.splitAndTransfer(0, 0);
+      assertEquals(valueCount, newListVector.getValueCount());
+
+      newListVector.clear();
+    }
+  }
+
+  @Test
+  public void testStructVectorZeroStartIndexAndLength() {
+    Map<String, String> metadata = new HashMap<>();
+    metadata.put("k1", "v1");
+    FieldType type = new FieldType(true, Struct.INSTANCE, null, metadata);
+    try (final StructVector structVector = new StructVector("structvec", allocator, type, null);
+         final StructVector newStructVector = new StructVector("newStructvec", allocator, type, null)) {
+
+      structVector.allocateNew();
+      final int valueCount = 0;
+      structVector.setValueCount(valueCount);
+
+      final TransferPair tp = structVector.makeTransferPair(newStructVector);
+
+      tp.splitAndTransfer(0, 0);
+      assertEquals(valueCount, newStructVector.getValueCount());
+
+      newStructVector.clear();
+    }
+  }
+
+
 }