Skip to content

Commit

Permalink
ARROW-15705: [JavaScript] Allowing appending null on children in a St…
Browse files Browse the repository at this point in the history
…ructBuilder

When trying to add a null value to a StructBuilder, the code previously only modified the null-bitmap.
I believe this breaks the spec as it results in the child arrays being a different length to the Struct array (and the child arrays should have the null value appended to their values).

Closes #12451 from Alfred-Mountfield/js-struct-initialisation

Lead-authored-by: Alfred Mountfield <[email protected]>
Co-authored-by: Dominik Moritz <[email protected]>
Signed-off-by: Dominik Moritz <[email protected]>
  • Loading branch information
Alfred-Mountfield and domoritz committed Apr 18, 2022
1 parent e0668e2 commit fae66cb
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 0 deletions.
9 changes: 9 additions & 0 deletions js/src/builder/struct.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,15 @@ export class StructBuilder<T extends TypeMap = any, TNull = any> extends Builder
default: return this.type.children.forEach((f, i) => children[i].set(index, value[f.name]));
}
}

/** @inheritdoc */
public setValid(index: number, valid: boolean) {
if (!super.setValid(index, valid)) {
this.children.forEach((child) => child.setValid(index, valid));
}
return valid;
}

public addChild(child: Builder, name = `${this.numChildren}`) {
const childIndex = this.children.push(child);
this.type = new Struct([...this.type.children, new Field(name, child.type, true)]);
Expand Down
38 changes: 38 additions & 0 deletions js/test/unit/builders/struct.tests.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
// 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, FixedSizeList, Float64, makeBuilder, Struct } from 'apache-arrow';

describe('StructBuilder', () => {
test('Appending Null', () => {
const listField = new Field('l', new FixedSizeList(2, new Field('item', new Float64())), true);

const builder = makeBuilder({
type: new Struct([listField]),
nullValues: [null, undefined]
});

builder.append(null);

expect(builder.numChildren).toBe(1);
expect(builder.nullCount).toBe(1);
expect(builder).toHaveLength(1);
expect(builder.children[0].numChildren).toBe(1);
expect(builder.children[0].nullCount).toBe(1);
expect(builder.children[0]).toHaveLength(1);
});
});

0 comments on commit fae66cb

Please sign in to comment.