wiedld commented on code in PR #6781:
URL: https://github.com/apache/arrow-rs/pull/6781#discussion_r1872590283
##########
arrow-array/src/array/union_array.rs:
##########
@@ -1966,10 +1972,13 @@ mod tests {
let array = UnionArray::try_new(fields.clone(), type_ids, None,
children).unwrap();
- let result = array.logical_nulls();
+ let expected = BooleanBuffer::from(vec![false, false, true, false]);
- let expected = NullBuffer::from(vec![false, false, true, false]);
- assert_eq!(Some(expected), result);
+ assert_eq!(expected, array.logical_nulls().unwrap().into_inner());
+ assert_eq!(
+ expected,
+
array.mask_sparse_all_with_nulls_skip_one(array.fields_logical_nulls())
Review Comment:
Each strategy for computing the logical_nulls are [outlined
here](https://github.com/apache/arrow-rs/pull/6781#pullrequestreview-2483594275).
The code determines a [cost
estimate](https://github.com/apache/arrow-rs/blob/30c14abd3340735e5e4ab9375628f8d8ba7223b4/arrow-array/src/array/union_array.rs#L825-L836)
to decide which strategy to use. Then it calls a helper function per each
strategy => and that did the computation.
With the way the tests were written before, it always got the same cost
estimate on certain architecture. For example, tests running on ARM would
always get the same cost estimate and therefore always pick the same strategy
(`UnionArray::gather_nulls` helper function). So even though this test is named
`test_sparse_union_logical_nulls_mask_all_nulls_skip_one`, it was not actually
testing that strategy and helper function.
With this change, the tests are directly calling each helper function.
In the example above, the test named
`test_sparse_union_logical_nulls_mask_all_nulls_skip_one` has been changed to
directly calling the helper method
`UnionArray::mask_sparse_all_with_nulls_skip_one`.
--
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]
