Rich-T-kid commented on code in PR #12150:
URL: https://github.com/apache/arrow-rs/pull/12150#discussion_r4557173505
##########
arrow-data/src/transform/mod.rs:
##########
@@ -856,29 +906,59 @@ impl<'a> MutableArrayData<'a> {
/// # Errors
///
/// Returns an error if this [`MutableArrayData`] was not created with
`use_nulls` and none
- /// of the source arrays are nullable, or if the run-end counter overflows.
+ /// of the source arrays are nullable, or if the run-end counter
overflows. `Null` arrays
+ /// are the exception because their slots are always null and need no
validity bitmap.
pub fn try_extend_nulls(&mut self, len: usize) -> Result<(), ArrowError> {
- if self.data.null_buffer.is_none() {
+ self.try_extend_nulls_inner(len, true)
+ }
+
+ /// Extends this array with `len` nulls.
+ ///
+ /// When `mark_null` is false, the newly added values are placeholders that
+ /// are covered by a parent null mask. They are kept valid so that no child
+ /// validity bitmap is required.
+ fn try_extend_nulls_inner(&mut self, len: usize, mark_null: bool) ->
Result<(), ArrowError> {
+ if self.data.data_type == DataType::Null {
+ // Every slot of a `Null` array is already null and it never has a
+ // validity bitmap.
+ self.data.len += len;
+ self.data.null_count += len;
+ return Ok(());
+ }
+
+ if len == 0 {
+ return Ok(());
+ }
+
+ // Dictionary keys cannot be arbitrary: keep them null so validation
+ // does not attempt to interpret placeholder keys.
+ let mark_null = mark_null || matches!(self.data.data_type,
DataType::Dictionary(_, _));
+
+ if mark_null && self.data.null_buffer.is_none() {
return Err(ArrowError::InvalidArgumentError(
"MutableArrayData cannot be extended with nulls: it was
created with `use_nulls` \
set to false and no source array is nullable"
.to_owned(),
));
}
- if len == 0 {
- return Ok(());
- }
-
+ let start = self.data.len;
(self.extend_nulls)(&mut self.data, len)?;
-
self.data.len += len;
- let bit_len = bit_util::ceil(self.data.len, 8);
- let nulls = self.data.null_buffer();
- nulls
- .try_resize(bit_len, 0)
- .map_err(|e| ArrowError::MemoryError(e.to_string()))?;
- self.data.null_count += len;
+
+ if let Some(nulls) = self.data.null_buffer.as_mut() {
+ utils::resize_for_bits(nulls, self.data.len);
Review Comment:
`try_extend_nulls` previously used `MutableBuffer::try_resize` and returned
`ArrowError::MemoryError` when growing the validity buffer failed.
`resize_for_bits` calls the infallible `resize`, so this now panics on the same
allocation failure. Since this is the fallible API—and this change is meant to
handle hostile allocation sizes can we keep this resize fallible, either here
or with a fallible `resize_for_bits` helper?
--
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]
