[ARROW-5583][Java] When the isSet of a NullableValueHolder is 0, the …

…buffer field should not be used
apache · Jun 13, 2019 · 40c1548 · 40c1548
1 parent 25b4a46
commit 40c1548
Show file tree

Hide file tree

Showing 3 changed files with 70 additions and 4 deletions.
diff --git a/java/vector/src/main/java/org/apache/arrow/vector/VarBinaryVector.java b/java/vector/src/main/java/org/apache/arrow/vector/VarBinaryVector.java
@@ -242,10 +242,12 @@ public void set(int index, NullableVarBinaryHolder holder) {
     assert index >= 0;
     fillHoles(index);
     BitVectorHelper.setValidityBit(validityBuffer, index, holder.isSet);
-    final int dataLength = holder.end - holder.start;
+    final int dataLength = holder.isSet == 0 ? 0 : holder.end - holder.start;
     final int startOffset = getStartOffset(index);
     offsetBuffer.setInt((index + 1) * OFFSET_WIDTH, startOffset + dataLength);
-    valueBuffer.setBytes(startOffset, holder.buffer, holder.start, dataLength);
+    if (holder.isSet != 0) {
+      valueBuffer.setBytes(startOffset, holder.buffer, holder.start, dataLength);
+    }
     lastSet = index;
   }
 

diff --git a/java/vector/src/main/java/org/apache/arrow/vector/VarCharVector.java b/java/vector/src/main/java/org/apache/arrow/vector/VarCharVector.java
@@ -241,10 +241,12 @@ public void set(int index, NullableVarCharHolder holder) {
     assert index >= 0;
     fillHoles(index);
     BitVectorHelper.setValidityBit(validityBuffer, index, holder.isSet);
-    final int dataLength = holder.end - holder.start;
+    final int dataLength = holder.isSet == 0 ? 0 : holder.end - holder.start;
     final int startOffset = getStartOffset(index);
     offsetBuffer.setInt((index + 1) * OFFSET_WIDTH, startOffset + dataLength);
-    valueBuffer.setBytes(startOffset, holder.buffer, holder.start, dataLength);
+    if (holder.isSet != 0) {
+      valueBuffer.setBytes(startOffset, holder.buffer, holder.start, dataLength);
+    }
     lastSet = index;
   }
 

diff --git a/java/vector/src/test/java/org/apache/arrow/vector/TestValueVector.java b/java/vector/src/test/java/org/apache/arrow/vector/TestValueVector.java
@@ -35,6 +35,8 @@
 import org.apache.arrow.memory.BaseAllocator;
 import org.apache.arrow.memory.BufferAllocator;
 import org.apache.arrow.memory.RootAllocator;
+import org.apache.arrow.vector.holders.NullableVarBinaryHolder;
+import org.apache.arrow.vector.holders.NullableVarCharHolder;
 import org.apache.arrow.vector.ipc.message.ArrowRecordBatch;
 import org.apache.arrow.vector.types.Types.MinorType;
 import org.apache.arrow.vector.types.pojo.ArrowType;
@@ -2051,4 +2053,64 @@ public void testDefaultAllocNewAll() {
 
     }
   }
+
+  @Test
+  public void testSetNullableVarCharHolder() {
+    try (VarCharVector vector = new VarCharVector("", allocator)) {
+      vector.allocateNew(100, 10);
+
+      NullableVarCharHolder nullHolder = new NullableVarCharHolder();
+      nullHolder.isSet = 0;
+
+      NullableVarCharHolder stringHolder = new NullableVarCharHolder();
+      stringHolder.isSet = 1;
+
+      String str = "hello";
+      ArrowBuf buf = allocator.buffer(16);
+      buf.setBytes(0, str.getBytes());
+
+      stringHolder.start = 0;
+      stringHolder.end = str.length();
+      stringHolder.buffer = buf;
+
+      vector.set(0, nullHolder);
+      vector.set(1, stringHolder);
+
+      // verify results
+      assertTrue(vector.isNull(0));
+      assertEquals(str, new String(vector.get(1)));
+
+      buf.close();
+    }
+  }
+
+  @Test
+  public void testSetNullableVarBinaryHolder() {
+    try (VarBinaryVector vector = new VarBinaryVector("", allocator)) {
+      vector.allocateNew(100, 10);
+
+      NullableVarBinaryHolder nullHolder = new NullableVarBinaryHolder();
+      nullHolder.isSet = 0;
+
+      NullableVarBinaryHolder binHolder = new NullableVarBinaryHolder();
+      binHolder.isSet = 1;
+
+      String str = "hello";
+      ArrowBuf buf = allocator.buffer(16);
+      buf.setBytes(0, str.getBytes());
+
+      binHolder.start = 0;
+      binHolder.end = str.length();
+      binHolder.buffer = buf;
+
+      vector.set(0, nullHolder);
+      vector.set(1, binHolder);
+
+      // verify results
+      assertTrue(vector.isNull(0));
+      assertEquals(str, new String(vector.get(1)));
+
+      buf.close();
+    }
+  }
 }