From 37029927af90872e8012fd177a009e94ca7a47d1 Mon Sep 17 00:00:00 2001 From: Dominik Moritz Date: Wed, 17 Apr 2024 12:15:38 -0400 Subject: [PATCH 1/2] GH-37920: Use scale when converting decimals to number --- js/src/util/bigint.ts | 10 +++++----- js/src/util/bn.ts | 25 ++++++++++--------------- js/src/visitor/get.ts | 2 +- js/test/generate-test-data.ts | 4 ++-- js/test/unit/bn-tests.ts | 13 +++++++++---- 5 files changed, 27 insertions(+), 27 deletions(-) diff --git a/js/src/util/bigint.ts b/js/src/util/bigint.ts index 470b83f5fba..2d768d4d620 100644 --- a/js/src/util/bigint.ts +++ b/js/src/util/bigint.ts @@ -26,14 +26,14 @@ export function bigIntToNumber(number: bigint | number): number { } /** - * Duivides the bigint number by the divisor and returns the result as a number. + * Divides the bigint numerator by the denominator and returns the result as a number. * Dividing bigints always results in bigints so we don't get the remainder. * This function gives us the remainder but assumes that the result fits into a number. * - * @param number The number to divide. - * @param divisor The divisor. + * @param numerator The number to divide. + * @param denominator The denominator. * @returns The result of the division as a number. */ -export function divideBigInts(number: bigint, divisor: bigint): number { - return bigIntToNumber(number / divisor) + bigIntToNumber(number % divisor) / bigIntToNumber(divisor); +export function divideBigInts(numerator: bigint, denominator: bigint): number { + return bigIntToNumber(numerator / denominator) + bigIntToNumber(numerator % denominator) / bigIntToNumber(denominator); } diff --git a/js/src/util/bn.ts b/js/src/util/bn.ts index 8f6dfe258fc..c6e6248f564 100644 --- a/js/src/util/bn.ts +++ b/js/src/util/bn.ts @@ -18,10 +18,7 @@ import { ArrayBufferViewInput, toArrayBufferView } from './buffer.js'; import { TypedArray, TypedArrayConstructor } from '../interfaces.js'; import { BigIntArray, BigIntArrayConstructor } from '../interfaces.js'; -import { bigIntToNumber } from './bigint.js'; - -/** @ignore */ -export const isArrowBigNumSymbol = Symbol.for('isArrowBigNum'); +import { bigIntToNumber, divideBigInts } from './bigint.js'; /** @ignore */ type BigNumArray = IntArray | UintArray; /** @ignore */ type IntArray = Int8Array | Int16Array | Int32Array; @@ -35,13 +32,12 @@ function BigNum(this: any, x: any, ...xs: any) { return Object.setPrototypeOf(new this['TypedArray'](x, ...xs), this.constructor.prototype); } -BigNum.prototype[isArrowBigNumSymbol] = true; BigNum.prototype.toJSON = function >(this: T) { return `"${bigNumToString(this)}"`; }; -BigNum.prototype.valueOf = function >(this: T, scale?: number) { return bigNumToNumber(this, scale); }; +BigNum.prototype.valueOf = function >(this: T, scale: number) { return bigNumToNumber(this, scale ?? (this as any)['scale']); }; BigNum.prototype.toString = function >(this: T) { return bigNumToString(this); }; BigNum.prototype[Symbol.toPrimitive] = function >(this: T, hint: 'string' | 'number' | 'default' = 'default') { switch (hint) { - case 'number': return bigNumToNumber(this); + case 'number': return bigNumToNumber(this, (this as any)['scale']); case 'string': return bigNumToString(this); case 'default': return bigNumToBigInt(this); } @@ -92,16 +88,13 @@ export function bigNumToNumber>(bn: T, scale?: number) } } if (typeof scale === 'number') { - const denominator = BigInt(Math.pow(10, scale)); - const quotient = number / denominator; - const remainder = number % denominator; - return bigIntToNumber(quotient) + (bigIntToNumber(remainder) / bigIntToNumber(denominator)); + return divideBigInts(number, BigInt(Math.pow(10, scale))); } return bigIntToNumber(number); } /** @ignore */ -export function bigNumToString>(a: T): string { +function bigNumToString>(a: T): string { // use BigInt native implementation if (a.byteLength === 8) { const bigIntArray = new a['BigIntArray'](a.buffer, a.byteOffset, 1); @@ -136,7 +129,7 @@ export function bigNumToString>(a: T): string { } /** @ignore */ -export function bigNumToBigInt>(a: T): bigint { +function bigNumToBigInt>(a: T): bigint { if (a.byteLength === 8) { const bigIntArray = new a['BigIntArray'](a.buffer, a.byteOffset, 1); return bigIntArray[0]; @@ -194,8 +187,10 @@ export class BN { return new (UnsignedBigNum)(num) as (T & BN); } /** @nocollapse */ - public static decimal(num: T): (T & BN) { - return new (DecimalBigNum)(num) as (T & BN); + public static decimal(num: T, scale = 0): (T & BN) { + const decimal = new (DecimalBigNum)(num) as (T & BN); + (decimal as any)['scale'] = scale; + return decimal; } constructor(num: T, isSigned?: boolean) { return BN.new(num, isSigned) as any; diff --git a/js/src/visitor/get.ts b/js/src/visitor/get.ts index ddfc04884f8..6bfd8c6dce5 100644 --- a/js/src/visitor/get.ts +++ b/js/src/visitor/get.ts @@ -209,7 +209,7 @@ const getTime = (data: Data, index: number): T['TValue'] => { }; /** @ignore */ -const getDecimal = ({ values, stride }: Data, index: number): T['TValue'] => BN.decimal(values.subarray(stride * index, stride * (index + 1))); +const getDecimal = ({ values, stride, type }: Data, index: number): T['TValue'] => BN.decimal(values.subarray(stride * index, stride * (index + 1)), type.scale); /** @ignore */ const getList = (data: Data, index: number): T['TValue'] => { diff --git a/js/test/generate-test-data.ts b/js/test/generate-test-data.ts index 65719f875c3..7979260d6cc 100644 --- a/js/test/generate-test-data.ts +++ b/js/test/generate-test-data.ts @@ -755,8 +755,8 @@ function createDate64(length: number, nullBitmap: Uint8Array, values: (number | return BigInt64Array.from(data32, x => BigInt(x * 86400000)); } -function divideBigInts(number: bigint, divisor: bigint): number { - return Number(number / divisor) + Number(number % divisor) / Number(divisor); +function divideBigInts(numerator: bigint, denominator: bigint): number { + return Number(numerator / denominator) + Number(numerator % denominator) / Number(denominator); } function createTimestamp(length: number, nullBitmap: Uint8Array, multiple: number, values: (number | null)[] = []) { diff --git a/js/test/unit/bn-tests.ts b/js/test/unit/bn-tests.ts index 2ea8f6055db..934666ac5d5 100644 --- a/js/test/unit/bn-tests.ts +++ b/js/test/unit/bn-tests.ts @@ -93,9 +93,14 @@ describe(`BN`, () => { expect(n3.valueOf()).toBe(-1); const n4 = new BN(new Uint32Array([0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF]), true); expect(n4.valueOf(1)).toBe(-0.1); - // const n5 = new BN(new Uint32Array([0x00000000, 0x00000000, 0x00000000, 0x80000000]), false); - // expect(n5.valueOf()).toBe(1.7014118346046923e+38); - // const n6 = new BN(new Uint32Array([0x00000000, 0x00000000, 0x00000000, 0x80000000]), false); - // expect(n6.valueOf(1)).toBe(1.7014118346046923e+37); + + const n5 = BN.decimal(new Uint32Array([0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF])); + expect(n5.valueOf()).toBe(-1); + const n6 = BN.decimal(new Uint32Array([0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF]), 1); + expect(n6.valueOf()).toBe(-0.1); + // const n7 = new BN(new Uint32Array([0x00000000, 0x00000000, 0x00000000, 0x80000000]), false); + // expect(n7.valueOf()).toBe(1.7014118346046923e+38); + // const n8 = new BN(new Uint32Array([0x00000000, 0x00000000, 0x00000000, 0x80000000]), false); + // expect(n8.valueOf(1)).toBe(1.7014118346046923e+37); }); }); From 28ebb3a0ae272a7ebc61ed6597bebb09a6d63896 Mon Sep 17 00:00:00 2001 From: Dominik Moritz Date: Wed, 17 Apr 2024 22:51:20 -0400 Subject: [PATCH 2/2] Move decimal tests to vectors --- js/test/unit/bn-tests.ts | 13 ++++--------- js/test/unit/vector/numeric-vector-tests.ts | 12 +++++++++++- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/js/test/unit/bn-tests.ts b/js/test/unit/bn-tests.ts index 934666ac5d5..2ea8f6055db 100644 --- a/js/test/unit/bn-tests.ts +++ b/js/test/unit/bn-tests.ts @@ -93,14 +93,9 @@ describe(`BN`, () => { expect(n3.valueOf()).toBe(-1); const n4 = new BN(new Uint32Array([0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF]), true); expect(n4.valueOf(1)).toBe(-0.1); - - const n5 = BN.decimal(new Uint32Array([0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF])); - expect(n5.valueOf()).toBe(-1); - const n6 = BN.decimal(new Uint32Array([0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF]), 1); - expect(n6.valueOf()).toBe(-0.1); - // const n7 = new BN(new Uint32Array([0x00000000, 0x00000000, 0x00000000, 0x80000000]), false); - // expect(n7.valueOf()).toBe(1.7014118346046923e+38); - // const n8 = new BN(new Uint32Array([0x00000000, 0x00000000, 0x00000000, 0x80000000]), false); - // expect(n8.valueOf(1)).toBe(1.7014118346046923e+37); + // const n5 = new BN(new Uint32Array([0x00000000, 0x00000000, 0x00000000, 0x80000000]), false); + // expect(n5.valueOf()).toBe(1.7014118346046923e+38); + // const n6 = new BN(new Uint32Array([0x00000000, 0x00000000, 0x00000000, 0x80000000]), false); + // expect(n6.valueOf(1)).toBe(1.7014118346046923e+37); }); }); diff --git a/js/test/unit/vector/numeric-vector-tests.ts b/js/test/unit/vector/numeric-vector-tests.ts index e5c1789068b..d13de0ef27f 100644 --- a/js/test/unit/vector/numeric-vector-tests.ts +++ b/js/test/unit/vector/numeric-vector-tests.ts @@ -21,7 +21,7 @@ import { util, Vector, makeVector, vectorFromArray, makeData, Float, Float16, Float32, Float64, - Int, Int8, Int16, Int32, Int64, Uint8, Uint16, Uint32, Uint64, DataType + Int, Int8, Int16, Int32, Int64, Uint8, Uint16, Uint32, Uint64, DataType, Decimal } from 'apache-arrow'; import type { TypedArray, @@ -233,6 +233,16 @@ describe(`IntVector`, () => { }); }); +describe(`DecimalVector`, () => { + test(`Values get scaled by the scale in the type`, () => { + const vec1 = vectorFromArray([new Uint32Array([0x00000001, 0x00000000, 0x00000000, 0x00000000])], new Decimal(0, 10)); + expect(+vec1.get(0)!).toBe(1); + + const vec2 = vectorFromArray([new Uint32Array([0x00000001, 0x00000000, 0x00000000, 0x00000000])], new Decimal(1, 10)); + expect(+vec2.get(0)!).toBe(0.1); + }); +}); + function testIntVector(DataType: new () => T, values?: Array) { const type = new DataType();