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 0e2a641 ARROW-5860: [Java][Vector] Fix decimal utils to handle
negative values.
0e2a641 is described below
commit 0e2a641e500f8af394757976ffe5537d458d0ea4
Author: Praveen <prav...@dremio.com>
AuthorDate: Mon Jul 8 11:48:05 2019 +0530
ARROW-5860: [Java][Vector] Fix decimal utils to handle negative values.
- Padvalue was defaulted to zero
- This incorrectly writes negative numbers to buffers.
Author: Praveen <prav...@dremio.com>
Closes #4811 from praveenbingo/ARROW-5860 and squashes the following
commits:
9c529d5d5 <Praveen> ARROW-5860: Fix decimal utils method to handle
negative numbers correctly.
---
.../apache/arrow/vector/util/DecimalUtility.java | 27 ++----
.../arrow/vector/util/DecimalUtilityTest.java | 104 +++++++++++++++++++++
2 files changed, 113 insertions(+), 18 deletions(-)
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
index d091152..a9f7a17 100644
--- 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
@@ -31,6 +31,8 @@ public class DecimalUtility {
private DecimalUtility() {}
public static final int DECIMAL_BYTE_LENGTH = 16;
+ public static final byte [] zeroes = new byte[]
{0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0};
+ public static final byte [] minus_one = new byte[]
{-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1};
/**
* Read an ArrowType.Decimal at the given value index in the ArrowBuf and
convert to a BigDecimal
@@ -100,8 +102,7 @@ public class DecimalUtility {
*/
public static void writeBigDecimalToArrowBuf(BigDecimal value, ArrowBuf
bytebuf, int index) {
final byte[] bytes = value.unscaledValue().toByteArray();
- final int padValue = value.signum() == -1 ? 0xFF : 0;
- writeByteArrayToArrowBuf(bytes, bytebuf, index, padValue);
+ writeByteArrayToArrowBufHelper(bytes, bytebuf, index);
}
/**
@@ -120,10 +121,10 @@ public class DecimalUtility {
* width.
*/
public static void writeByteArrayToArrowBuf(byte[] bytes, ArrowBuf bytebuf,
int index) {
- writeByteArrayToArrowBuf(bytes, bytebuf, index, 0);
+ writeByteArrayToArrowBufHelper(bytes, bytebuf, index);
}
- private static void writeByteArrayToArrowBuf(byte[] bytes, ArrowBuf bytebuf,
int index, int padValue) {
+ private static void writeByteArrayToArrowBufHelper(byte[] bytes, ArrowBuf
bytebuf, int index) {
final int startIndex = index * DECIMAL_BYTE_LENGTH;
if (bytes.length > DECIMAL_BYTE_LENGTH) {
throw new UnsupportedOperationException("Decimal size greater than 16
bytes");
@@ -131,23 +132,13 @@ public class DecimalUtility {
// Decimal stored as little endian, need to swap data bytes before writing
to ArrowBuf
byte[] bytesLE = new byte[bytes.length];
- int stop = bytes.length / 2;
- for (int i = 0, j; i < stop; i++) {
- j = (bytes.length - 1) - i;
- bytesLE[i] = bytes[j];
- bytesLE[j] = bytes[i];
- }
- if (bytes.length % 2 != 0) {
- int i = (bytes.length / 2);
- bytesLE[i] = bytes[i];
+ for (int i = 0; i < bytes.length; i++) {
+ bytesLE[i] = bytes[bytes.length - 1 - i];
}
// Write LE data
+ byte [] padByes = bytes[0] < 0 ? minus_one : zeroes;
bytebuf.setBytes(startIndex, bytesLE, 0, bytes.length);
-
- // Write padding after data
- for (int i = bytes.length; i < DECIMAL_BYTE_LENGTH; i++) {
- bytebuf.setByte(startIndex + i, padValue);
- }
+ bytebuf.setBytes(startIndex + bytes.length, padByes, 0,
DECIMAL_BYTE_LENGTH - 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
new file mode 100644
index 0000000..1840b4e
--- /dev/null
+++
b/java/vector/src/test/java/org/apache/arrow/vector/util/DecimalUtilityTest.java
@@ -0,0 +1,104 @@
+/*
+ * 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 org.apache.arrow.vector.util;
+
+import java.math.BigDecimal;
+import java.math.BigInteger;
+
+import org.apache.arrow.memory.BufferAllocator;
+import org.apache.arrow.memory.RootAllocator;
+import org.junit.Assert;
+import org.junit.Test;
+
+import io.netty.buffer.ArrowBuf;
+
+public class DecimalUtilityTest {
+ private static final BigInteger MAX_BIG_INT =
java.math.BigInteger.valueOf(10).pow(38)
+ .subtract(java.math.BigInteger.ONE);
+ private static final BigDecimal MAX_DECIMAL = new
java.math.BigDecimal(MAX_BIG_INT, 0);
+ private static final BigInteger MIN_BIG_INT =
MAX_BIG_INT.multiply(BigInteger.valueOf(-1));
+ private static final BigDecimal MIN_DECIMAL = new
java.math.BigDecimal(MIN_BIG_INT, 0);
+
+ @Test
+ public void testSetByteArrayInDecimalArrowBuf() {
+ try (BufferAllocator allocator = new RootAllocator(128);
+ ArrowBuf buf = allocator.buffer(16);
+ ) {
+ int [] intValues = new int [] {Integer.MAX_VALUE, Integer.MIN_VALUE, 0};
+ for (int val : intValues) {
+ buf.clear();
+
DecimalUtility.writeByteArrayToArrowBuf(BigInteger.valueOf(val).toByteArray(),
buf, 0);
+ BigDecimal actual = DecimalUtility.getBigDecimalFromArrowBuf(buf, 0,
0);
+ 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.writeByteArrayToArrowBuf(BigInteger.valueOf(val).toByteArray(),
buf, 0);
+ BigDecimal actual = DecimalUtility.getBigDecimalFromArrowBuf(buf, 0,
0);
+ BigDecimal expected = BigDecimal.valueOf(val);
+ Assert.assertEquals(expected, actual);
+ }
+
+ BigInteger [] decimals = new BigInteger[] {MAX_BIG_INT, new
BigInteger("0"), MIN_BIG_INT};
+ for (BigInteger val : decimals) {
+ buf.clear();
+ DecimalUtility.writeByteArrayToArrowBuf(val.toByteArray(), buf, 0);
+ BigDecimal actual = DecimalUtility.getBigDecimalFromArrowBuf(buf, 0,
0);
+ BigDecimal expected = new BigDecimal(val);
+ Assert.assertEquals(expected, actual);
+ }
+ }
+ }
+
+ @Test
+ public void testSetBigDecimalInDecimalArrowBuf() {
+ try (BufferAllocator allocator = new RootAllocator(128);
+ ArrowBuf buf = allocator.buffer(16);
+ ) {
+ int [] intValues = new int [] {Integer.MAX_VALUE, Integer.MIN_VALUE, 0};
+ for (int val : intValues) {
+ buf.clear();
+ DecimalUtility.writeBigDecimalToArrowBuf(BigDecimal.valueOf(val), buf,
0);
+ BigDecimal actual = DecimalUtility.getBigDecimalFromArrowBuf(buf, 0,
0);
+ 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.writeBigDecimalToArrowBuf(BigDecimal.valueOf(val), buf,
0);
+ BigDecimal actual = DecimalUtility.getBigDecimalFromArrowBuf(buf, 0,
0);
+ BigDecimal expected = BigDecimal.valueOf(val);
+ Assert.assertEquals(expected, actual);
+ }
+
+ BigInteger [] decimals = new BigInteger[] {MAX_BIG_INT, new
BigInteger("0"), MIN_BIG_INT};
+ for (BigInteger val : decimals) {
+ buf.clear();
+ DecimalUtility.writeBigDecimalToArrowBuf(new BigDecimal(val), buf, 0);
+ BigDecimal actual = DecimalUtility.getBigDecimalFromArrowBuf(buf, 0,
0);
+ BigDecimal expected = new BigDecimal(val);
+ Assert.assertEquals(expected, actual);
+ }
+ }
+ }
+}
