alamb commented on code in PR #5612:
URL: https://github.com/apache/arrow-rs/pull/5612#discussion_r1557585888
##########
arrow-array/src/builder/fixed_size_list_builder.rs:
##########
@@ -169,116 +168,52 @@ where
/// Builds the [`FixedSizeListBuilder`] and reset this builder.
pub fn finish(&mut self) -> FixedSizeListArray {
let len = self.len();
- let values_arr = self.values_builder.finish();
- let values_data = values_arr.to_data();
+ let values = self.values_builder.finish();
+ let nulls = self.null_buffer_builder.finish();
assert_eq!(
- values_data.len(), len * self.list_len as usize,
+ values.len(), len * self.list_len as usize,
"Length of the child array ({}) must be the multiple of the value
length ({}) and the array length ({}).",
- values_data.len(),
+ values.len(),
self.list_len,
len,
);
- let nulls = self.null_buffer_builder.finish();
+ let field = self
+ .field
+ .clone()
+ .unwrap_or_else(|| Arc::new(Field::new("item",
values.data_type().clone(), true)));
- let field = match &self.field {
- Some(f) => {
- let size = self.value_length();
- assert_eq!(
- f.data_type(),
- values_data.data_type(),
- "DataType of field ({}) should be the same as the
values_builder DataType ({})",
- f.data_type(),
- values_data.data_type()
- );
-
- if let Some(a) = values_arr.logical_nulls() {
- let nulls_valid = f.is_nullable()
- || nulls
- .as_ref()
- .map(|n| n.expand(size as _).contains(&a))
- .unwrap_or_default();
-
- assert!(
- nulls_valid,
- "Found unmasked nulls for non-nullable
FixedSizeListBuilder field {:?}",
- f.name()
- );
- }
- f.clone()
- }
- None => Arc::new(Field::new("item",
values_data.data_type().clone(), true)),
- };
-
- let array_data = ArrayData::builder(DataType::FixedSizeList(field,
self.list_len))
- .len(len)
- .add_child_data(values_data)
- .nulls(nulls);
-
- let array_data = unsafe { array_data.build_unchecked() };
-
- FixedSizeListArray::from(array_data)
+ FixedSizeListArray::new(field, self.list_len, values, nulls)
}
/// Builds the [`FixedSizeListBuilder`] without resetting the builder.
pub fn finish_cloned(&self) -> FixedSizeListArray {
let len = self.len();
- let values_arr = self.values_builder.finish_cloned();
- let values_data = values_arr.to_data();
+ let values = self.values_builder.finish_cloned();
+ let nulls = self.null_buffer_builder.finish_cloned();
assert_eq!(
- values_data.len(), len * self.list_len as usize,
+ values.len(), len * self.list_len as usize,
"Length of the child array ({}) must be the multiple of the value
length ({}) and the array length ({}).",
- values_data.len(),
+ values.len(),
self.list_len,
len,
);
- let nulls = self.null_buffer_builder.finish_cloned();
+ let field = self
+ .field
+ .clone()
+ .unwrap_or_else(|| Arc::new(Field::new("item",
values.data_type().clone(), true)));
- let field = match &self.field {
- Some(f) => {
- let size = self.value_length();
- assert_eq!(
- f.data_type(),
- values_data.data_type(),
- "DataType of field ({}) should be the same as the
values_builder DataType ({})",
- f.data_type(),
- values_data.data_type()
- );
- if let Some(a) = values_arr.logical_nulls() {
- let nulls_valid = f.is_nullable()
- || nulls
- .as_ref()
- .map(|n| n.expand(size as _).contains(&a))
- .unwrap_or_default();
-
- assert!(
- nulls_valid,
- "Found unmasked nulls for non-nullable
FixedSizeListBuilder field {:?}",
- f.name()
- );
- }
- f.clone()
- }
- None => Arc::new(Field::new("item",
values_data.data_type().clone(), true)),
- };
-
- let array_data = ArrayData::builder(DataType::FixedSizeList(field,
self.list_len))
- .len(len)
- .add_child_data(values_data)
- .nulls(nulls);
-
- let array_data = unsafe { array_data.build_unchecked() };
-
- FixedSizeListArray::from(array_data)
+ FixedSizeListArray::new(field, self.list_len, values, nulls)
Review Comment:
It is great to use consolidate the validation logic 👍
--
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]
