Skip to content

Commit

Permalink
ARROW-14647: [JS] fix bignumToNumber for negative numbers
Browse files Browse the repository at this point in the history
`bignumToNumber` is giving incorrect results for negative numbers. For example, the following data exists in my dataset:

```javascript
// this represents the value -180846 but bignumToNumber() returns -18446744069414765000
const v = new Uint32Array([4294786450, 4294967295, 4294967295, 4294967295])
```

In this PR, I rewrote the function to handle negative numbers correctly. I've tested it locally. Not sure if there's a more efficient way to write the function. It basically checks the most significant bit to see if it's negative, and, if it is, applies the 2's compliment and multiplies by -1 at the end. I couldn't think of any safe way to avoid multiplying at the end since JS's Number type is not guaranteed to be able to handle the values BigNum can represent, so, there's going to (potentially) be some bits lost in the conversion. So, any bitwise method may result in losing more information than we'd like.

Thinking about it some more, I actually think the existing bignumToNumber will fail on anything with more than 64-bits - doesn't really have anything to do with the number being negative. The original function actually works on my example above if I only let it run on the first two 32-bits.

Closes apache#11655 from bmatcuk/master

Lead-authored-by: Bob Matcuk <[email protected]>
Co-authored-by: ptaylor <[email protected]>
Co-authored-by: Dominik Moritz <[email protected]>
Signed-off-by: Dominik Moritz <[email protected]>
  • Loading branch information
3 people committed Apr 6, 2022
1 parent adfa605 commit c4c5c03
Showing 1 changed file with 13 additions and 9 deletions.
22 changes: 13 additions & 9 deletions js/src/util/bn.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,15 +72,19 @@ Object.assign(DecimalBigNum.prototype, BigNum.prototype, { 'constructor': Decima
/** @ignore */
function bignumToNumber<T extends BN<BigNumArray>>(bn: T) {
const { buffer, byteOffset, length, 'signed': signed } = bn;
const words = new Int32Array(buffer, byteOffset, length);
let number = 0, i = 0;
const n = words.length;
let hi, lo;
while (i < n) {
lo = words[i++];
hi = words[i++];
signed || (hi = hi >>> 0);
number += (lo >>> 0) + (hi * (i ** 32));
const words = new BigUint64Array(buffer, byteOffset, length);
const negative = signed && words[words.length - 1] & (BigInt(1) << BigInt(63));
let number = negative ? BigInt(1) : BigInt(0);
let i = BigInt(0);
if (!negative) {
for (const word of words) {
number += word * (BigInt(1) << (BigInt(32) * i++));
}
} else {
for (const word of words) {
number += ~word * (BigInt(1) << (BigInt(32) * i++));
}
number *= BigInt(-1);
}
return number;
}
Expand Down

0 comments on commit c4c5c03

Please sign in to comment.