Skip to content

Commit

Permalink
ARROW-16209: [JS] Support setting arbitrary symbols on Tables
Browse files Browse the repository at this point in the history
Fixes vega/vega-lite#8105

Similar to apache#12906 but only allows symbols and only allows them on structs.

Closes apache#12907 from domoritz/dom/set-symbol

Authored-by: Dominik Moritz <[email protected]>
Signed-off-by: Dominik Moritz <[email protected]>
  • Loading branch information
domoritz committed Apr 19, 2022
1 parent 5eb8957 commit eedf695
Show file tree
Hide file tree
Showing 6 changed files with 60 additions and 8 deletions.
2 changes: 1 addition & 1 deletion js/src/Arrow.dom.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ RecordBatchFileWriter['throughDOM'] = recordBatchWriterThroughDOMStream;
RecordBatchStreamWriter['throughDOM'] = recordBatchWriterThroughDOMStream;

export type {
TypeMap,
TypeMap, StructRowProxy,
ReadableSource, WritableSink,
ArrowJSONLike, FileHandle, Readable, Writable, ReadableWritable, ReadableDOMStreamOptions,
} from './Arrow.js';
Expand Down
1 change: 1 addition & 0 deletions js/src/Arrow.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ export { Schema, Field } from './schema.js';

export { MapRow } from './row/map.js';
export { StructRow } from './row/struct.js';
export type { StructRowProxy } from './row/struct.js';

export { Builder } from './builder.js';
export { makeBuilder, vectorFromArray, builderThroughIterable, builderThroughAsyncIterable } from './factories.js';
Expand Down
8 changes: 4 additions & 4 deletions js/src/builder/struct.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,11 @@ import { Struct, TypeMap } from '../type.js';
/** @ignore */
export class StructBuilder<T extends TypeMap = any, TNull = any> extends Builder<Struct<T>, TNull> {
public setValue(index: number, value: Struct<T>['TValue']) {
const children = this.children;
const { children, type } = this;
switch (Array.isArray(value) || value.constructor) {
case true: return this.type.children.forEach((_, i) => children[i].set(index, value[i]));
case Map: return this.type.children.forEach((f, i) => children[i].set(index, value.get(f.name)));
default: return this.type.children.forEach((f, i) => children[i].set(index, value[f.name]));
case true: return type.children.forEach((_, i) => children[i].set(index, value[i]));
case Map: return type.children.forEach((f, i) => children[i].set(index, value.get(f.name)));
default: return type.children.forEach((f, i) => children[i].set(index, value[f.name]));
}
}

Expand Down
4 changes: 3 additions & 1 deletion js/src/row/struct.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ import { instance as setVisitor } from '../visitor/set.js';

export type StructRowProxy<T extends TypeMap = any> = StructRow<T> & {
[P in keyof T]: T[P]['TValue'];
} & {
[key: symbol]: any;
};

export class StructRow<T extends TypeMap = any> {
Expand Down Expand Up @@ -149,7 +151,7 @@ class StructRowProxyHandler<T extends TypeMap = any> implements ProxyHandler<Str
setVisitor.visit(row[kParent].children[idx], row[kRowIndex], val);
// Cache key/val lookups
return Reflect.set(row, key, val);
} else if (Reflect.has(row, key)) {
} else if (Reflect.has(row, key) || typeof key === 'symbol') {
return Reflect.set(row, key, val);
}
return false;
Expand Down
4 changes: 2 additions & 2 deletions js/src/visitor/get.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import { BN } from '../util/bn.js';
import { Vector } from '../vector.js';
import { Visitor } from '../visitor.js';
import { MapRow } from '../row/map.js';
import { StructRow } from '../row/struct.js';
import { StructRow, StructRowProxy } from '../row/struct.js';
import { decodeUtf8 } from '../util/utf8.js';
import { TypeToDataType } from '../interfaces.js';
import { uint16ToFloat64 } from '../util/math.js';
Expand Down Expand Up @@ -228,7 +228,7 @@ const getMap = <T extends Map_>(data: Data<T>, index: number): T['TValue'] => {

/** @ignore */
const getStruct = <T extends Struct>(data: Data<T>, index: number): T['TValue'] => {
return new StructRow(data, index);
return new StructRow(data, index) as StructRowProxy<T['TValue']>;
};

/* istanbul ignore next */
Expand Down
49 changes: 49 additions & 0 deletions js/test/unit/row/struct-tests.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
// Licensed to the Apache Software Foundation (ASF) under one
// or more contributor license agreements. See the NOTICE file
// distributed with this work for additional information
// regarding copyright ownership. The ASF licenses this file
// to you under the Apache License, Version 2.0 (the
// "License"); you may not use this file except in compliance
// with the License. You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing,
// software distributed under the License is distributed on an
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
// KIND, either express or implied. See the License for the
// specific language governing permissions and limitations
// under the License.

import { Field, Float32, makeData, Struct, StructRow, StructRowProxy } from 'apache-arrow';

function makeStructRow() {
const struct = makeData({
type: new Struct<{ foo: Float32 }>([
new Field('foo', new Float32())
]),
});
return new StructRow(struct, 0) as StructRowProxy<{ foo: Float32 }>;
}

describe('StructRow', () => {
test('Can set existing property', () => {
const row = makeStructRow();
row.foo = 42;
expect(row.foo).toBe(42);
});

test('Can set arbitrary symbols', () => {
const row = makeStructRow();
const s = Symbol.for('mySymbol');
row[s] = 42;
expect(row[s]).toBe(42);
});

test('Cannot set arbitrary property', () => {
const row = makeStructRow();
expect(() => {
(row as any).bar = 42;
}).toThrow();
});
});

0 comments on commit eedf695

Please sign in to comment.