BryanCutler commented on a change in pull request #8056:
URL: https://github.com/apache/arrow/pull/8056#discussion_r516276482
##########
File path:
java/vector/src/main/java/org/apache/arrow/vector/util/DecimalUtility.java
##########
@@ -128,12 +131,26 @@ 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 area in 128-bit
or 256-bit.
*/
- public static void writeLongToArrowBuf(long value, ArrowBuf bytebuf, int
index) {
- final long addressOfValue = bytebuf.memoryAddress() + (long) index *
DECIMAL_BYTE_LENGTH;
- PlatformDependent.putLong(addressOfValue, value);
+ public static void writeLongToArrowBuf(long value, ArrowBuf bytebuf, int
index, int byteWidth) {
+ if (byteWidth != 16 && byteWidth != 32) {
+ throw new
UnsupportedOperationException("DeciimalUtility.writeLongToArrowBuf() currently
supports " +
Review comment:
typo: `DeciimalUtility` -> `DecimalUtility`
##########
File path:
java/vector/src/test/java/org/apache/arrow/vector/complex/impl/TestComplexCopier.java
##########
@@ -526,7 +526,7 @@ public void testCopyFixedSizedListOfDecimalsVector() {
to.addOrGetVector(FieldType.nullable(new ArrowType.Decimal(3, 0, 128)));
DecimalHolder holder = new DecimalHolder();
- holder.buffer = allocator.buffer(DecimalUtility.DECIMAL_BYTE_LENGTH);
+ holder.buffer = allocator.buffer(16);
Review comment:
Could you use `DecimalVector.TYPE_WIDTH` here?
##########
File path:
java/vector/src/test/java/org/apache/arrow/vector/complex/writer/TestComplexWriter.java
##########
@@ -310,7 +310,7 @@ public void listDecimalType() {
listVector.allocateNew();
UnionListWriter listWriter = new UnionListWriter(listVector);
DecimalHolder holder = new DecimalHolder();
- holder.buffer = allocator.buffer(DecimalUtility.DECIMAL_BYTE_LENGTH);
+ holder.buffer = allocator.buffer(16);
Review comment:
Same here
##########
File path: java/vector/src/main/java/org/apache/arrow/vector/DecimalVector.java
##########
@@ -218,25 +220,37 @@ public void setBigEndian(int index, byte[] value) {
valueBuffer.checkBytes((long) index * TYPE_WIDTH, (long) (index + 1) *
TYPE_WIDTH);
long outAddress = valueBuffer.memoryAddress() + (long) index * TYPE_WIDTH;
- // swap bytes to convert BE to LE
- for (int byteIdx = 0; byteIdx < length; ++byteIdx) {
- PlatformDependent.putByte(outAddress + byteIdx, value[length - 1 -
byteIdx]);
- }
-
- if (length == TYPE_WIDTH) {
- return;
- }
-
if (length == 0) {
PlatformDependent.setMemory(outAddress, DecimalVector.TYPE_WIDTH, (byte)
0);
- } else if (length < TYPE_WIDTH) {
- // sign extend
- final byte pad = (byte) (value[0] < 0 ? 0xFF : 0x00);
- PlatformDependent.setMemory(outAddress + length,
DecimalVector.TYPE_WIDTH - length, pad);
+ return;
+ }
+ if (LITTLE_ENDIAN) {
+ // swap bytes to convert BE to LE
+ for (int byteIdx = 0; byteIdx < length; ++byteIdx) {
+ PlatformDependent.putByte(outAddress + byteIdx, value[length - 1 -
byteIdx]);
+ }
+
+ if (length == TYPE_WIDTH) {
+ return;
+ }
+
+ if (length < TYPE_WIDTH) {
+ // sign extend
+ final byte pad = (byte) (value[0] < 0 ? 0xFF : 0x00);
+ PlatformDependent.setMemory(outAddress + length,
DecimalVector.TYPE_WIDTH - length, pad);
+ return;
+ }
} else {
- throw new IllegalArgumentException(
- "Invalid decimal value length. Valid length in [1 - 16], got " +
length);
+ if (length <= TYPE_WIDTH) {
Review comment:
ok, I see above it is already set during the swap. I guess there is no
harm in calling `PlatformDependent.setMemory(outAddress,
DecimalVector.TYPE_WIDTH - length, pad)` if `length == TYPE_WIDTH`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
