check bounds in getChildAt(i) to avoid NPEs

Author: trxcllnt

js/src/predicate.ts

@@ -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);
     }
 

js/src/recordbatch.ts

@@ -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';
@@ -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])
         );
     }
 }

js/src/table.ts

@@ -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;
@@ -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;
     }
@@ -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);
@@ -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);

js/src/vector.ts

@@ -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;

js/src/vector/nested.ts

@@ -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;
@@ -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;
@@ -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));
+                }
+            }
         }
     }
 }
@@ -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));
+                }
+            }
         }
     }
 }
@@ -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);