This is an automated email from the ASF dual-hosted git repository.
tustvold pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/arrow-rs.git
The following commit(s) were added to refs/heads/master by this push:
new d9206ba83fe Deprecate NullBuilder capacity, as it behaves in a
surprising way (#5721)
d9206ba83fe is described below
commit d9206ba83fe9739772029577b66ba415c8494021
Author: Hadrien G <[email protected]>
AuthorDate: Sat May 4 21:53:16 2024 +0200
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 | 7 +++++--
arrow-array/src/builder/null_builder.rs | 12 +++++++-----
arrow-array/src/builder/struct_builder.rs | 2 +-
arrow-csv/src/reader/mod.rs | 8 ++++++--
4 files changed, 19 insertions(+), 10 deletions(-)
diff --git a/arrow-array/src/array/null_array.rs
b/arrow-array/src/array/null_array.rs
index af3ec0b57d2..88cc2d911f8 100644
--- a/arrow-array/src/array/null_array.rs
+++ b/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()
}
}
diff --git a/arrow-array/src/builder/null_builder.rs
b/arrow-array/src/builder/null_builder.rs
index 53a6b103d54..59086dffa90 100644
--- a/arrow-array/src/builder/null_builder.rs
+++ b/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());
}
}
diff --git a/arrow-array/src/builder/struct_builder.rs
b/arrow-array/src/builder/struct_builder.rs
index 1e2e402f745..46202bfa395 100644
--- a/arrow-array/src/builder/struct_builder.rs
+++ b/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)),
diff --git a/arrow-csv/src/reader/mod.rs b/arrow-csv/src/reader/mod.rs
index 943bd890536..4d7d60e10cf 100644
--- a/arrow-csv/src/reader/mod.rs
+++ b/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| {
