tustvold commented on code in PR #6966:
URL: https://github.com/apache/arrow-rs/pull/6966#discussion_r1911956254
##########
arrow-data/src/data.rs:
##########
@@ -1877,51 +1887,78 @@ impl ArrayDataBuilder {
/// Creates an array data, without any validation
///
+ /// This is shorthand for `self.with_skip_validation(true).build()`
+ ///
/// # Safety
///
/// The same caveats as [`ArrayData::new_unchecked`]
/// apply.
- #[allow(clippy::let_and_return)]
pub unsafe fn build_unchecked(self) -> ArrayData {
- let data = self.build_impl();
- // Provide a force_validate mode
- #[cfg(feature = "force_validate")]
- data.validate_data().unwrap();
- data
+ self.with_skip_validation(true).build().unwrap()
}
- /// Same as [`Self::build_unchecked`] but ignoring `force_validate`
feature flag
- unsafe fn build_impl(self) -> ArrayData {
- let nulls = self
- .nulls
+ /// Creates an `ArrayData`, consuming `self`
+ pub fn build(self) -> Result<ArrayData, ArrowError> {
+ let Self {
+ data_type,
+ len,
+ null_count,
+ null_bit_buffer,
+ nulls,
+ offset,
+ buffers,
+ child_data,
+ align_buffers,
+ skip_validation,
+ } = self;
+
+ let nulls = nulls
.or_else(|| {
- let buffer = self.null_bit_buffer?;
- let buffer = BooleanBuffer::new(buffer, self.offset, self.len);
- Some(match self.null_count {
- Some(n) => NullBuffer::new_unchecked(buffer, n),
+ let buffer = null_bit_buffer?;
+ let buffer = BooleanBuffer::new(buffer, offset, len);
+ Some(match null_count {
+ Some(n) => {
+ // SAFETY: if validation is on, call to
`data.validate_data()` will validate the null buffer
+ unsafe { NullBuffer::new_unchecked(buffer, n) }
+ }
None => NullBuffer::new(buffer),
})
})
.filter(|b| b.null_count() != 0);
- ArrayData {
- data_type: self.data_type,
- len: self.len,
- offset: self.offset,
- buffers: self.buffers,
- child_data: self.child_data,
+ let mut data = ArrayData {
+ data_type,
+ len,
+ offset,
+ buffers,
+ child_data,
nulls,
+ };
+
+ if align_buffers {
+ data.align_buffers();
}
- }
- /// Creates an array data, validating all inputs
- pub fn build(self) -> Result<ArrayData, ArrowError> {
- let data = unsafe { self.build_impl() };
- data.validate_data()?;
+ #[cfg(feature = "force_validate")]
+ let force_validation = true;
+ #[cfg(not(feature = "force_validate"))]
+ let force_validation = false;
Review Comment:
```suggestion
let force_validation = cfg!(feature = "force_validate");
```
I believe should work
--
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]
