Jefffrey commented on code in PR #10798:
URL: https://github.com/apache/arrow-rs/pull/10798#discussion_r3925327874
##########
arrow-array/src/array/fixed_size_binary_array.rs:
##########
@@ -1029,7 +1029,10 @@ mod tests {
}
#[test]
- #[should_panic(expected = "The child array cannot contain null values.")]
+ // No `expected`: the panic message differs by config. Under force_validate
+ // `build_unchecked` rejects the data; otherwise
`FixedSizeBinaryArray::from` does.
+ #[expect(clippy::should_panic_without_expect)]
Review Comment:
i think we should just gate this under not force_validate and keep the old
except; the idea being we want to test specifically
`FixedSizeBinaryArray::from` panic and not the arraydata panic
##########
arrow-array/src/array/run_array.rs:
##########
@@ -128,16 +128,7 @@ impl<R: RunEndIndexType> RunArray<R> {
.add_child_data(run_ends.to_data())
.add_child_data(values.to_data());
- // `build_unchecked` is used to avoid recursive validation of child
arrays.
- let array_data = unsafe { builder.build_unchecked() };
-
- // Safety: `validate_data` checks below
- // 1. The given array data has exactly two child arrays.
- // 2. The first child array (run_ends) has valid data type.
- // 3. run_ends array does not have null values
- // 4. run_ends array has non-zero and strictly increasing values.
- // 5. The length of run_ends array and values array are the same.
- array_data.validate_data()?;
+ let array_data = builder.build()?;
Review Comment:
i just merged https://github.com/apache/arrow-rs/pull/10807 which should
make changes to this file unnecessary hopefully (since it removes the arraydata
path)
--
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]
