This is an automated email from the ASF dual-hosted git repository.
Jefffrey pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-rs.git
The following commit(s) were added to refs/heads/main by this push:
new 38054b7713 fix(arrow-array): preserve type params in
`PrimitiveArray::into_builder` (#11321)
38054b7713 is described below
commit 38054b7713eade3bd8092bbf0fc3eaad69d4ce65
Author: Neil Conway <[email protected]>
AuthorDate: Thu Oct 1 22:19:47 2026 -0400
fix(arrow-array): preserve type params in `PrimitiveArray::into_builder`
(#11321)
# Which issue does this PR close?
- Follow-on to #11310
# Rationale for this change
#11310 made the `Err` path of `PrimitiveArray::into_builder` return the
array with its original data type. Looking closer, the `Ok` path still
built the builder with `PrimitiveBuilder::new_from_buffer`, which uses
`T::DATA_TYPE`, so `into_builder()?.finish()` lost type parameters.
These public APIs use `PrimitiveArray::into_builder` and will now behave
correctly:
- `PrimitiveArray::unary_mut` and `try_unary_mut`
- `binary_mut` and `try_binary_mut` in arrow-arith
- `DictionaryArray::unary_mut` and `into_primitive_dict_builder`
- `PrimitiveRunBuilder::finish_cloned`
# What changes are included in this PR?
* Ensure success path of `PrimitiveArray::into_builder` also preserves
type parameters
* Update unit tests
* Add new unit tests for some of the public APIs above, where the
behavior is non-trivial
# Are these changes tested?
Yes; existing tests pass, new test added.
# Are there any user-facing changes?
No, aside from the bug fix.
# AI usage
Developed with Claude Code (Fable 5.1).
---
arrow-array/src/array/dictionary_array.rs | 22 +++++++++++++++++++++-
arrow-array/src/array/primitive_array.rs | 18 +++++++++++++-----
arrow-array/src/builder/primitive_run_builder.rs | 11 +++++++++++
3 files changed, 45 insertions(+), 6 deletions(-)
diff --git a/arrow-array/src/array/dictionary_array.rs
b/arrow-array/src/array/dictionary_array.rs
index 6bd3e64bef..c561d3aa78 100644
--- a/arrow-array/src/array/dictionary_array.rs
+++ b/arrow-array/src/array/dictionary_array.rs
@@ -1068,7 +1068,7 @@ impl<K: ArrowDictionaryKeyType> AnyDictionaryArray for
DictionaryArray<K> {
mod tests {
use super::*;
use crate::cast::as_dictionary_array;
- use crate::{Int8Array, Int16Array, Int32Array, RunArray, UInt8Array};
+ use crate::{Decimal128Array, Int8Array, Int16Array, Int32Array, RunArray,
UInt8Array};
use arrow_buffer::{Buffer, ToByteSlice};
#[test]
@@ -1455,6 +1455,26 @@ mod tests {
assert_eq!(expected, returned);
}
+ #[test]
+ fn test_unary_mut_keeps_value_type() {
+ let values = || {
+ Decimal128Array::from(vec![123, 456])
+ .with_precision_and_scale(5, 2)
+ .unwrap()
+ };
+ let keys = Int8Array::from_iter_values([0, 1, 0]);
+ let dict = DictionaryArray::new(keys, Arc::new(values()));
+ let data_type = dict.data_type().clone();
+ let updated = dict.unary_mut::<_, Decimal128Type>(|x| x + 1).unwrap();
+ assert_eq!(updated.data_type(), &data_type);
+
+ // When the keys are shared, the original array is returned
+ let keys = Int8Array::from_iter_values([0, 1, 0]);
+ let dict = DictionaryArray::new(keys.clone(), Arc::new(values()));
+ let returned = dict.unary_mut::<_, Decimal128Type>(|x| x +
1).unwrap_err();
+ assert_eq!(returned, DictionaryArray::new(keys, Arc::new(values())));
+ }
+
#[test]
fn test_occupancy() {
let keys = Int32Array::new((100..200).collect(), None);
diff --git a/arrow-array/src/array/primitive_array.rs
b/arrow-array/src/array/primitive_array.rs
index ae1b727745..b88089f527 100644
--- a/arrow-array/src/array/primitive_array.rs
+++ b/arrow-array/src/array/primitive_array.rs
@@ -1190,7 +1190,7 @@ impl<T: ArrowPrimitiveType> PrimitiveArray<T> {
};
match try_mutable_buffers {
- Ok(builder) => Ok(builder),
+ Ok(builder) => Ok(builder.with_data_type(data_type)),
Err((buffer, null_bit_buffer)) => {
let builder = ArrayData::builder(data_type)
.len(len)
@@ -2937,14 +2937,22 @@ mod tests {
}
#[test]
- fn test_into_builder_err_keeps_data_type() {
+ fn test_into_builder_keeps_data_type() {
let array = Decimal128Array::from(vec![1, 2])
.with_precision_and_scale(10, 2)
.unwrap();
- let shared = array.clone();
+ let data_type = array.data_type().clone();
- let err_return = array.into_builder().unwrap_err();
- assert_eq!(err_return.data_type(), shared.data_type());
+ // Shared buffers: the array is returned
+ let shared = array.clone();
+ let array = array.into_builder().unwrap_err();
+ assert_eq!(array.data_type(), &data_type);
+ drop(shared);
+
+ // Unshared buffers: the buffers are reused by the builder
+ let mut builder = array.into_builder().unwrap();
+ builder.append_value(3);
+ assert_eq!(builder.finish().data_type(), &data_type);
}
#[test]
diff --git a/arrow-array/src/builder/primitive_run_builder.rs
b/arrow-array/src/builder/primitive_run_builder.rs
index 71fcf87475..a78e5844ed 100644
--- a/arrow-array/src/builder/primitive_run_builder.rs
+++ b/arrow-array/src/builder/primitive_run_builder.rs
@@ -366,4 +366,15 @@ mod tests {
let values = array.values();
assert_eq!(values.timezone(), Some("Europe/Paris"));
}
+
+ #[test]
+ fn test_finish_cloned_keeps_data_type() {
+ let data_type = DataType::Decimal128(5, 2);
+ let mut builder = PrimitiveRunBuilder::<Int16Type,
Decimal128Type>::new()
+ .with_data_type(data_type.clone());
+ // Leave the run open, so `finish_cloned` has to add it to a copy of
the values
+ builder.append_value(123);
+ let array = builder.finish_cloned();
+ assert_eq!(array.values().data_type(), &data_type);
+ }
}
