apacheGH-35067: [JavaScript] toString for signed BigNums (apache#35067)

### What changes are included in this PR?

Closes apache#22932

Basically, signed negative numbers were displayed as very large positive numbers.

### Are these changes tested?

Yes, I've also added tests for `bn.ts` (BigNum) that was not tested before.

### Are there any user-facing changes?

Negative numbers stored in BigNum will now be displayed as negative numbers instead of very large positive numbers.

Note that this change also affects decimals, because they are stored in BigNum as signed numbers. Decimals still don't convert to string correctly, because inserting the decimal dot is not implemented. Ref #28804.
* Closes: apache#35067

Lead-authored-by: Aljaž Mur Eržen <[email protected]>
Co-authored-by: Aljaž Mur Eržen <[email protected]>
Co-authored-by: Paul Taylor <[email protected]>
Signed-off-by: Dominik Moritz <[email protected]>

Author: 2 people

js/gulp/test-task.js

@@ -41,6 +41,7 @@ const testFiles = [
     `test/unit/`,
     // `test/unit/bit-tests.ts`,
     // `test/unit/int-tests.ts`,
+    // `test/unit/bn-tests.ts`,
     // `test/unit/math-tests.ts`,
     // `test/unit/table-tests.ts`,
     // `test/unit/generated-data-tests.ts`,

js/src/util/bn.ts

@@ -36,17 +36,17 @@ function BigNum(this: any, x: any, ...xs: any) {
 }
 
 BigNum.prototype[isArrowBigNumSymbol] = true;
-BigNum.prototype.toJSON = function <T extends BN<BigNumArray>>(this: T) { return `"${bignumToString(this)}"`; };
-BigNum.prototype.valueOf = function <T extends BN<BigNumArray>>(this: T) { return bignumToNumber(this); };
-BigNum.prototype.toString = function <T extends BN<BigNumArray>>(this: T) { return bignumToString(this); };
+BigNum.prototype.toJSON = function <T extends BN<BigNumArray>>(this: T) { return `"${bigNumToString(this)}"`; };
+BigNum.prototype.valueOf = function <T extends BN<BigNumArray>>(this: T) { return bigNumToNumber(this); };
+BigNum.prototype.toString = function <T extends BN<BigNumArray>>(this: T) { return bigNumToString(this); };
 BigNum.prototype[Symbol.toPrimitive] = function <T extends BN<BigNumArray>>(this: T, hint: 'string' | 'number' | 'default' = 'default') {
     switch (hint) {
-        case 'number': return bignumToNumber(this);
-        case 'string': return bignumToString(this);
-        case 'default': return bignumToBigInt(this);
+        case 'number': return bigNumToNumber(this);
+        case 'string': return bigNumToString(this);
+        case 'default': return bigNumToBigInt(this);
     }
     // @ts-ignore
-    return bignumToString(this);
+    return bigNumToString(this);
 };
 
 /** @ignore */
@@ -70,7 +70,7 @@ Object.assign(UnsignedBigNum.prototype, BigNum.prototype, { 'constructor': Unsig
 Object.assign(DecimalBigNum.prototype, BigNum.prototype, { 'constructor': DecimalBigNum, 'signed': true, 'TypedArray': Uint32Array, 'BigIntArray': BigUint64Array });
 
 /** @ignore */
-function bignumToNumber<T extends BN<BigNumArray>>(bn: T) {
+function bigNumToNumber<T extends BN<BigNumArray>>(bn: T) {
     const { buffer, byteOffset, length, 'signed': signed } = bn;
     const words = new BigUint64Array(buffer, byteOffset, length);
     const negative = signed && words[words.length - 1] & (BigInt(1) << BigInt(63));
@@ -90,12 +90,52 @@ function bignumToNumber<T extends BN<BigNumArray>>(bn: T) {
 }
 
 /** @ignore */
-export const bignumToString: { <T extends BN<BigNumArray>>(a: T): string } = (<T extends BN<BigNumArray>>(a: T) => a.byteLength === 8 ? `${new a['BigIntArray'](a.buffer, a.byteOffset, 1)[0]}` : decimalToString(a));
+export const bigNumToString: { <T extends BN<BigNumArray>>(a: T): string } = (<T extends BN<BigNumArray>>(a: T) => {
+    // use BigInt native implementation
+    if (a.byteLength === 8) {
+        const bigIntArray = new a['BigIntArray'](a.buffer, a.byteOffset, 1);
+        return `${bigIntArray[0]}`;
+    }
+
+    // unsigned numbers
+    if (!a['signed']) {
+        return unsignedBigNumToString(a);
+    }
+
+    let array = new Uint16Array(a.buffer, a.byteOffset, a.byteLength / 2);
+
+    // detect positive numbers
+    const highOrderWord = new Int16Array([array[array.length - 1]])[0];
+    if (highOrderWord >= 0) {
+        return unsignedBigNumToString(a);
+    }
+
+    // flip the negative value
+    array = array.slice();
+    let carry = 1;
+    for (let i = 0; i < array.length; i++) {
+        const elem = array[i];
+        const updated = ~elem + carry;
+        array[i] = updated;
+        carry &= elem === 0 ? 1 : 0;
+    }
+
+    const negated = unsignedBigNumToString(<any>array);
+    return `-${negated}`;
+});
+
 /** @ignore */
-export const bignumToBigInt: { <T extends BN<BigNumArray>>(a: T): bigint } = (<T extends BN<BigNumArray>>(a: T) => a.byteLength === 8 ? new a['BigIntArray'](a.buffer, a.byteOffset, 1)[0] : <any>decimalToString(a));
+export const bigNumToBigInt: { <T extends BN<BigNumArray>>(a: T): bigint } = (<T extends BN<BigNumArray>>(a: T) => {
+    if (a.byteLength === 8) {
+        const bigIntArray = new a['BigIntArray'](a.buffer, a.byteOffset, 1);
+        return bigIntArray[0];
+    } else {
+        return <any>bigNumToString(a);
+    }
+});
 
 /** @ignore */
-function decimalToString<T extends BN<BigNumArray>>(a: T) {
+function unsignedBigNumToString<T extends BN<BigNumArray>>(a: T) {
     let digits = '';
     const base64 = new Uint32Array(2);
     let base32 = new Uint16Array(a.buffer, a.byteOffset, a.byteLength / 2);

js/test/unit/bn-tests.ts

@@ -0,0 +1,86 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+import * as Arrow from 'apache-arrow';
+const { BN } = Arrow.util;
+
+describe(`BN`, () => {
+    test(`to detect signed numbers, unsigned numbers and decimals`, () => {
+        // SignedBigNum
+        const i = new BN(new Int32Array([5, 0]));
+        expect(i.signed).toBe(true);
+
+        // UnsignedBigNum
+        const u = new BN(new Uint32Array([5, 0]));
+        expect(u.signed).toBe(false);
+
+        // DecimalBigNum
+        const d = new BN(new Uint16Array([1, 2, 3, 4, 5, 6, 7, 8]));
+        expect(d.signed).toBe(true);
+    });
+
+    test(`toString for signed numbers`, () => {
+        const i1 = new BN(new Int32Array([5, 33]), true);
+        expect(i1.toString()).toBe('141733920773');
+
+        const i2 = new BN(new Int32Array([0xFFFFFFFF, 0xFFFFFFFF]), true);
+        expect(i2.toString()).toBe('-1');
+
+        const i3 = new BN(new Int32Array([0x11111111, 0x11111111, 0x11111111]), true);
+        expect(i3.toString()).toBe('5281877500950955839569596689');
+
+        const i4 = new BN(new Int16Array([0xFFFF, 0xFFFF]), true);
+        expect(i4.toString()).toBe('-1');
+
+        const i5 = new BN(new Int32Array([0xFFFFFFFE, 0xFFFFFFFF, 0xFFFFFFFF]), true);
+        expect(i5.toString()).toBe('-2');
+    });
+
+    test(`toString for unsigned numbers`, () => {
+        const u1 = new BN(new Uint32Array([5, 33]), false);
+        expect(u1.toString()).toBe('141733920773');
+
+        const u2 = new BN(new Uint32Array([0xFFFFFFFF, 0xFFFFFFFF]), false);
+        expect(u2.toString()).toBe('18446744073709551615');
+
+        const u3 = new BN(new Uint32Array([0x11111111, 0x11111111, 0x11111111]), false);
+        expect(u3.toString()).toBe('5281877500950955839569596689');
+
+        const u4 = new BN(new Uint16Array([0xFFFF, 0xFFFF]), false);
+        expect(u4.toString()).toBe('4294967295');
+    });
+
+    test(`toString for decimals`, () => {
+        const toDecimal = (val: Uint32Array) => {
+            const builder = Arrow.makeBuilder({
+                type: new Arrow.Decimal(128, 0),
+                nullValues: []
+            });
+            builder.append(val);
+            return <Arrow.Type.Decimal><any>builder.finish().toVector().get(0);
+        };
+
+        const d2 = toDecimal(new Uint32Array([0xFFFFFFFE, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF]));
+        expect(d2.toString()).toBe('-2');
+
+        const d3 = toDecimal(new Uint32Array([0x00000000, 0x00000000, 0x00000000, 0x80000000]));
+        expect(d3.toString()).toBe('-170141183460469231731687303715884105728');
+
+        const d4 = toDecimal(new Uint32Array([0x9D91E773, 0x4BB90CED, 0xAB2354CC, 0x54278E9B]));
+        expect(d4.toString()).toBe('111860543658909349380118287427608635251');
+    });
+});