Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 4.3k
ARROW-9861: [Java] Support big-endian in DecimalVector#8056
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
1db584c0d696faaa14a87d62d990ff33c3cd9e9ce9File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -20,6 +20,7 @@ | ||
| import static org.apache.arrow.vector.NullCheckingForGet.NULL_CHECKING_ENABLED; | ||
| import java.math.BigDecimal; | ||
| import java.nio.ByteOrder; | ||
| import org.apache.arrow.memory.ArrowBuf; | ||
| import org.apache.arrow.memory.BufferAllocator; | ||
| @@ -43,6 +44,7 @@ | ||
| */ | ||
| public final class DecimalVector extends BaseFixedWidthVector { | ||
| public static final byte TYPE_WIDTH = 16; | ||
| private static final boolean LITTLE_ENDIAN = ByteOrder.nativeOrder() == ByteOrder.LITTLE_ENDIAN; | ||
| private final FieldReader reader; | ||
| private final int precision; | ||
| @@ -197,7 +199,7 @@ public void set(int index, ArrowBuf buffer) { | ||
| /** | ||
| * Set the decimal element at given index to the provided array of bytes. | ||
| * Decimal is now implemented as Little Endian. This API allows the user | ||
| * Decimal is now implemented as Native Endian. This API allows the user | ||
| * to pass a decimal value in the form of byte array in BE byte order. | ||
| * | ||
| * <p>Consumers of Arrow code can use this API instead of first swapping | ||
| @@ -218,25 +220,38 @@ 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) { | ||
| // 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); | ||
| PlatformDependent.setMemory(outAddress, DecimalVector.TYPE_WIDTH - length, pad); | ||
| return; | ||
| } | ||
| } | ||
| throw new IllegalArgumentException( | ||
| "Invalid decimal value length. Valid length in [1 - 16], got " + length); | ||
Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should probably be in a | ||
| } | ||
| /** | ||
| @@ -255,7 +270,7 @@ public void set(int index, long start, ArrowBuf buffer) { | ||
| * Sets the element at given index using the buffer whose size maybe <= 16 bytes. | ||
| * @param index index to write the decimal to | ||
| * @param start start of value in the buffer | ||
| * @param buffer contains the decimal in little endian bytes | ||
| * @param buffer contains the decimal in native endian bytes | ||
| * @param length length of the value in the buffer | ||
| */ | ||
| public void setSafe(int index, long start, ArrowBuf buffer, int length) { | ||
| @@ -268,12 +283,22 @@ public void setSafe(int index, long start, ArrowBuf buffer, int length) { | ||
| long inAddress = buffer.memoryAddress() + start; | ||
| long outAddress = valueBuffer.memoryAddress() + (long) index * TYPE_WIDTH; | ||
| PlatformDependent.copyMemory(inAddress, outAddress, length); | ||
| // sign extend | ||
| if (length < 16) { | ||
| byte msb = PlatformDependent.getByte(inAddress + length - 1); | ||
| final byte pad = (byte) (msb < 0 ? 0xFF : 0x00); | ||
| PlatformDependent.setMemory(outAddress + length, DecimalVector.TYPE_WIDTH - length, pad); | ||
| if (LITTLE_ENDIAN) { | ||
| PlatformDependent.copyMemory(inAddress, outAddress, length); | ||
| // sign extend | ||
| if (length < TYPE_WIDTH) { | ||
| byte msb = PlatformDependent.getByte(inAddress + length - 1); | ||
| final byte pad = (byte) (msb < 0 ? 0xFF : 0x00); | ||
| PlatformDependent.setMemory(outAddress + length, DecimalVector.TYPE_WIDTH - length, pad); | ||
| } | ||
| } else { | ||
| PlatformDependent.copyMemory(inAddress, outAddress + DecimalVector.TYPE_WIDTH - length, length); | ||
| // sign extend | ||
| if (length < TYPE_WIDTH) { | ||
| byte msb = PlatformDependent.getByte(inAddress); | ||
| final byte pad = (byte) (msb < 0 ? 0xFF : 0x00); | ||
| PlatformDependent.setMemory(outAddress, DecimalVector.TYPE_WIDTH - length, pad); | ||
| } | ||
| } | ||
| } | ||
| @@ -296,16 +321,26 @@ public void setBigEndianSafe(int index, long start, ArrowBuf buffer, int length) | ||
| // not using buffer.getByte() to avoid boundary checks for every byte. | ||
| long inAddress = buffer.memoryAddress() + start; | ||
| long outAddress = valueBuffer.memoryAddress() + (long) index * TYPE_WIDTH; | ||
| // swap bytes to convert BE to LE | ||
| for (int byteIdx = 0; byteIdx < length; ++byteIdx) { | ||
| byte val = PlatformDependent.getByte((inAddress + length - 1) - byteIdx); | ||
| PlatformDependent.putByte(outAddress + byteIdx, val); | ||
| } | ||
| // sign extend | ||
| if (length < 16) { | ||
| byte msb = PlatformDependent.getByte(inAddress); | ||
| final byte pad = (byte) (msb < 0 ? 0xFF : 0x00); | ||
| PlatformDependent.setMemory(outAddress + length, DecimalVector.TYPE_WIDTH - length, pad); | ||
| if (LITTLE_ENDIAN) { | ||
| // swap bytes to convert BE to LE | ||
| for (int byteIdx = 0; byteIdx < length; ++byteIdx) { | ||
| byte val = PlatformDependent.getByte((inAddress + length - 1) - byteIdx); | ||
| PlatformDependent.putByte(outAddress + byteIdx, val); | ||
| } | ||
| // sign extend | ||
| if (length < TYPE_WIDTH) { | ||
| byte msb = PlatformDependent.getByte(inAddress); | ||
| final byte pad = (byte) (msb < 0 ? 0xFF : 0x00); | ||
| PlatformDependent.setMemory(outAddress + length, DecimalVector.TYPE_WIDTH - length, pad); | ||
| } | ||
| } else { | ||
| PlatformDependent.copyMemory(inAddress, outAddress + DecimalVector.TYPE_WIDTH - length, length); | ||
| // sign extend | ||
| if (length < TYPE_WIDTH) { | ||
| byte msb = PlatformDependent.getByte(inAddress); | ||
| final byte pad = (byte) (msb < 0 ? 0xFF : 0x00); | ||
| PlatformDependent.setMemory(outAddress, DecimalVector.TYPE_WIDTH - length, pad); | ||
| } | ||
| } | ||
| } | ||
| @@ -329,7 +364,7 @@ public void set(int index, BigDecimal value) { | ||
| */ | ||
| public void set(int index, long value) { | ||
| BitVectorHelper.setBit(validityBuffer, index); | ||
| DecimalUtility.writeLongToArrowBuf(value, valueBuffer, index); | ||
| DecimalUtility.writeLongToArrowBuf(value, valueBuffer, index, TYPE_WIDTH); | ||
| } | ||
| /** | ||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not
returniflength == TYPE_WIDTH?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we add
if (length == TYPE_WIDTH) returnbefore line 244, no data is copied fromvaluetooutAddress. I will add a commentcopy data from value to outAddressafter line 244.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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)iflength == TYPE_WIDTH