You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@arrow.apache.org by we...@apache.org on 2019/06/26 21:13:47 UTC
[arrow] branch master updated: ARROW-5714: [JS] Inconsistent
behavior in Int64Builder with/without BigNum
This is an automated email from the ASF dual-hosted git repository.
wesm pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/arrow.git
The following commit(s) were added to refs/heads/master by this push:
new e980b20 ARROW-5714: [JS] Inconsistent behavior in Int64Builder with/without BigNum
e980b20 is described below
commit e980b2024a28bb1bf961a25a22df145175084bf0
Author: Brian Hulette <hu...@gmail.com>
AuthorDate: Wed Jun 26 16:12:33 2019 -0500
ARROW-5714: [JS] Inconsistent behavior in Int64Builder with/without BigNum
- Use stride in `DataBufferBuilder.set` to ensure that Int64Builder is consistent with and without BigNum.
- Use `WideBufferBuilder` for `Int64` and `Uint64` builders, even when `BigInt` is not available. Moves check for `BigInt` availability into `WideBufferBuilder`. (Thanks to Paul Taylor)
Author: Brian Hulette <hu...@gmail.com>
Author: ptaylor <pa...@me.com>
Closes #4691 from TheNeuralBit/js-data-buffer-builder-stride and squashes the following commits:
786261207 <Brian Hulette> Add clarifying comment for int64 generation in builder tests
b08ac1b8e <Brian Hulette> Merge pull request #3 from trxcllnt/js/data-buffer-builder-64bit-stride
2e4ce7af0 <Brian Hulette> Use stride in DataBufferBuilder.set
4567d0752 <ptaylor> use WideBufferBuilder for Int64 and Uint64 builders
dc9ed3a1a <ptaylor> update package-lock.json
---
js/package-lock.json | 8 ++++----
js/src/builder/buffer.ts | 22 ++++++++++------------
js/src/builder/int.ts | 17 +++++++----------
js/test/unit/builders/utils.ts | 24 ++++++++++++++++++++++--
4 files changed, 43 insertions(+), 28 deletions(-)
diff --git a/js/package-lock.json b/js/package-lock.json
index bf0c1e7..153fbfd 100644
--- a/js/package-lock.json
+++ b/js/package-lock.json
@@ -13425,7 +13425,7 @@
"progress": "^2.0.3",
"shelljs": "^0.8.3",
"typedoc-default-themes": "^0.6.0-0",
- "typescript": "3.4.x"
+ "typescript": "3.5.x"
},
"dependencies": {
"handlebars": {
@@ -13447,9 +13447,9 @@
"dev": true
},
"typescript": {
- "version": "3.4.5",
- "resolved": "https://registry.npmjs.org/typescript/-/typescript-3.4.5.tgz",
- "integrity": "sha512-YycBxUb49UUhdNMU5aJ7z5Ej2XGmaIBL0x34vZ82fn3hGvD+bgrMrVDpatgz2f7YxUMJxMkbWxJZeAvDxVe7Vw==",
+ "version": "3.5.2",
+ "resolved": "https://registry.npmjs.org/typescript/-/typescript-3.5.2.tgz",
+ "integrity": "sha512-7KxJovlYhTX5RaRbUdkAXN1KUZ8PwWlTzQdHV6xNqvuFOs7+WBo10TQUqT19Q/Jz2hk5v9TQDIhyLhhJY4p5AA==",
"dev": true
}
}
diff --git a/js/src/builder/buffer.ts b/js/src/builder/buffer.ts
index b9f7aa4..7aa336a 100644
--- a/js/src/builder/buffer.ts
+++ b/js/src/builder/buffer.ts
@@ -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> =
@@ -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;
}
}
@@ -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);
@@ -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;
}
}
diff --git a/js/src/builder/int.ts b/js/src/builder/int.ts
index 867203b..5777bd1 100644
--- a/js/src/builder/int.ts
+++ b/js/src/builder/int.ts
@@ -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 */
@@ -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)); }
}
@@ -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)); }
}
diff --git a/js/test/unit/builders/utils.ts b/js/test/unit/builders/utils.ts
index 55d6a7b..c6a29a7 100644
--- a/js/test/unit/builders/utils.ts
+++ b/js/test/unit/builders/utils.ts
@@ -53,7 +53,17 @@ 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));
@@ -61,7 +71,17 @@ export const uint16sNoNulls = (length = 20) => Array.from(new Uint16Array(random
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));