You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@arrow.apache.org by do...@apache.org on 2023/04/22 07:42:32 UTC
[arrow] branch main updated: GH-35067: [JavaScript] toString for signed `BigNum`s (#35067)
This is an automated email from the ASF dual-hosted git repository.
domoritz pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow.git
The following commit(s) were added to refs/heads/main by this push:
new a1403d4691 GH-35067: [JavaScript] toString for signed `BigNum`s (#35067)
a1403d4691 is described below
commit a1403d46916ac6daa419cc688c5fb19980a59e60
Author: Aljaž Mur Eržen <al...@users.noreply.github.com>
AuthorDate: Sat Apr 22 09:42:25 2023 +0200
GH-35067: [JavaScript] toString for signed `BigNum`s (#35067)
### What changes are included in this PR?
Closes #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: #35067
Lead-authored-by: Aljaž Mur Eržen <al...@gmail.com>
Co-authored-by: Aljaž Mur Eržen <al...@users.noreply.github.com>
Co-authored-by: Paul Taylor <17...@users.noreply.github.com>
Signed-off-by: Dominik Moritz <do...@gmail.com>
---
js/gulp/test-task.js | 1 +
js/src/util/bn.ts | 62 +++++++++++++++++++++++++++-------
js/test/unit/bn-tests.ts | 86 ++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 138 insertions(+), 11 deletions(-)
diff --git a/js/gulp/test-task.js b/js/gulp/test-task.js
index 016d33892c..03d44bf91d 100644
--- a/js/gulp/test-task.js
+++ b/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`,
diff --git a/js/src/util/bn.ts b/js/src/util/bn.ts
index a7051e73df..e04c70751a 100644
--- a/js/src/util/bn.ts
+++ b/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);
diff --git a/js/test/unit/bn-tests.ts b/js/test/unit/bn-tests.ts
new file mode 100644
index 0000000000..c9606baf85
--- /dev/null
+++ b/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');
+ });
+});