alamb commented on code in PR #6966:
URL: https://github.com/apache/arrow-rs/pull/6966#discussion_r1911458541
##########
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;
+ // force validation in testing mode
+ let skip_validation = skip_validation && !force_validation;
+
+ if !skip_validation {
+ data.validate_data()?;
+ }
Ok(data)
}
/// Creates an array data, validating all inputs, and aligning any buffers
+ #[deprecated(since = "54.1.0", note = "Use ArrayData::with_align_buffers
instead")]
Review Comment:
deprecated one API in favor of setting a flag
##########
arrow-data/src/data.rs:
##########
@@ -1786,6 +1790,10 @@ pub struct ArrayDataBuilder {
offset: usize,
buffers: Vec<Buffer>,
child_data: Vec<ArrayData>,
+ /// Should buffers be realigned (copying if necessary)? Defaults to false.
Review Comment:
This is the point of this PR -- to add these flags to the builder directly
and then check them during calls to `build()`
--
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]
