This is an automated email from the ASF dual-hosted git repository.
brycemecum pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow.git
The following commit(s) were added to refs/heads/main by this push:
new 25d5c055ce GH-45862: [JS] Fix FixedSizeListBuilder behavior for null
slots (#45889)
25d5c055ce is described below
commit 25d5c055ceb55aaad12446da7e65bfec3530d6b3
Author: Bryce Mecum <[email protected]>
AuthorDate: Tue Mar 25 06:04:17 2025 -0700
GH-45862: [JS] Fix FixedSizeListBuilder behavior for null slots (#45889)
### Rationale for this change
`vectorFromArray` could produce an incorrect result when used on a
`FixedSizeList<T>` with one or more null slots in trailing positions.
### What changes are included in this PR?
Fixed behavior of the FixedSizeListBuilder.
### Are these changes tested?
Yes. This PR includes specific test cases to cover this bug fix.
### Are there any user-facing changes?
No.
* GitHub Issue: #45862
Lead-authored-by: Bryce Mecum <[email protected]>
Co-authored-by: Paul Taylor <[email protected]>
Signed-off-by: Bryce Mecum <[email protected]>
---
js/src/builder/fixedsizelist.ts | 8 +++++++-
js/test/unit/vector/vector-tests.ts | 39 ++++++++++++++++++++++++++++++++++++-
2 files changed, 45 insertions(+), 2 deletions(-)
diff --git a/js/src/builder/fixedsizelist.ts b/js/src/builder/fixedsizelist.ts
index f4b4b95df2..95df0ce901 100644
--- a/js/src/builder/fixedsizelist.ts
+++ b/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]);
}
}
+ public setValid(index: number, valid: boolean) {
+ if (!super.setValid(index, valid)) {
+ this.children[0].setValid((index + 1) * this.stride - 1, false);
+ }
+ return valid;
+ }
public addChild(child: Builder<T>, name = '0') {
if (this.numChildren > 0) {
throw new Error('FixedSizeListBuilder can only have one child.');
diff --git a/js/test/unit/vector/vector-tests.ts
b/js/test/unit/vector/vector-tests.ts
index c05838d7a9..9dadaf7c63 100644
--- a/js/test/unit/vector/vector-tests.ts
+++ b/js/test/unit/vector/vector-tests.ts
@@ -16,7 +16,7 @@
// under the License.
import {
- Bool, DateDay, DateMillisecond, Dictionary, Float64, Int32, List,
makeVector, Struct, Utf8, LargeUtf8, util, Vector, vectorFromArray, makeData
+ Bool, DateDay, DateMillisecond, Dictionary, Float64, Int32, List,
makeVector, Struct, Utf8, LargeUtf8, util, Vector, vectorFromArray, makeData,
FixedSizeList, Field,
} from 'apache-arrow';
describe(`makeVectorFromArray`, () => {
@@ -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);
+ });
+
+ test(`correct child length with null slot last`, () => {
+ let vector = vectorFromArray(
+ [[1, 2, 3], null],
+ new FixedSizeList(3, new Field('item', new Int32())),
+ );
+ let child = vector.getChildAt(0);
+
+ expect(child).toHaveLength(6);
+ expect(child?.nullCount).toBe(3);
+ });
+
+ test(`correct child length with null in the middle`, () => {
+ let vector = vectorFromArray(
+ [[1, 2, 3], null, [7, 8, 9]],
+ new FixedSizeList(3, new Field('item', new Int32())),
+ );
+ let child = vector.getChildAt(0);
+
+ expect(child).toHaveLength(9);
+ expect(child?.nullCount).toBe(3);
+ });
+});
