[arrow] branch master updated: ARROW-14647: [JS] fix bignumToNumber for negative numbers

[do...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

domoritz 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 c4c5c03206 ARROW-14647: [JS] fix bignumToNumber for negative numbers
c4c5c03206 is described below

commit c4c5c0320608c495295034b1983cacf77b2c1661
Author: Bob Matcuk <ro...@omnisci.com>
AuthorDate: Wed Apr 6 13:00:04 2022 -0400

    ARROW-14647: [JS] fix bignumToNumber for negative numbers
    
    `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 (p [...]
    
    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 #11655 from bmatcuk/master
    
    Lead-authored-by: Bob Matcuk <ro...@omnisci.com>
    Co-authored-by: ptaylor <pa...@me.com>
    Co-authored-by: Dominik Moritz <do...@gmail.com>
    Signed-off-by: Dominik Moritz <do...@gmail.com>
---
 js/src/util/bn.ts | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/js/src/util/bn.ts b/js/src/util/bn.ts
index 925a850840..c10d7d3c46 100644
--- a/js/src/util/bn.ts
+++ b/js/src/util/bn.ts
@@ -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;
 }