arrow-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From w...@apache.org
Subject arrow git commit: ARROW-1443: [Java] Fixed a small bug on ArrowBuf.setBytes with unsliced ByteBuffers
Date Fri, 01 Sep 2017 17:57:11 GMT
Repository: arrow
Updated Branches:
  refs/heads/master 8344f28f1 -> 28553b4f1


ARROW-1443: [Java] Fixed a small bug on ArrowBuf.setBytes with unsliced ByteBuffers

There is a small bug on ArrowBuf at line 750. It says:
`udle.setBytes(index + offset, buf);`

But it should say:
`udle.setBytes(index + offset, newBuf);`

There is a wraparound: You can call the method with the already sliced buffer.

Author: Gonzalo Ortiz <golthiryus@gmail.com>
Author: Li Jin <ice.xelloss@gmail.com>

Closes #1022 from gortiz/master and squashes the following commits:

cdffbcc7 [Gonzalo Ortiz] Removed the old ArrowBufTest (which was renamed as TestArrowBuf)
d8aa00f5 [Gonzalo Ortiz] Renamed to ArrowBufTest to TestArrowBuf
1968a98e [Gonzalo Ortiz] Added license header
aa6c4579 [Li Jin] Add more tests for NullableVarChar and NullableVarBinary
e65e3804 [Gonzalo Ortiz] ARROW-1443 Reapplying the bug fix
41bcacc3 [Gonzalo Ortiz] ARROW-1443 Added a test to verify the bug
f70ceff9 [Gonzalo Ortiz] ARROW-1443 Reverted the previous commit to reproduce the bug
cef014f3 [Gonzalo Ortiz] [Java] Fixed a small bug on ArrowBuf.setBytes with unsliced ByteBuffers


Project: http://git-wip-us.apache.org/repos/asf/arrow/repo
Commit: http://git-wip-us.apache.org/repos/asf/arrow/commit/28553b4f
Tree: http://git-wip-us.apache.org/repos/asf/arrow/tree/28553b4f
Diff: http://git-wip-us.apache.org/repos/asf/arrow/diff/28553b4f

Branch: refs/heads/master
Commit: 28553b4f1e06cdcf8f88cc2ac0d9ae18913b59ab
Parents: 8344f28
Author: Gonzalo Ortiz <golthiryus@gmail.com>
Authored: Fri Sep 1 13:57:07 2017 -0400
Committer: Wes McKinney <wes.mckinney@twosigma.com>
Committed: Fri Sep 1 13:57:07 2017 -0400

----------------------------------------------------------------------
 .../src/main/java/io/netty/buffer/ArrowBuf.java |  2 +-
 .../test/java/io/netty/buffer/TestArrowBuf.java | 85 ++++++++++++++++++++
 .../java/org/apache/arrow/vector/TestUtils.java |  8 +-
 .../apache/arrow/vector/TestValueVector.java    | 57 ++++++++++++-
 4 files changed, 148 insertions(+), 4 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/arrow/blob/28553b4f/java/memory/src/main/java/io/netty/buffer/ArrowBuf.java
----------------------------------------------------------------------
diff --git a/java/memory/src/main/java/io/netty/buffer/ArrowBuf.java b/java/memory/src/main/java/io/netty/buffer/ArrowBuf.java
index 6d17430..e2bbe35 100644
--- a/java/memory/src/main/java/io/netty/buffer/ArrowBuf.java
+++ b/java/memory/src/main/java/io/netty/buffer/ArrowBuf.java
@@ -747,7 +747,7 @@ public final class ArrowBuf extends AbstractByteBuf implements AutoCloseable
{
         ByteBuffer newBuf = src.duplicate();
         newBuf.position(srcIndex);
         newBuf.limit(srcIndex + length);
-        udle.setBytes(index + offset, src);
+        udle.setBytes(index + offset, newBuf);
       }
     }
 

http://git-wip-us.apache.org/repos/asf/arrow/blob/28553b4f/java/memory/src/test/java/io/netty/buffer/TestArrowBuf.java
----------------------------------------------------------------------
diff --git a/java/memory/src/test/java/io/netty/buffer/TestArrowBuf.java b/java/memory/src/test/java/io/netty/buffer/TestArrowBuf.java
new file mode 100644
index 0000000..8853220
--- /dev/null
+++ b/java/memory/src/test/java/io/netty/buffer/TestArrowBuf.java
@@ -0,0 +1,85 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package io.netty.buffer;
+
+import java.nio.ByteBuffer;
+import java.util.Arrays;
+import org.apache.arrow.memory.RootAllocator;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import static org.junit.Assert.*;
+
+public class TestArrowBuf {
+
+  private final static int MAX_ALLOCATION = 8 * 1024;
+  private static RootAllocator allocator;
+  
+  @BeforeClass
+  public static void beforeClass() {
+    allocator = new RootAllocator(MAX_ALLOCATION);
+  }
+  
+  @AfterClass
+  public static void afterClass() {
+    if (allocator != null) {
+      allocator.close();
+    }
+  }
+  
+  @Test
+  public void testSetBytesSliced() {
+    int arrLength = 64;
+    byte[] expecteds = new byte[arrLength];
+    for (int i = 0; i < expecteds.length; i++) {
+      expecteds[i] = (byte) i;
+    }
+    ByteBuffer data = ByteBuffer.wrap(expecteds);
+    try (ArrowBuf buf = allocator.buffer(expecteds.length)) {
+      buf.setBytes(0, data, 0, data.capacity());
+      
+      byte[] actuals = new byte[expecteds.length];
+      buf.getBytes(0, actuals);
+      assertArrayEquals(expecteds, actuals);
+    }
+  }
+  
+  @Test
+  public void testSetBytesUnsliced() {
+    int arrLength = 64;
+    byte[] arr = new byte[arrLength];
+    for (int i = 0; i < arr.length; i++) {
+      arr[i] = (byte) i;
+    }
+    ByteBuffer data = ByteBuffer.wrap(arr);
+    
+    int from = 10;
+    int to = arrLength;
+    byte[] expecteds = Arrays.copyOfRange(arr, from, to);
+    try (ArrowBuf buf = allocator.buffer(expecteds.length)) {
+      buf.setBytes(0, data, from, to - from);
+      
+      byte[] actuals = new byte[expecteds.length];
+      buf.getBytes(0, actuals);
+      assertArrayEquals(expecteds, actuals);
+    }
+  }
+  
+}

http://git-wip-us.apache.org/repos/asf/arrow/blob/28553b4f/java/vector/src/test/java/org/apache/arrow/vector/TestUtils.java
----------------------------------------------------------------------
diff --git a/java/vector/src/test/java/org/apache/arrow/vector/TestUtils.java b/java/vector/src/test/java/org/apache/arrow/vector/TestUtils.java
index 15f5117..a148813 100644
--- a/java/vector/src/test/java/org/apache/arrow/vector/TestUtils.java
+++ b/java/vector/src/test/java/org/apache/arrow/vector/TestUtils.java
@@ -26,7 +26,13 @@ import org.apache.arrow.vector.types.pojo.FieldType;
 public class TestUtils {
 
   public static NullableVarCharVector newNullableVarCharVector(String name, BufferAllocator
allocator) {
-    return (NullableVarCharVector) FieldType.nullable(new ArrowType.Utf8()).createNewSingleVector(name,
allocator, null);
+    return (NullableVarCharVector)
+        FieldType.nullable(new ArrowType.Utf8()).createNewSingleVector(name, allocator, null);
+  }
+
+  public static NullableVarBinaryVector newNullableVarBinaryVector(String name, BufferAllocator
allocator) {
+    return (NullableVarBinaryVector)
+        FieldType.nullable(new ArrowType.Binary()).createNewSingleVector(name, allocator,
null);
   }
 
   public static <T> T newVector(Class<T> c, String name, ArrowType type, BufferAllocator
allocator) {

http://git-wip-us.apache.org/repos/asf/arrow/blob/28553b4f/java/vector/src/test/java/org/apache/arrow/vector/TestValueVector.java
----------------------------------------------------------------------
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
index cb1fa89..57119bf 100644
--- a/java/vector/src/test/java/org/apache/arrow/vector/TestValueVector.java
+++ b/java/vector/src/test/java/org/apache/arrow/vector/TestValueVector.java
@@ -17,8 +17,10 @@
  */
 
 package org.apache.arrow.vector;
+import org.apache.arrow.vector.holders.VarCharHolder;
 import org.apache.arrow.vector.util.OversizedAllocationException;
 
+import static org.apache.arrow.vector.TestUtils.newNullableVarBinaryVector;
 import static org.apache.arrow.vector.TestUtils.newNullableVarCharVector;
 import static org.apache.arrow.vector.TestUtils.newVector;
 import static org.junit.Assert.assertArrayEquals;
@@ -27,7 +29,9 @@ import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.assertFalse;
 
+import java.nio.ByteBuffer;
 import java.nio.charset.Charset;
+import java.util.Arrays;
 import java.util.List;
 import java.util.ArrayList;
 
@@ -787,12 +791,12 @@ public class TestValueVector {
    * Covered types as of now
    *
    *  -- NullableVarCharVector
+   *  -- NullableVarBinaryVector
    *
    * TODO:
    *
    *  -- VarCharVector
    *  -- VarBinaryVector
-   *  -- NullableVarBinaryVector
    */
 
   @Test /* NullableVarCharVector */
@@ -806,17 +810,26 @@ public class TestValueVector {
       m.set(0, STR1);
       m.set(1, STR2);
       m.set(2, STR3);
+      m.setSafe(3, STR3, 1, STR3.length - 1);
+      m.setSafe(4, STR3, 2, STR3.length - 2);
+      ByteBuffer STR3ByteBuffer = ByteBuffer.wrap(STR3);
+      m.setSafe(5, STR3ByteBuffer, 1, STR3.length - 1);
+      m.setSafe(6, STR3ByteBuffer, 2, STR3.length - 2);
 
       // Check the sample strings.
       final NullableVarCharVector.Accessor accessor = vector.getAccessor();
       assertArrayEquals(STR1, accessor.get(0));
       assertArrayEquals(STR2, accessor.get(1));
       assertArrayEquals(STR3, accessor.get(2));
+      assertArrayEquals(Arrays.copyOfRange(STR3, 1, STR3.length), accessor.get(3));
+      assertArrayEquals(Arrays.copyOfRange(STR3, 2, STR3.length), accessor.get(4));
+      assertArrayEquals(Arrays.copyOfRange(STR3, 1, STR3.length), accessor.get(5));
+      assertArrayEquals(Arrays.copyOfRange(STR3, 2, STR3.length), accessor.get(6));
 
       // Ensure null value throws.
       boolean b = false;
       try {
-        vector.getAccessor().get(3);
+        vector.getAccessor().get(7);
       } catch (IllegalStateException e) {
         b = true;
       } finally {
@@ -825,6 +838,46 @@ public class TestValueVector {
     }
   }
 
+  @Test /* NullableVarBinaryVector */
+  public void testNullableVarType2() {
+
+    // Create a new value vector for 1024 integers.
+    try (final NullableVarBinaryVector vector = newNullableVarBinaryVector(EMPTY_SCHEMA_PATH,
allocator)) {
+      final NullableVarBinaryVector.Mutator m = vector.getMutator();
+      vector.allocateNew(1024 * 10, 1024);
+
+      m.set(0, STR1);
+      m.set(1, STR2);
+      m.set(2, STR3);
+      m.setSafe(3, STR3, 1, STR3.length - 1);
+      m.setSafe(4, STR3, 2, STR3.length - 2);
+      ByteBuffer STR3ByteBuffer = ByteBuffer.wrap(STR3);
+      m.setSafe(5, STR3ByteBuffer, 1, STR3.length - 1);
+      m.setSafe(6, STR3ByteBuffer, 2, STR3.length - 2);
+
+      // Check the sample strings.
+      final NullableVarBinaryVector.Accessor accessor = vector.getAccessor();
+      assertArrayEquals(STR1, accessor.get(0));
+      assertArrayEquals(STR2, accessor.get(1));
+      assertArrayEquals(STR3, accessor.get(2));
+      assertArrayEquals(Arrays.copyOfRange(STR3, 1, STR3.length), accessor.get(3));
+      assertArrayEquals(Arrays.copyOfRange(STR3, 2, STR3.length), accessor.get(4));
+      assertArrayEquals(Arrays.copyOfRange(STR3, 1, STR3.length), accessor.get(5));
+      assertArrayEquals(Arrays.copyOfRange(STR3, 2, STR3.length), accessor.get(6));
+
+      // Ensure null value throws.
+      boolean b = false;
+      try {
+        vector.getAccessor().get(7);
+      } catch (IllegalStateException e) {
+        b = true;
+      } finally {
+        assertTrue(b);
+      }
+    }
+  }
+
+
   /*
    * generic tests
    *


Mime
View raw message