address review comments

apache · Nov 2, 2020 · a8c17fd · a8c17fd
1 parent 8f78959
commit a8c17fd
Show file tree

Hide file tree

Showing 3 changed files with 5 additions and 13 deletions.
diff --git a/java/vector/src/main/java/org/apache/arrow/vector/DecimalVector.java b/java/vector/src/main/java/org/apache/arrow/vector/DecimalVector.java
@@ -242,6 +242,7 @@ public void setBigEndian(int index, byte[] value) {
       }
     } else {
       if (length <= TYPE_WIDTH) {
+        // copy data from value to outAddress
         PlatformDependent.copyMemory(value, 0, outAddress + DecimalVector.TYPE_WIDTH - length, length);
         // sign extend
         final byte pad = (byte) (value[0] < 0 ? 0xFF : 0x00);

diff --git a/java/vector/src/main/java/org/apache/arrow/vector/util/DecimalUtility.java b/java/vector/src/main/java/org/apache/arrow/vector/util/DecimalUtility.java
@@ -132,6 +132,7 @@ public static void writeBigDecimalToArrowBuf(BigDecimal value, ArrowBuf bytebuf,
 
   /**
    * Write the given long to the ArrowBuf at the given value index.
+   * This routine extends the original sign bit to a new upper 64-bit in 128-bit.
    */
   public static void writeLongToArrowBuf(long value, ArrowBuf bytebuf, int index) {
     final long addressOfValue = bytebuf.memoryAddress() + (long) index * DECIMAL_BYTE_LENGTH;
@@ -160,6 +161,7 @@ private static void writeByteArrayToArrowBufHelper(byte[] bytes, ArrowBuf bytebu
       throw new UnsupportedOperationException("Decimal size greater than " + byteWidth + " bytes: " + bytes.length);
     }
 
+    byte [] padBytes = bytes[0] < 0 ? minus_one : zeroes;
     if (LITTLE_ENDIAN) {
       // Decimal stored as native-endian, need to swap data bytes before writing to ArrowBuf if LE
       byte[] bytesLE = new byte[bytes.length];
@@ -168,14 +170,12 @@ private static void writeByteArrayToArrowBufHelper(byte[] bytes, ArrowBuf bytebu
       }
 
       // Write LE data
-      byte [] padByes = bytes[0] < 0 ? minus_one : zeroes;
       bytebuf.setBytes(startIndex, bytesLE, 0, bytes.length);
-      bytebuf.setBytes(startIndex + bytes.length, padByes, 0, byteWidth - bytes.length);
+      bytebuf.setBytes(startIndex + bytes.length, padBytes, 0, byteWidth - bytes.length);
     } else {
       // Write BE data
-      byte [] padByes = bytes[0] < 0 ? minus_one : zeroes;
       bytebuf.setBytes(startIndex + byteWidth - bytes.length, bytes, 0, bytes.length);
-      bytebuf.setBytes(startIndex, padByes, 0, byteWidth - bytes.length);
+      bytebuf.setBytes(startIndex, padBytes, 0, byteWidth - bytes.length);
     }
   }
 }
diff --git a/java/vector/src/test/java/org/apache/arrow/vector/util/DecimalUtilityTest.java b/java/vector/src/test/java/org/apache/arrow/vector/util/DecimalUtilityTest.java
@@ -45,15 +45,6 @@ public void testSetLongInDecimalArrowBuf() {
         BigDecimal expected = BigDecimal.valueOf(val);
         Assert.assertEquals(expected, actual);
       }
-
-      long [] longValues = new long[] {Long.MIN_VALUE, 0 , Long.MAX_VALUE};
-      for (long val : longValues) {
-        buf.clear();
-        DecimalUtility.writeLongToArrowBuf(val, buf, 0);
-        BigDecimal actual = DecimalUtility.getBigDecimalFromArrowBuf(buf, 0, 0);
-        BigDecimal expected = BigDecimal.valueOf(val);
-        Assert.assertEquals(expected, actual);
-      }
     }
   }