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

GH-39131: [JS] Add at() for array like types #40712

Closed
wants to merge 2 commits into from
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
2 changes: 1 addition & 1 deletion js/bin/integration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ function compareVectors(actual: Vector, expected: Vector) {

(() => {
for (let i = -1, n = actual.length; ++i < n;) {
const x1 = actual.get(i), x2 = expected.get(i);
const x1 = actual.at(i), x2 = expected.at(i);
if (!createElementComparator(x2)(x1)) {
throw new Error(`${i}: ${x1} !== ${x2}`);
}
Expand Down
13 changes: 6 additions & 7 deletions js/bin/print-buffer-alignment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,9 @@ import { RecordBatch, AsyncMessageReader, makeData, Struct, Schema, Field } from
while (1) {
// @ts-ignore
if ((metadataLength = (await reader.readMetadataLength())).done) { break; }
if (metadataLength.value === -1) {
if (metadataLength.value === -1 &&
// @ts-ignore
if ((metadataLength = (await reader.readMetadataLength())).done) { break; }
}
(metadataLength = (await reader.readMetadataLength())).done) { break; }
// @ts-ignore
if ((message = (await reader.readMetadata(metadataLength.value))).done) { break; }

Expand All @@ -65,9 +64,9 @@ import { RecordBatch, AsyncMessageReader, makeData, Struct, Schema, Field } from
bodyByteLength: body.byteLength,
});
byteOffset += metadataLength.value;
bufferRegions.forEach(({ offset, length: byteLength }, i) => {
for (const [i, { offset, length: byteLength }] of bufferRegions.entries()) {
console.log(`\tbuffer ${i + 1}:`, { byteOffset: byteOffset + offset, byteLength });
});
}
byteOffset += body.byteLength;
} else if (message.value.isDictionaryBatch()) {
const header = message.value.header();
Expand All @@ -85,9 +84,9 @@ import { RecordBatch, AsyncMessageReader, makeData, Struct, Schema, Field } from
bodyByteLength: body.byteLength,
});
byteOffset += metadataLength.value;
bufferRegions.forEach(({ offset, length: byteLength }, i) => {
for (const [i, { offset, length: byteLength }] of bufferRegions.entries()) {
console.log(`\tbuffer ${i + 1}:`, { byteOffset: byteOffset + offset, byteLength });
});
}
byteOffset += body.byteLength;
}
}
Expand Down
12 changes: 6 additions & 6 deletions js/perf/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ b.suite(
...Object.entries(vectors).map(([name, vector]) =>
b.add(`from: ${name}`, () => {
for (let i = -1, n = vector.length; ++i < n;) {
vector.get(i);
vector.at(i);
}
})),

Expand Down Expand Up @@ -155,7 +155,7 @@ for (const { name, ipc, table } of config) {
suite_name: `Get values by index`,
fn(vector: Arrow.Vector<any>) {
for (let i = -1, n = vector.length; ++i < n;) {
vector.get(i);
vector.at(i);
}
}
}, {
Expand Down Expand Up @@ -205,9 +205,9 @@ for (const { name, table, counts } of config) {
table.toArray();
}),

b.add(`get, dataset: ${name}, numRows: ${formatNumber(table.numRows)}`, () => {
b.add(`at, dataset: ${name}, numRows: ${formatNumber(table.numRows)}`, () => {
for (let i = -1, n = table.numRows; ++i < n;) {
table.get(i);
table.at(i);
}
}),

Expand All @@ -233,7 +233,7 @@ for (const { name, table, counts } of config) {
const vector = batch.getChildAt(colidx)!;
// yield all indices
for (let index = -1, length = batch.numRows; ++index < length;) {
sum += (vector.get(index) >= value) ? 1 : 0;
sum += (vector.at(index) >= value) ? 1 : 0;
}
}
return sum;
Expand All @@ -249,7 +249,7 @@ for (const { name, table, counts } of config) {
const vector = batch.getChildAt(colidx)!;
// yield all indices
for (let index = -1, length = batch.numRows; ++index < length;) {
sum += (vector.get(index) === value) ? 1 : 0;
sum += (vector.at(index) === value) ? 1 : 0;
}
}
return sum;
Expand Down
6 changes: 3 additions & 3 deletions js/src/builder/buffer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,8 @@ export class BufferBuilder<T extends TypedArray | BigIntArray> {

/** @ignore */
export class DataBufferBuilder<T extends TypedArray | BigIntArray> extends BufferBuilder<T> {
public last() { return this.get(this.length - 1); }
public get(index: number): T[0] { return this.buffer[index]; }
public last() { return this.at(- 1); }
public at(index: number): T[0] { return this.buffer[index]; }
public set(index: number, value: T[0]) {
this.reserve(index - this.length + 1);
this.buffer[index * this.stride] = value;
Expand All @@ -107,7 +107,7 @@ export class BitmapBufferBuilder extends DataBufferBuilder<Uint8Array> {

public numValid = 0;
public get numInvalid() { return this.length - this.numValid; }
public get(idx: number) { return this.buffer[idx >> 3] >> idx % 8 & 1; }
public at(idx: number) { return this.buffer[idx >> 3] >> idx % 8 & 1; }
public set(idx: number, val: number) {
const { buffer } = this.reserve(idx - this.length + 1);
const byte = idx >> 3, bit = idx % 8, cur = buffer[byte] >> bit & 1;
Expand Down
14 changes: 12 additions & 2 deletions js/src/recordbatch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import { Vector } from './vector.js';
import { Schema, Field } from './schema.js';
import { DataType, Struct, Null, TypeMap } from './type.js';

import { instance as getVisitor } from './visitor/get.js';
import { instance as atVisitor } from './visitor/at.js';
import { instance as setVisitor } from './visitor/set.js';
import { instance as indexOfVisitor } from './visitor/indexof.js';
import { instance as iteratorVisitor } from './visitor/iterator.js';
Expand Down Expand Up @@ -124,11 +124,21 @@ export class RecordBatch<T extends TypeMap = any> {
}

/**
* @deprecated Use `at()` instead.
*
* Get a row by position.
* @param index The index of the element to read.
*/
public get(index: number) {
return getVisitor.visit(this.data, index);
return this.at(index);
}

/**
* Get an element value by position.
* @param index The index of the element to read. A negative index will count back from the last element.
*/
public at(index: number) {
return atVisitor.visit(this.data, index);
}

/**
Expand Down
10 changes: 5 additions & 5 deletions js/src/row/map.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import { Data } from '../data.js';
import { Vector } from '../vector.js';
import { DataType, Struct } from '../type.js';
import { valueToString } from '../util/pretty.js';
import { instance as getVisitor } from '../visitor/get.js';
import { instance as atVisitor } from '../visitor/at.js';
import { instance as setVisitor } from '../visitor/set.js';

/** @ignore */ export const kKeys = Symbol.for('keys');
Expand Down Expand Up @@ -51,7 +51,7 @@ export class MapRow<K extends DataType = any, V extends DataType = any> {
const vals = this[kVals];
const json = {} as { [P in K['TValue']]: V['TValue'] };
for (let i = -1, n = keys.length; ++i < n;) {
json[keys.get(i)] = getVisitor.visit(vals, i);
json[keys.at(i)] = atVisitor.visit(vals, i);
}
return json;
}
Expand Down Expand Up @@ -94,8 +94,8 @@ class MapRowIterator<K extends DataType = any, V extends DataType = any>
return {
done: false,
value: [
this.keys.get(i),
getVisitor.visit(this.vals, i),
this.keys.at(i),
atVisitor.visit(this.vals, i),
] as [K['TValue'], V['TValue'] | null]
};
}
Expand Down Expand Up @@ -126,7 +126,7 @@ class MapRowProxyHandler<K extends DataType = any, V extends DataType = any> imp
}
const idx = row[kKeys].indexOf(key);
if (idx !== -1) {
const val = getVisitor.visit(Reflect.get(row, kVals), idx);
const val = atVisitor.visit(Reflect.get(row, kVals), idx);
// Cache key/val lookups
Reflect.set(row, key, val);
return val;
Expand Down
8 changes: 4 additions & 4 deletions js/src/row/struct.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import { Data } from '../data.js';
import { Field } from '../schema.js';
import { Struct, TypeMap } from '../type.js';
import { valueToString } from '../util/pretty.js';
import { instance as getVisitor } from '../visitor/get.js';
import { instance as atVisitor } from '../visitor/at.js';
import { instance as setVisitor } from '../visitor/set.js';

/** @ignore */ const kParent = Symbol.for('parent');
Expand Down Expand Up @@ -50,7 +50,7 @@ export class StructRow<T extends TypeMap = any> {
const keys = parent.type.children;
const json = {} as { [P in string & keyof T]: T[P]['TValue'] };
for (let j = -1, n = keys.length; ++j < n;) {
json[keys[j].name as string & keyof T] = getVisitor.visit(parent.children[j], i);
json[keys[j].name as string & keyof T] = atVisitor.visit(parent.children[j], i);
}
return json;
}
Expand Down Expand Up @@ -102,7 +102,7 @@ class StructRowIterator<T extends TypeMap = any>
done: false,
value: [
this.childFields[i].name,
getVisitor.visit(this.children[i], this.rowIndex)
atVisitor.visit(this.children[i], this.rowIndex)
]
} as IteratorYieldResult<[any, any]>;
}
Expand Down Expand Up @@ -139,7 +139,7 @@ class StructRowProxyHandler<T extends TypeMap = any> implements ProxyHandler<Str
}
const idx = row[kParent].type.children.findIndex((f) => f.name === key);
if (idx !== -1) {
const val = getVisitor.visit(row[kParent].children[idx], row[kRowIndex]);
const val = atVisitor.visit(row[kParent].children[idx], row[kRowIndex]);
// Cache key/val lookups
Reflect.set(row, key, val);
return val;
Expand Down
17 changes: 14 additions & 3 deletions js/src/table.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ import {
sliceChunks,
} from './util/chunk.js';

import { instance as getVisitor } from './visitor/get.js';
import { instance as atVisitor } from './visitor/at.js';
import { instance as setVisitor } from './visitor/set.js';
import { instance as indexOfVisitor } from './visitor/indexof.js';
import { instance as iteratorVisitor } from './visitor/iterator.js';
Expand Down Expand Up @@ -189,12 +189,23 @@ export class Table<T extends TypeMap = any> {
public isValid(index: number): boolean { return false; }

/**
* @deprecated Use `at()` instead.
*
* Get an element value by position.
*
* @param index The index of the element to read.
*/
// @ts-ignore
public get(index: number): Struct<T>['TValue'] | null { return null; }
public get(index: number): Struct<T>['TValue'] | null {
return this.at(index);
}

/**
* Get an element value by position.
* @param index The index of the element to read. A negative index will count back from the last element.
*/
// @ts-ignore
public at(index: number): Struct<T>['TValue'] | null { return null; }

/**
* Set an element value by position.
Expand Down Expand Up @@ -379,7 +390,7 @@ export class Table<T extends TypeMap = any> {
(proto as any)._nullCount = -1;
(proto as any)[Symbol.isConcatSpreadable] = true;
(proto as any)['isValid'] = wrapChunkedCall1(isChunkedValid);
(proto as any)['get'] = wrapChunkedCall1(getVisitor.getVisitFn(Type.Struct));
(proto as any)['at'] = wrapChunkedCall1(atVisitor.getVisitFn(Type.Struct));
(proto as any)['set'] = wrapChunkedCall2(setVisitor.getVisitFn(Type.Struct));
(proto as any)['indexOf'] = wrapChunkedIndexOf(indexOfVisitor.getVisitFn(Type.Struct));
return 'Table';
Expand Down
6 changes: 6 additions & 0 deletions js/src/util/chunk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,9 @@ export function isChunkedValid<T extends DataType>(data: Data<T>, index: number)
export function wrapChunkedCall1<T extends DataType>(fn: (c: Data<T>, _1: number) => any) {
function chunkedFn(chunks: ReadonlyArray<Data<T>>, i: number, j: number) { return fn(chunks[i], j); }
return function (this: any, index: number) {
// negative indexes count from the back
if (index < 0) index += this.length;

const data = this.data as ReadonlyArray<Data<T>>;
return binarySearch(data, this._offsets, index, chunkedFn);
};
Expand All @@ -130,6 +133,9 @@ export function wrapChunkedCall2<T extends DataType>(fn: (c: Data<T>, _1: number
let _2: any;
function chunkedFn(chunks: ReadonlyArray<Data<T>>, i: number, j: number) { return fn(chunks[i], j, _2); }
return function (this: any, index: number, value: any) {
// negative indexes count from the back
if (index < 0) index += this.length;

const data = this.data as ReadonlyArray<Data<T>>;
_2 = value;
const result = binarySearch(data, this._offsets, index, chunkedFn);
Expand Down
4 changes: 2 additions & 2 deletions js/src/util/vector.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ function createMapComparator(lhs: Map<any, any>) {
function createVectorComparator(lhs: Vector<any>) {
const comparators = [] as ((x: any) => boolean)[];
for (let i = -1, n = lhs.length; ++i < n;) {
comparators[i] = createElementComparator(lhs.get(i));
comparators[i] = createElementComparator(lhs.at(i));
}
return createSubElementsComparator(comparators);
}
Expand Down Expand Up @@ -163,7 +163,7 @@ function compareVector(comparators: ((x: any) => boolean)[], vec: Vector) {
const n = comparators.length;
if (vec.length !== n) { return false; }
for (let i = -1; ++i < n;) {
if (!(comparators[i](vec.get(i)))) { return false; }
if (!(comparators[i](vec.at(i)))) { return false; }
}
return true;
}
Expand Down
Loading
Loading