trxcllnt commented on code in PR #45889:
URL: https://github.com/apache/arrow/pull/45889#discussion_r2009205402


##########
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;) {
-            child.set(start + i, value[i]);
+        for (let i = -1, n = this.stride; ++i < n;) {
+            // Use either the actual value or a null (if the slot is null)
+            const finalVal = value ? value[i] : null;
+            child.set(start + i, finalVal);
         }
     }
+    public setValid(index: number, valid: boolean) {
+        this.length = this._nulls.set(index, +valid).length;
+        return true;
+    }

Review Comment:
   Since the BitmapBuilder initializes its internal buffer with zeroes (i.e. 
nulls), how about instead of inserting a null `stride` number of times, we 
expand the null buffer `stride` number of slots in one call?
   
   ```suggestion
           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;
       }
   ```
   
   I tested the above change and it seems to work:
   ```shell
   $ node
   > const Arrow = require('./index.cjs')
   undefined
   > const fsl_t = new Arrow.FixedSizeList(3, new Arrow.Field('item', new 
Arrow.Int32()))
   undefined
   > ((v = Arrow.vectorFromArray([null, [1, 2, 3]], fsl_t)) => [v.length, 
v.getChildAt(0).length, v.getChildAt(0).nullCount, [...v.getChildAt(0)]])()
   [ 2, 6, 3, [ null, null, null, 1, 2, 3 ] ]
   > ((v = Arrow.vectorFromArray([[1, 2, 3], null], fsl_t)) => [v.length, 
v.getChildAt(0).length, v.getChildAt(0).nullCount, [...v.getChildAt(0)]])()
   [ 2, 6, 3, [ 1, 2, 3, null, null, null ] ]
   > ((v = Arrow.vectorFromArray([[1, 2, 3], null, [7, 8, 9]], fsl_t)) => 
[v.length, v.getChildAt(0).length, v.getChildAt(0).nullCount, 
[...v.getChildAt(0)]])()
   [ 3, 9, 3, [ 1, 2, 3, null, null, null, 7, 8, 9 ] ]
   ```



-- 
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]

Reply via email to