wjones127 commented on code in PR #45889:
URL: https://github.com/apache/arrow/pull/45889#discussion_r2010702679
##########
js/test/unit/vector/vector-tests.ts:
##########
@@ -323,3 +323,40 @@ function basicVectorTests(vector: Vector, values: any[],
extras: any[]) {
}
});
}
+
+// GH-45862: Make sure vectorFromArray produces the correct result for
+// FixedSizeList with null slots
+describe(`vecorFromArray() with FixedSizeList<T> and null slots`, () => {
+ test(`correct child length with null slot first`, () => {
+ let vector = vectorFromArray(
+ [null, [1, 2, 3]],
+ new FixedSizeList(3, new Field('item', new Int32())),
+ );
+ let child = vector.getChildAt(0);
+
+ expect(child).toHaveLength(6);
+ expect(child?.nullCount).toBe(3);
Review Comment:
One problem here: In Typescript, `Field()` constructor defaults to
non-nullable. So the child isn't supposed to have nulls in this case.
Technically, what I was expecting is that the child would just be filled
with arbitrary (zero?) values, while the outer array masked them out in the
bitmap. I think if the inner array is nullable, it's a good idea to pass down
the nulls. But if it's not supposed to be nullable, I think it could cause
problems with IPC if we do set nulls.
##########
js/src/builder/fixedsizelist.ts:
##########
@@ -24,10 +24,16 @@ export class FixedSizeListBuilder<T extends DataType = any,
TNull = any> extends
public setValue(index: number, value: T['TValue']) {
const [child] = this.children;
const start = index * this.stride;
- for (let i = -1, n = value.length; ++i < n;) {
+ for (let i = -1, n = this.stride; ++i < n;) {
child.set(start + i, value[i]);
}
}
Review Comment:
I'm probably missing where the nulls are handled. This seems like it would
error if `value` was `null`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
