Jefffrey commented on code in PR #3450:
URL: https://github.com/apache/arrow-rs/pull/3450#discussion_r1061128427
##########
arrow-array/src/builder/map_builder.rs:
##########
@@ -143,56 +142,49 @@ impl<K: ArrayBuilder, V: ArrayBuilder> MapBuilder<K, V> {
/// Builds the [`MapArray`]
pub fn finish(&mut self) -> MapArray {
let len = self.len();
-
// Build the keys
let keys_arr = self.key_builder.finish();
let values_arr = self.value_builder.finish();
-
- let keys_field = Field::new(
- self.field_names.key.as_str(),
- keys_arr.data_type().clone(),
- false, // always nullable
- );
- let values_field = Field::new(
- self.field_names.value.as_str(),
- values_arr.data_type().clone(),
- true,
- );
-
- let struct_array =
- StructArray::from(vec![(keys_field, keys_arr), (values_field,
values_arr)]);
-
let offset_buffer = self.offsets_builder.finish();
- let null_bit_buffer = self.null_buffer_builder.finish();
self.offsets_builder.append(0);
- let map_field = Box::new(Field::new(
- self.field_names.entry.as_str(),
- struct_array.data_type().clone(),
- false, // always non-nullable
- ));
- let array_data = ArrayData::builder(DataType::Map(map_field, false))
// TODO: support sorted keys
- .len(len)
- .add_buffer(offset_buffer)
- .add_child_data(struct_array.into_data())
- .null_bit_buffer(null_bit_buffer);
-
- let array_data = unsafe { array_data.build_unchecked() };
+ let null_bit_buffer = self.null_buffer_builder.finish();
- MapArray::from(array_data)
+ self.finish_helper(keys_arr, values_arr, offset_buffer,
null_bit_buffer, len)
}
/// Builds the [`MapArray`] without resetting the builder.
pub fn finish_cloned(&self) -> MapArray {
let len = self.len();
-
// Build the keys
let keys_arr = self.key_builder.finish_cloned();
let values_arr = self.value_builder.finish_cloned();
+ let offset_buffer =
Buffer::from_slice_ref(self.offsets_builder.as_slice());
+ let null_bit_buffer = self
+ .null_buffer_builder
+ .as_slice()
+ .map(Buffer::from_slice_ref);
+
+ self.finish_helper(keys_arr, values_arr, offset_buffer,
null_bit_buffer, len)
+ }
+
+ fn finish_helper(
+ &self,
+ keys_arr: Arc<dyn Array>,
+ values_arr: Arc<dyn Array>,
+ offset_buffer: Buffer,
+ null_bit_buffer: Option<Buffer>,
+ len: usize,
+ ) -> MapArray {
+ assert!(
+ keys_arr.null_count() == 0,
+ "Keys array must have no null values, found {} null value(s)",
+ keys_arr.null_count()
+ );
Review Comment:
ideally would return an Err instead of panicking, but would change signature
of finish & finish_cloned, unsure if that is desirable? especially since other
structs like StructBuilder has a similar signature (finish doesn't return a
Result, just the StructArray)
--
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]
