Deprecate NullBuilder capacity, as it behaves in a surprising way (#5721

)

* Deprecate NullBuilder capacity, as it behaves in a surprising way

* Ignore capacity

* Forgot one use of NullArray::builder

arrow-array/src/array/null_array.rs

@@ -67,8 +67,11 @@ impl NullArray {
     }
 
     /// Returns a new null array builder
-    pub fn builder(capacity: usize) -> NullBuilder {
-        NullBuilder::with_capacity(capacity)
+    ///
+    /// Note that the `capacity` parameter to this function is _deprecated_. It
+    /// now does nothing, and will be removed in a future version.
+    pub fn builder(_capacity: usize) -> NullBuilder {
+        NullBuilder::new()
     }
 }
 

arrow-array/src/builder/null_builder.rs

@@ -60,11 +60,13 @@ impl NullBuilder {
     }
 
     /// Creates a new null builder with space for `capacity` elements without re-allocating
-    pub fn with_capacity(capacity: usize) -> Self {
-        Self { len: capacity }
+    #[deprecated = "there is no actual notion of capacity in the NullBuilder, so emulating it makes little sense"]
+    pub fn with_capacity(_capacity: usize) -> Self {
+        Self::new()
     }
 
     /// Returns the capacity of this builder measured in slots of type `T`
+    #[deprecated = "there is no actual notion of capacity in the NullBuilder, so emulating it makes little sense"]
     pub fn capacity(&self) -> usize {
         self.len
     }
@@ -158,7 +160,7 @@ mod tests {
         builder.append_empty_values(4);
 
         let arr = builder.finish();
-        assert_eq!(20, arr.len());
+        assert_eq!(10, arr.len());
         assert_eq!(0, arr.offset());
         assert_eq!(0, arr.null_count());
         assert!(arr.is_nullable());
@@ -171,10 +173,10 @@ mod tests {
         builder.append_empty_value();
         builder.append_empty_values(3);
         let mut array = builder.finish_cloned();
-        assert_eq!(21, array.len());
+        assert_eq!(5, array.len());
 
         builder.append_empty_values(5);
         array = builder.finish();
-        assert_eq!(26, array.len());
+        assert_eq!(10, array.len());
     }
 }

arrow-array/src/builder/struct_builder.rs

@@ -168,7 +168,7 @@ impl ArrayBuilder for StructBuilder {
 pub fn make_builder(datatype: &DataType, capacity: usize) -> Box<dyn ArrayBuilder> {
     use crate::builder::*;
     match datatype {
-        DataType::Null => Box::new(NullBuilder::with_capacity(capacity)),
+        DataType::Null => Box::new(NullBuilder::new()),
         DataType::Boolean => Box::new(BooleanBuilder::with_capacity(capacity)),
         DataType::Int8 => Box::new(Int8Builder::with_capacity(capacity)),
         DataType::Int16 => Box::new(Int16Builder::with_capacity(capacity)),

arrow-csv/src/reader/mod.rs

@@ -125,7 +125,7 @@
 
 mod records;
 
-use arrow_array::builder::PrimitiveBuilder;
+use arrow_array::builder::{NullBuilder, PrimitiveBuilder};
 use arrow_array::types::*;
 use arrow_array::*;
 use arrow_cast::parse::{parse_decimal, string_to_datetime, Parser};
@@ -759,7 +759,11 @@ fn parse(
                         null_regex,
                     )
                 }
-                DataType::Null => Ok(Arc::new(NullArray::builder(rows.len()).finish()) as ArrayRef),
+                DataType::Null => Ok(Arc::new({
+                    let mut builder = NullBuilder::new();
+                    builder.append_nulls(rows.len());
+                    builder.finish()
+                }) as ArrayRef),
                 DataType::Utf8 => Ok(Arc::new(
                     rows.iter()
                         .map(|row| {