Skip to content

Commit

Permalink
check bounds in getChildAt(i) to avoid NPEs
Browse files Browse the repository at this point in the history
  • Loading branch information
trxcllnt committed Jan 25, 2018
1 parent f1dead0 commit 8ddce0a
Show file tree
Hide file tree
Showing 5 changed files with 53 additions and 37 deletions.
2 changes: 1 addition & 1 deletion js/src/predicate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ export class Col<T= any> extends Value<T> {
}
if (this.colidx < 0) { throw new Error(`Failed to bind Col "${this.name}"`); }
}
this.vector = batch.getChildAt(this.colidx);
this.vector = batch.getChildAt(this.colidx)!;
return this.vector.get.bind(this.vector);
}

Expand Down
21 changes: 9 additions & 12 deletions js/src/recordbatch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
// specific language governing permissions and limitations
// under the License.

import { Schema, Struct } from './type';
import { Schema, Struct, DataType } from './type';
import { flatbuffers } from 'flatbuffers';
import { View, Vector, StructVector } from './vector';
import { Data, NestedData } from './data';
Expand All @@ -40,34 +40,31 @@ export class RecordBatch extends StructVector {
super(data, args[2]);
this.schema = args[0];
this.length = data.length;
this.numCols = this.schema.fields.length;
} else {
const [schema, numRows, cols] = args;
const columns: Vector<any>[] = new Array(cols.length);
const columnsData: Data<any>[] = new Array(cols.length);
const childData: Data<any>[] = new Array(cols.length);
for (let index = -1, length = cols.length; ++index < length;) {
const col: Data<any> | Vector = cols[index];
if (col instanceof Vector) {
columnsData[index] = (columns[index] = col as Vector).data;
} else {
columns[index] = Vector.create(columnsData[index] = col);
}
childData[index] = col instanceof Vector ? col.data : col;
}
super(new NestedData(new Struct(schema.fields), numRows, null, columnsData));
super(new NestedData(new Struct(schema.fields), numRows, null, childData));
this.schema = schema;
this.length = numRows;
this.numCols = schema.fields.length;
}
this.numCols = this.schema.fields.length;
}
public clone<R extends Struct>(data: Data<R>, view: View<R> = this.view.clone(data)): this {
return new RecordBatch(this.schema, data as any, view) as any;
}
public getChildAt<R extends DataType = DataType>(index: number): Vector<R> | null {
return index < 0 || index >= this.numCols ? null : super.getChildAt<R>(index);
}
public select(...columnNames: string[]) {
const fields = this.schema.fields;
const namesToKeep = columnNames.reduce((xs, x) => (xs[x] = true) && xs, Object.create(null));
return new RecordBatch(
this.schema.select(...columnNames), this.length,
this.childData.filter((_, index) => namesToKeep[fields[index].name])
this.childData.filter((_, i) => namesToKeep[fields[i].name])
);
}
}
15 changes: 8 additions & 7 deletions js/src/table.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,9 @@ export class Table implements DataFrame {
static fromStruct(struct: StructVector) {
const schema = new Schema(struct.type.children);
const chunks = struct.view instanceof ChunkedView ?
(struct.view.childVectors as StructVector[]) :
(struct.view.chunkVectors as StructVector[]) :
[struct];
return new Table(chunks.map((chunk)=>new RecordBatch(schema, chunk.length, chunk.view.childData)));
return new Table(chunks.map((chunk) => new RecordBatch(schema, chunk.length, chunk.view.childData)));
}

public readonly schema: Schema;
Expand Down Expand Up @@ -102,7 +102,6 @@ export class Table implements DataFrame {
this.schema = schema;
this.batches = batches;
this.batchesUnion = batches.reduce((union, batch) => union.concat(batch));
// this.columns = schema.fields.map((_, i) => this.batchesUnion.getChildAt(i));
this.length = this.batchesUnion.length;
this.numCols = this.batchesUnion.numCols;
}
Expand All @@ -113,8 +112,10 @@ export class Table implements DataFrame {
return this.getColumnAt(this.getColumnIndex(name));
}
public getColumnAt(index: number) {
return this._columns[index] || (
this._columns[index] = this.batchesUnion.getChildAt(index));
return index < 0 || index >= this.numCols
? null
: this._columns[index] || (
this._columns[index] = this.batchesUnion.getChildAt(index)!);
}
public getColumnIndex(name: string) {
return this.schema.fields.findIndex((f) => f.name === name);
Expand Down Expand Up @@ -271,8 +272,8 @@ export class CountByResult extends Table implements DataFrame {
));
}
public toJSON(): Object {
const values = this.getColumnAt(0);
const counts = this.getColumnAt(1);
const values = this.getColumnAt(0)!;
const counts = this.getColumnAt(1)!;
const result = {} as { [k: string]: number | null };
for (let i = -1; ++i < this.length;) {
result[values.get(i)] = counts.get(i);
Expand Down
4 changes: 2 additions & 2 deletions js/src/vector.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@ export class Vector<T extends DataType = any> implements VectorLike, View<T>, Vi
public static create<T extends DataType>(data: Data<T>): Vector<T> {
return createVector(data);
}
public static concat<T extends DataType>(...sources: Vector<T>[]): Vector<T> {
return sources.length === 1 ? sources[0] : sources.reduce((a, b) => a.concat(b));
public static concat<T extends DataType>(source?: Vector<T> | null, ...others: Vector<T>[]): Vector<T> {
return others.reduce((a, b) => a ? a.concat(b) : b, source!);
}
public type: T;
public length: number;
Expand Down
48 changes: 33 additions & 15 deletions js/src/vector/nested.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,15 @@ export abstract class NestedView<T extends NestedType> implements View<T> {
public length: number;
public numChildren: number;
public childData: Data<any>[];
protected _childColumns: Vector<any>[];
protected _children: Vector<any>[];
constructor(data: Data<T>, children?: Vector<any>[]) {
this.length = data.length;
this.childData = data.childData;
this.numChildren = data.childData.length;
this._childColumns = children || new Array(this.numChildren);
this._children = children || new Array(this.numChildren);
}
public clone(data: Data<T>): this {
return new (<any> this.constructor)(data, this._childColumns) as this;
return new (<any> this.constructor)(data, this._children) as this;
}
public isValid(): boolean {
return true;
Expand All @@ -52,9 +52,11 @@ export abstract class NestedView<T extends NestedType> implements View<T> {
}
protected abstract getNested(self: NestedView<T>, index: number): T['TValue'];
protected abstract setNested(self: NestedView<T>, index: number, value: T['TValue']): void;
public getChildAt<R extends DataType = DataType>(index: number) {
return this._childColumns[index] || (
this._childColumns[index] = Vector.create<R>(this.childData[index]));
public getChildAt<R extends DataType = DataType>(index: number): Vector<R> | null {
return index < 0 || index >= this.numChildren
? null
: (this._children[index] as Vector<R>) ||
(this._children[index] = Vector.create<R>(this.childData[index]));
}
public *[Symbol.iterator](): IterableIterator<T['TValue']> {
const get = this.getNested;
Expand Down Expand Up @@ -120,14 +122,22 @@ export class DenseUnionView extends UnionView<DenseUnion> {

export class StructView extends NestedView<Struct> {
protected getNested(self: StructView, index: number) {
return new RowView(self as any, self._childColumns, index);
return new RowView(self as any, self._children, index);
}
protected setNested(self: StructView, index: number, value: any): void {
let idx = -1, len = self.numChildren;
let idx = -1, len = self.numChildren, child: Vector | null;
if (!(value instanceof NestedView || value instanceof Vector)) {
while (++idx < len) { self.getChildAt(idx).set(index, value[idx]); }
while (++idx < len) {
if (child = self.getChildAt(idx)) {
child.set(index, value[idx]);
}
}
} else {
while (++idx < len) { self.getChildAt(idx).set(index, value.get(idx)); }
while (++idx < len) {
if (child = self.getChildAt(idx)) {
child.set(index, value.get(idx));
}
}
}
}
}
Expand All @@ -140,14 +150,22 @@ export class MapView extends NestedView<Map_> {
(xs[x.name] = i) && xs || xs, Object.create(null));
}
protected getNested(self: MapView, index: number) {
return new MapRowView(self as any, self._childColumns, index);
return new MapRowView(self as any, self._children, index);
}
protected setNested(self: MapView, index: number, value: { [k: string]: any }): void {
const typeIds = self.typeIds as any;
let typeIds = self.typeIds as any, child: Vector | null;
if (!(value instanceof NestedView || value instanceof Vector)) {
for (const key in typeIds) { self.getChildAt(typeIds[key]).set(index, value[key]); }
for (const key in typeIds) {
if (child = self.getChildAt(typeIds[key])) {
child.set(index, value[key]);
}
}
} else {
for (const key in typeIds) { self.getChildAt(typeIds[key]).set(index, value.get(key as any)); }
for (const key in typeIds) {
if (child = self.getChildAt(typeIds[key])) {
child.set(index, value.get(key as any));
}
}
}
}
}
Expand All @@ -160,7 +178,7 @@ export class RowView extends UnionView<SparseUnion> {
this.length = data.numChildren;
}
public clone(data: Data<SparseUnion> & NestedView<any>): this {
return new (<any> this.constructor)(data, this._childColumns, this.rowIndex) as this;
return new (<any> this.constructor)(data, this._children, this.rowIndex) as this;
}
protected getChildValue(self: RowView, index: number, _typeIds: any, _valueOffsets?: any): any | null {
const child = self.getChildAt(index);
Expand Down

0 comments on commit 8ddce0a

Please sign in to comment.