Skip to content

GH-37920: [JS] Use scale when converting decimals to number #41260

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions js/src/util/bigint.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
25 changes: 10 additions & 15 deletions js/src/util/bn.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 <T extends BN<BigNumArray>>(this: T) { return `"${bigNumToString(this)}"`; };
BigNum.prototype.valueOf = function <T extends BN<BigNumArray>>(this: T, scale?: number) { return bigNumToNumber(this, scale); };
BigNum.prototype.valueOf = function <T extends BN<BigNumArray>>(this: T, scale: number) { return bigNumToNumber(this, scale ?? (this as any)['scale']); };
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 'number': return bigNumToNumber(this, (this as any)['scale']);
case 'string': return bigNumToString(this);
case 'default': return bigNumToBigInt(this);
}
Expand Down Expand Up @@ -92,16 +88,13 @@ export function bigNumToNumber<T extends BN<BigNumArray>>(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<T extends BN<BigNumArray>>(a: T): string {
function bigNumToString<T extends BN<BigNumArray>>(a: T): string {
// use BigInt native implementation
if (a.byteLength === 8) {
const bigIntArray = new a['BigIntArray'](a.buffer, a.byteOffset, 1);
Expand Down Expand Up @@ -136,7 +129,7 @@ export function bigNumToString<T extends BN<BigNumArray>>(a: T): string {
}

/** @ignore */
export function bigNumToBigInt<T extends BN<BigNumArray>>(a: T): bigint {
function bigNumToBigInt<T extends BN<BigNumArray>>(a: T): bigint {
if (a.byteLength === 8) {
const bigIntArray = new a['BigIntArray'](a.buffer, a.byteOffset, 1);
return bigIntArray[0];
Expand Down Expand Up @@ -194,8 +187,10 @@ export class BN<T extends BigNumArray> {
return new (<any>UnsignedBigNum)(num) as (T & BN<T>);
}
/** @nocollapse */
public static decimal<T extends UintArray>(num: T): (T & BN<T>) {
return new (<any>DecimalBigNum)(num) as (T & BN<T>);
public static decimal<T extends UintArray>(num: T, scale = 0): (T & BN<T>) {
const decimal = new (<any>DecimalBigNum)(num) as (T & BN<T>);
(decimal as any)['scale'] = scale;
return decimal;
}
constructor(num: T, isSigned?: boolean) {
return BN.new(num, isSigned) as any;
Expand Down
2 changes: 1 addition & 1 deletion js/src/visitor/get.ts
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ const getTime = <T extends Time>(data: Data<T>, index: number): T['TValue'] => {
};

/** @ignore */
const getDecimal = <T extends Decimal>({ values, stride }: Data<T>, index: number): T['TValue'] => BN.decimal(values.subarray(stride * index, stride * (index + 1)));
const getDecimal = <T extends Decimal>({ values, stride, type }: Data<T>, index: number): T['TValue'] => BN.decimal(values.subarray(stride * index, stride * (index + 1)), type.scale);

/** @ignore */
const getList = <T extends List>(data: Data<T>, index: number): T['TValue'] => {
Expand Down
4 changes: 2 additions & 2 deletions js/test/generate-test-data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)[] = []) {
Expand Down
12 changes: 11 additions & 1 deletion js/test/unit/vector/numeric-vector-tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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<T extends Int>(DataType: new () => T, values?: Array<any>) {

const type = new DataType();
Expand Down