Skip to content
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

ARROW-5714: [JS] Inconsistent behavior in Int64Builder with/without BigNum #4691

Closed
Closed
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
8 changes: 4 additions & 4 deletions js/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

22 changes: 10 additions & 12 deletions js/src/builder/buffer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,12 @@
// under the License.

import { memcpy } from '../util/buffer';
import { BigInt64Array, BigUint64Array } from '../util/compat';
import { BigIntAvailable, BigInt64Array, BigUint64Array } from '../util/compat';
import {
TypedArray, TypedArrayConstructor,
BigIntArray, BigIntArrayConstructor
} from '../interfaces';

/** @ignore */ type WideArray<T extends BigIntArray> = T extends BigIntArray ? Int32Array : Uint32Array;
/** @ignore */ type DataValue<T> = T extends TypedArray ? number : T extends BigIntArray ? WideValue<T> : T;
/** @ignore */ type WideValue<T extends BigIntArray> = T extends BigIntArray ? bigint | Int32Array | Uint32Array : never;
/** @ignore */ type ArrayCtor<T extends TypedArray | BigIntArray> =
Expand Down Expand Up @@ -105,7 +104,7 @@ export class DataBufferBuilder<T extends TypedArray> extends BufferBuilder<T, nu
public get(index: number) { return this.buffer[index]; }
public set(index: number, value: number) {
this.reserve(index - this.length + 1);
this.buffer[index] = value;
this.buffer[index * this.stride] = value;
return this;
}
}
Expand Down Expand Up @@ -157,16 +156,13 @@ export class OffsetsBufferBuilder extends DataBufferBuilder<Int32Array> {
}

/** @ignore */
export class WideBufferBuilder<T extends BigIntArray> extends BufferBuilder<WideArray<T>, DataValue<T>> {
export class WideBufferBuilder<T extends TypedArray, R extends BigIntArray> extends BufferBuilder<T, DataValue<T>> {
// @ts-ignore
public buffer64: T;
public buffer64: R;
// @ts-ignore
constructor(buffer: T, stride: number) {
const ArrayType = buffer instanceof BigInt64Array ? Int32Array : Uint32Array;
super(new ArrayType(buffer.buffer, buffer.byteOffset, buffer.byteLength / 4) as WideArray<T>, stride);
}
public get ArrayType64(): BigIntArrayConstructor<T> {
return this.buffer instanceof Int32Array ? BigInt64Array : BigUint64Array as any;
protected _ArrayType64: BigIntArrayConstructor<R>;
public get ArrayType64() {
return this._ArrayType64 || (this._ArrayType64 = <BigIntArrayConstructor<R>> (this.buffer instanceof Int32Array ? BigInt64Array : BigUint64Array));
}
public set(index: number, value: DataValue<T>) {
this.reserve(index - this.length + 1);
Expand All @@ -180,7 +176,9 @@ export class WideBufferBuilder<T extends BigIntArray> extends BufferBuilder<Wide
protected _resize(newLength: number) {
const data = super._resize(newLength);
const length = data.byteLength / (this.BYTES_PER_ELEMENT * this.stride);
this.buffer64 = new this.ArrayType64(data.buffer, data.byteOffset, length);
if (BigIntAvailable) {
this.buffer64 = new this.ArrayType64(data.buffer, data.byteOffset, length);
}
return data;
}
}
17 changes: 7 additions & 10 deletions js/src/builder/int.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,8 @@

import { bignumToBigInt } from '../util/bn';
import { WideBufferBuilder } from './buffer';
import { BigInt64Array } from '../util/compat';
import { FixedWidthBuilder, BuilderOptions } from '../builder';
import { BigInt64ArrayAvailable, BigInt64Array } from '../util/compat';
import { BigUint64ArrayAvailable, BigUint64Array } from '../util/compat';
import { Int, Int8, Int16, Int32, Int64, Uint8, Uint16, Uint32, Uint64 } from '../type';

/** @ignore */
Expand All @@ -37,16 +36,15 @@ export class Int16Builder<TNull = any> extends IntBuilder<Int16, TNull> {}
export class Int32Builder<TNull = any> extends IntBuilder<Int32, TNull> {}
/** @ignore */
export class Int64Builder<TNull = any> extends IntBuilder<Int64, TNull> {
protected _values: WideBufferBuilder<Int32Array, BigInt64Array>;
constructor(options: BuilderOptions<Int64, TNull>) {
if (options['nullValues']) {
options['nullValues'] = (options['nullValues'] as TNull[]).map(toBigInt);
}
super(options);
if (BigInt64ArrayAvailable) {
this._values = <any> new WideBufferBuilder(new BigInt64Array(0), 2);
}
this._values = new WideBufferBuilder(new Int32Array(0), 2);
}
public get values64() { return (this._values as any).buffer64 as BigInt64Array; }
public get values64() { return this._values.buffer64; }
public isValid(value: Int32Array | bigint | TNull) { return super.isValid(toBigInt(value)); }
}

Expand All @@ -58,16 +56,15 @@ export class Uint16Builder<TNull = any> extends IntBuilder<Uint16, TNull> {}
export class Uint32Builder<TNull = any> extends IntBuilder<Uint32, TNull> {}
/** @ignore */
export class Uint64Builder<TNull = any> extends IntBuilder<Uint64, TNull> {
protected _values: WideBufferBuilder<Uint32Array, BigUint64Array>;
constructor(options: BuilderOptions<Uint64, TNull>) {
if (options['nullValues']) {
options['nullValues'] = (options['nullValues'] as TNull[]).map(toBigInt);
}
super(options);
if (BigUint64ArrayAvailable) {
this._values = <any> new WideBufferBuilder(new BigUint64Array(0), 2);
}
this._values = new WideBufferBuilder(new Uint32Array(0), 2);
}
public get values64() { return (this._values as any).buffer64 as BigUint64Array; }
public get values64() { return this._values.buffer64; }
public isValid(value: Uint32Array | bigint | TNull) { return super.isValid(toBigInt(value)); }
}

Expand Down
24 changes: 22 additions & 2 deletions js/test/unit/builders/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,15 +53,35 @@ export const int16sNoNulls = (length = 20) => Array.from(new Int16Array(randomBy
export const int32sNoNulls = (length = 20) => Array.from(new Int32Array(randomBytes(length * Int32Array.BYTES_PER_ELEMENT).buffer));
export const int64sNoNulls = (length = 20) => Array.from({ length }, (_, i) => {
const bn = util.BN.new(new Int32Array(randomBytes(2 * 4).buffer));
return i % 3 === 0 ? bn : i % 2 === 0 ? bn[Symbol.toPrimitive]() : bn[0];
// Evenly distribute the three types of arguments we support in the Int64
// builder
switch (i % 3) {
// Int32Array (util.BN is-a Int32Array)
case 0: return bn;
// BigInt
case 1: return bn[Symbol.toPrimitive]()
// number
case 2:
default: return bn[0];
}
});

export const uint8sNoNulls = (length = 20) => Array.from(new Uint8Array(randomBytes(length * Uint8Array.BYTES_PER_ELEMENT).buffer));
export const uint16sNoNulls = (length = 20) => Array.from(new Uint16Array(randomBytes(length * Uint16Array.BYTES_PER_ELEMENT).buffer));
export const uint32sNoNulls = (length = 20) => Array.from(new Uint32Array(randomBytes(length * Uint32Array.BYTES_PER_ELEMENT).buffer));
export const uint64sNoNulls = (length = 20) => Array.from({ length }, (_, i) => {
const bn = util.BN.new(new Uint32Array(randomBytes(2 * 4).buffer));
return i % 3 === 0 ? bn : i % 2 === 0 ? bn[Symbol.toPrimitive]() : bn[0];
// Evenly distribute the three types of arguments we support in the Uint64
// builder
switch (i % 3) {
// UInt32Array (util.BN is-a Uint32Array)
case 0: return bn;
// BigInt
case 1: return bn[Symbol.toPrimitive]()
// number
case 2:
default: return bn[0];
}
});
export const float16sNoNulls = (length = 20) => Array.from(new Uint16Array(randomBytes(length * Uint16Array.BYTES_PER_ELEMENT).buffer)).map((x) => (x - 32767) / 32767);
export const float32sNoNulls = (length = 20) => Array.from(new Float32Array(randomBytes(length * Float32Array.BYTES_PER_ELEMENT).buffer));
Expand Down