[
https://issues.apache.org/jira/browse/ARROW-6575?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17621431#comment-17621431
]
Jonathan Swenson edited comment on ARROW-6575 at 10/21/22 1:11 AM:
-------------------------------------------------------------------
Is this the correct method to use? It appears as though this still does not
support negative numbers, and perhaps also does not support non-zero scale.
>From the implementation in the c++ source, I believe there are two missing
>pieces.
+ handling of negative values. (determine if negative and negate before
rendering) – [determine if
negative|https://github.com/apache/arrow/blob/master/cpp/src/arrow/util/basic_decimal.h#L125]
[negation |
https://github.com/apache/arrow/blob/master/cpp/src/arrow/util/basic_decimal.cc#L377-L385]
+ scaling values appropriately (insert the decimal place / prepend leading
zeros as necessary).
[implementation|https://github.com/apache/arrow/blob/master/cpp/src/arrow/util/decimal.cc#L328-L386]
The conversion to integer string is implemented in c++
[here|https://github.com/apache/arrow/blob/master/cpp/src/arrow/util/decimal.cc#L685-L698]
which includes checking if negative before rendering.
I see values like:
+ 101 (with scale 10, precision 13) renders as `1010000000000` – where I
expect this as `101.0000000000`
+ -101 (with scale 10, precision 13) renders as
`340282366920938500000000000000000000000` - I expect this as `-101.0000000000`
I'm not really a Javascript expert, but a similar approach to the negation
check and flipping appears to work, but I'm fairly confident I'm missing some
edge cases.
General algorithm:
+ check to see if the high bits are negative
+ if so, number is negative (prepend with "-") and add 1 to each "chunk" and
handle overflows appropriately.
+ render the string using current implementation.
+ place decimal place (or prepend with 0s) based on the scale.
was (Author: jswenson):
Is this the correct method to use? It appears as though this still does not
support negative numbers, and perhaps also does not support non-zero scale.
>From the implementation in the c++ source, I believe there are two missing
>pieces.
+ handling of negative values. (determine if negative and negate before
rendering) – [[determine if
negative|https://github.com/apache/arrow/blob/master/cpp/src/arrow/util/basic_decimal.h#L125]][[negation|https://github.com/apache/arrow/blob/master/cpp/src/arrow/util/basic_decimal.cc#L377-L385]]
+ scaling values appropriately (insert the decimal place / prepend leading
zeros as necessary).
[[implementation|https://github.com/apache/arrow/blob/master/cpp/src/arrow/util/decimal.cc#L328-L386]]
The conversion to integer string is implemented in c++
[[here|https://github.com/apache/arrow/blob/master/cpp/src/arrow/util/decimal.cc#L685-L698]]
which includes checking if negative before rendering.
I see values like:
+ 101 (with scale 10, precision 13) renders as `1010000000000` – where I
expect this as `101.0000000000`
+ -101 (with scale 10, precision 13) renders as
`340282366920938500000000000000000000000`- I expect this as `-101.0000000000`
I'm not really a Javascript expert, but a similar approach to the negation
check and flipping appears to work, but I'm fairly confident I'm missing some
edge cases.
General algorithm:
+ check to see if the high bits are negative
+ if so, number is negative (prepend with "-") and add 1 to each "chunk" and
handle overflows appropriately.
+ render the string using current implementation.
+ place decimal place (or prepend with 0s) based on the scale.
> [JS] decimal toString does not support negative values
> ------------------------------------------------------
>
> Key: ARROW-6575
> URL: https://issues.apache.org/jira/browse/ARROW-6575
> Project: Apache Arrow
> Issue Type: Bug
> Components: JavaScript
> Affects Versions: 0.14.1
> Reporter: Andong Zhan
> Priority: Critical
>
> The main description is here: [https://github.com/apache/arrow/issues/5397]
> Also, I have a simple test case (slightly changed generate-test-data.js and
> generated-data-validators):
> {code:java}
> export const decimal = (length = 2, nullCount = length * 0.2 | 0, scale = 0,
> precision = 38) => vectorGenerator.visit(new Decimal(scale, precision),
> length, nullCount);
> function fillDecimal(length: number) {
> // const BPE = Uint32Array.BYTES_PER_ELEMENT; // 4
> const array = new Uint32Array(length);
> // const max = (2 ** (8 * BPE)) - 1;
> // for (let i = -1; ++i < length; array[i] = rand() * max * (rand() > 0.5
> ? -1 : 1));
> array[0] = 0;
> array[1] = 1286889712;
> array[2] = 2218195178;
> array[3] = 4282345521;
> array[4] = 0;
> array[5] = 16004768;
> array[6] = 3587851993;
> array[7] = 126217744;
> return array;
> }
> {code}
> and the expected value should be
> {code:java}
> expect(vector.get(0).toString()).toBe('-1000000000000000000000000000000000000');
> expect(vector.get(1).toString()).toBe('1000000000000000000000000000000000000');
> {code}
> However, the actual first value is 339282366920938463463374607431768211456
> which is wrong! The second value is correct by the way.
> I believe the bug is in the function called
> function decimalToString<T extends BN<BigNumArray>>(a: T) because it cannot
> return a negative value at all.
> [arrow/js/src/util/bn.ts|https://github.com/apache/arrow/blob/d54425de19b7dbb2764a40355d76d1c785cf64ec/js/src/util/bn.ts#L99]
> Line 99
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
