This is an automated email from the ASF dual-hosted git repository.
alamb 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 38778f0110 Replace `From<Vec<_>>` impls with `TryFrom`s for
`FixedSizeBinaryArray` (#10019)
38778f0110 is described below
commit 38778f0110725eb36f38f9fa2d43d2060b8cf928
Author: quantumish <[email protected]>
AuthorDate: Mon Jun 1 15:32:35 2026 +0100
Replace `From<Vec<_>>` impls with `TryFrom`s for `FixedSizeBinaryArray`
(#10019)
# Which issue does this PR close?
- Closes #10018.
# Rationale for this change
There isn't a clear way to fix the `From<Vec<_>>` implementations for
`FixedSizeBinaryArray` that wouldn't be confusing, so making them
`TryFrom` is a better fit since they are in genuine use across e.g.
tests within the Arrow library as well as a terser way of calling
`FixedSizeBinaryArray::try_from_iter` or
`FixedSizeBinaryArray::try_from_sparse_iter`.
# What changes are included in this PR?
- Converts `From<Vec<&[u8]>>`, `From<Vec<&[u8; N]>>`, and
`From<Vec<Option<&[u8]>>>` implementations for `FixedSizeBinaryArray` to
`TryFrom` implementations.
- Adds a `TryFrom<Vec<Option<&[u8; N]>>>` implementation for the missing
combination of types.
- Updates various test cases within the arrow/parquet libraries to use
`try_from().unwrap()` instead of `from()`.
# Are these changes tested?
This is sort of a transparent change in that only the API for expressing
failure cases has changed rather than the actual failure cases. All
existing tests surrounding conversion failures have been updated to
check whether a conversion has correctly failed.
# Are there any user-facing changes?
Yes, this is a breaking API change since user-facing trait
implementations have been replaced with different trait implementations.
---
arrow-array/src/array/fixed_size_binary_array.rs | 50 ++++++++++++++++--------
arrow-cast/src/cast/mod.rs | 4 +-
arrow-ord/src/comparison.rs | 6 ++-
arrow-select/src/filter.rs | 2 +-
arrow-string/src/concat_elements.rs | 28 +++++++------
arrow-string/src/substring.rs | 19 ++++++++-
parquet/tests/arrow_reader/mod.rs | 3 +-
7 files changed, 77 insertions(+), 35 deletions(-)
diff --git a/arrow-array/src/array/fixed_size_binary_array.rs
b/arrow-array/src/array/fixed_size_binary_array.rs
index a44d21ad5b..67af863228 100644
--- a/arrow-array/src/array/fixed_size_binary_array.rs
+++ b/arrow-array/src/array/fixed_size_binary_array.rs
@@ -679,22 +679,42 @@ impl From<FixedSizeListArray> for FixedSizeBinaryArray {
}
}
-impl From<Vec<Option<&[u8]>>> for FixedSizeBinaryArray {
- fn from(v: Vec<Option<&[u8]>>) -> Self {
+impl TryFrom<Vec<Option<&[u8]>>> for FixedSizeBinaryArray {
+ type Error = ArrowError;
+
+ fn try_from(v: Vec<Option<&[u8]>>) -> Result<Self, Self::Error> {
#[allow(deprecated)]
- Self::try_from_sparse_iter(v.into_iter()).unwrap()
+ Self::try_from_sparse_iter(v.into_iter())
+ }
+}
+
+impl TryFrom<Vec<&[u8]>> for FixedSizeBinaryArray {
+ type Error = ArrowError;
+
+ fn try_from(v: Vec<&[u8]>) -> Result<Self, Self::Error> {
+ Self::try_from_iter(v.into_iter())
}
}
-impl From<Vec<&[u8]>> for FixedSizeBinaryArray {
- fn from(v: Vec<&[u8]>) -> Self {
- Self::try_from_iter(v.into_iter()).unwrap()
+impl<const N: usize> TryFrom<Vec<Option<&[u8; N]>>> for FixedSizeBinaryArray {
+ type Error = ArrowError;
+
+ fn try_from(v: Vec<Option<&[u8; N]>>) -> Result<Self, Self::Error> {
+ N.try_into()
+ .map_err(|_| {
+ ArrowError::InvalidArgumentError(format!(
+ "FixedSizeBinaryArray value length exceeds i32, got {N}"
+ ))
+ })
+ .and_then(|x| Self::try_from_sparse_iter_with_size(v.into_iter(),
x))
}
}
-impl<const N: usize> From<Vec<&[u8; N]>> for FixedSizeBinaryArray {
- fn from(v: Vec<&[u8; N]>) -> Self {
- Self::try_from_iter(v.into_iter()).unwrap()
+impl<const N: usize> TryFrom<Vec<&[u8; N]>> for FixedSizeBinaryArray {
+ type Error = ArrowError;
+
+ fn try_from(v: Vec<&[u8; N]>) -> Result<Self, Self::Error> {
+ Self::try_from_iter(v.into_iter())
}
}
@@ -1009,7 +1029,7 @@ mod tests {
#[test]
fn test_fixed_size_binary_array_from_vec() {
let values = vec!["one".as_bytes(), b"two", b"six", b"ten"];
- let array = FixedSizeBinaryArray::from(values);
+ let array = FixedSizeBinaryArray::try_from(values).unwrap();
assert_eq!(array.len(), 4);
assert_eq!(array.null_count(), 0);
assert_eq!(array.logical_null_count(), 0);
@@ -1024,10 +1044,9 @@ mod tests {
}
#[test]
- #[should_panic(expected = "Nested array size mismatch: one is 3, and the
other is 5")]
fn test_fixed_size_binary_array_from_vec_incorrect_length() {
let values = vec!["one".as_bytes(), b"two", b"three", b"four"];
- let _ = FixedSizeBinaryArray::from(values);
+ assert!(FixedSizeBinaryArray::try_from(values).is_err());
}
#[test]
@@ -1039,7 +1058,7 @@ mod tests {
Some(b"six"),
Some(b"ten"),
];
- let array = FixedSizeBinaryArray::from(values);
+ let array = FixedSizeBinaryArray::try_from(values).unwrap();
assert_eq!(array.len(), 5);
assert_eq!(array.value(0), b"one");
assert_eq!(array.value(1), b"two");
@@ -1053,7 +1072,6 @@ mod tests {
}
#[test]
- #[should_panic(expected = "Nested array size mismatch: one is 3, and the
other is 5")]
fn test_fixed_size_binary_array_from_opt_vec_incorrect_length() {
let values = vec![
Some("one".as_bytes()),
@@ -1062,7 +1080,7 @@ mod tests {
Some(b"three"),
Some(b"four"),
];
- let _ = FixedSizeBinaryArray::from(values);
+ assert!(FixedSizeBinaryArray::try_from(values).is_err());
}
#[test]
@@ -1098,7 +1116,7 @@ mod tests {
)]
fn test_fixed_size_binary_array_get_value_index_out_of_bound() {
let values = vec![Some("one".as_bytes()), Some(b"two"), None];
- let array = FixedSizeBinaryArray::from(values);
+ let array = FixedSizeBinaryArray::try_from(values).unwrap();
array.value(4);
}
diff --git a/arrow-cast/src/cast/mod.rs b/arrow-cast/src/cast/mod.rs
index 67da85b8c1..0367a54121 100644
--- a/arrow-cast/src/cast/mod.rs
+++ b/arrow-cast/src/cast/mod.rs
@@ -5908,7 +5908,7 @@ mod tests {
let bytes_2 = "Hello".as_bytes();
let binary_data = vec![Some(bytes_1), Some(bytes_2), None];
- let a1 = Arc::new(FixedSizeBinaryArray::from(binary_data.clone())) as
ArrayRef;
+ let a1 =
Arc::new(FixedSizeBinaryArray::try_from(binary_data.clone()).unwrap()) as
ArrayRef;
let array_ref = cast(&a1, &DataType::Binary).unwrap();
let down_cast = array_ref.as_binary::<i32>();
@@ -5935,7 +5935,7 @@ mod tests {
let bytes_2 = "Hello".as_bytes();
let binary_data = vec![Some(bytes_1), Some(bytes_2), Some(bytes_1),
None];
- let a1 = Arc::new(FixedSizeBinaryArray::from(binary_data.clone())) as
ArrayRef;
+ let a1 =
Arc::new(FixedSizeBinaryArray::try_from(binary_data.clone()).unwrap()) as
ArrayRef;
let cast_type = DataType::Dictionary(
Box::new(DataType::Int8),
diff --git a/arrow-ord/src/comparison.rs b/arrow-ord/src/comparison.rs
index 3aff2c6234..4d61eb8758 100644
--- a/arrow-ord/src/comparison.rs
+++ b/arrow-ord/src/comparison.rs
@@ -1801,7 +1801,8 @@ mod tests {
expected
);
- let fsb_array = FixedSizeBinaryArray::from(vec![&[0u8], &[0u8],
&[0u8], &[1u8]]);
+ let fsb_array =
+ FixedSizeBinaryArray::try_from(vec![&[0u8], &[0u8], &[0u8],
&[1u8]]).unwrap();
let scalar = FixedSizeBinaryArray::new_scalar([1u8]);
let expected = BooleanArray::from(vec![Some(false), Some(false),
Some(false), Some(true)]);
assert_eq!(crate::cmp::eq(&fsb_array, &scalar).unwrap(), expected);
@@ -1824,7 +1825,8 @@ mod tests {
expected
);
- let fsb_array = FixedSizeBinaryArray::from(vec![&[0u8], &[0u8],
&[0u8], &[1u8]]);
+ let fsb_array =
+ FixedSizeBinaryArray::try_from(vec![&[0u8], &[0u8], &[0u8],
&[1u8]]).unwrap();
let scalar = FixedSizeBinaryArray::new_scalar([1u8]);
let expected = BooleanArray::from(vec![Some(true), Some(true),
Some(true), Some(false)]);
assert_eq!(crate::cmp::neq(&fsb_array, &scalar).unwrap(), expected);
diff --git a/arrow-select/src/filter.rs b/arrow-select/src/filter.rs
index aaab9d2020..fcbce82d5d 100644
--- a/arrow-select/src/filter.rs
+++ b/arrow-select/src/filter.rs
@@ -1273,7 +1273,7 @@ mod tests {
let v2 = [3_u8, 4];
let v3 = [5_u8, 6];
let v = vec![&v1, &v2, &v3];
- let a = FixedSizeBinaryArray::from(v);
+ let a = FixedSizeBinaryArray::try_from(v).unwrap();
let b = BooleanArray::from(vec![true, false, true]);
let c = filter(&a, &b).unwrap();
let d = c
diff --git a/arrow-string/src/concat_elements.rs
b/arrow-string/src/concat_elements.rs
index cd4676d287..8742b17392 100644
--- a/arrow-string/src/concat_elements.rs
+++ b/arrow-string/src/concat_elements.rs
@@ -505,30 +505,33 @@ mod tests {
#[test]
fn test_fixed_size_binary_concat() {
- let left = FixedSizeBinaryArray::from(vec![Some(b"foo" as &[u8]),
Some(b"bar"), None]);
- let right = FixedSizeBinaryArray::from(vec![None, Some(b"yyy" as
&[u8]), Some(b"zzz")]);
+ let left = FixedSizeBinaryArray::try_from(vec![Some(b"foo" as &[u8]),
Some(b"bar"), None])
+ .unwrap();
+ let right = FixedSizeBinaryArray::try_from(vec![None, Some(b"yyy" as
&[u8]), Some(b"zzz")])
+ .unwrap();
let output = concat_elements_fixed_size_binary(&left, &right).unwrap();
- let expected = FixedSizeBinaryArray::from(vec![None, Some(b"baryyy" as
&[u8]), None]);
+ let expected =
+ FixedSizeBinaryArray::try_from(vec![None, Some(b"baryyy" as
&[u8]), None]).unwrap();
assert_eq!(output, expected);
}
#[test]
fn test_fixed_size_binary_concat_no_null() {
- let left = FixedSizeBinaryArray::from(vec![b"ab" as &[u8], b"cd"]);
- let right = FixedSizeBinaryArray::from(vec![b"12" as &[u8], b"34"]);
+ let left = FixedSizeBinaryArray::try_from(vec![b"ab" as &[u8],
b"cd"]).unwrap();
+ let right = FixedSizeBinaryArray::try_from(vec![b"12" as &[u8],
b"34"]).unwrap();
let output = concat_elements_fixed_size_binary(&left, &right).unwrap();
- let expected = FixedSizeBinaryArray::from(vec![b"ab12" as &[u8],
b"cd34"]);
+ let expected = FixedSizeBinaryArray::try_from(vec![b"ab12" as &[u8],
b"cd34"]).unwrap();
assert_eq!(output, expected);
}
#[test]
fn test_fixed_size_binary_concat_error() {
- let left = FixedSizeBinaryArray::from(vec![b"ab" as &[u8], b"cd"]);
- let right = FixedSizeBinaryArray::from(vec![b"12" as &[u8]]);
+ let left = FixedSizeBinaryArray::try_from(vec![b"ab" as &[u8],
b"cd"]).unwrap();
+ let right = FixedSizeBinaryArray::try_from(vec![b"12" as
&[u8]]).unwrap();
let output = concat_elements_fixed_size_binary(&left, &right);
assert_eq!(
@@ -673,13 +676,16 @@ mod tests {
assert_eq!(output, expected);
// test for FixedSizeBinaryArray
- let left = FixedSizeBinaryArray::from(vec![Some(b"foo" as &[u8]),
Some(b"bar"), None]);
- let right = FixedSizeBinaryArray::from(vec![None, Some(b"yyy" as
&[u8]), Some(b"zzz")]);
+ let left = FixedSizeBinaryArray::try_from(vec![Some(b"foo" as &[u8]),
Some(b"bar"), None])
+ .unwrap();
+ let right = FixedSizeBinaryArray::try_from(vec![None, Some(b"yyy" as
&[u8]), Some(b"zzz")])
+ .unwrap();
let output: FixedSizeBinaryArray = concat_elements_dyn(&left, &right)
.unwrap()
.into_data()
.into();
- let expected = FixedSizeBinaryArray::from(vec![None, Some(b"baryyy" as
&[u8]), None]);
+ let expected =
+ FixedSizeBinaryArray::try_from(vec![None, Some(b"baryyy" as
&[u8]), None]).unwrap();
assert_eq!(output, expected);
}
diff --git a/arrow-string/src/substring.rs b/arrow-string/src/substring.rs
index 123c77cdf9..563c664480 100644
--- a/arrow-string/src/substring.rs
+++ b/arrow-string/src/substring.rs
@@ -429,6 +429,21 @@ mod tests {
};
}
+ /// A helper macro to test the substring functions for array types only
implementing TryFrom.
+ macro_rules! do_test_tryfrom {
+ ($cases:expr, $array_ty:ty, $substring_fn:ident) => {
+ $cases
+ .into_iter()
+ .for_each(|(array, start, length, expected)| {
+ let array = <$array_ty>::try_from(array).unwrap();
+ let result = $substring_fn(&array, start, length).unwrap();
+ let result =
result.as_any().downcast_ref::<$array_ty>().unwrap();
+ let expected = <$array_ty>::try_from(expected).unwrap();
+ assert_eq!(&expected, result);
+ })
+ };
+ }
+
fn with_nulls_generic_binary<O: OffsetSizeTrait>() {
let input = vec![
Some("hello".as_bytes()),
@@ -591,7 +606,7 @@ mod tests {
(-3, Some(4), input.clone())
);
- do_test!(
+ do_test_tryfrom!(
[&base_case[..], &cases[..]].concat(),
FixedSizeBinaryArray,
substring
@@ -630,7 +645,7 @@ mod tests {
(-3, Some(4), input.clone())
);
- do_test!(
+ do_test_tryfrom!(
[&base_case[..], &cases[..]].concat(),
FixedSizeBinaryArray,
substring
diff --git a/parquet/tests/arrow_reader/mod.rs
b/parquet/tests/arrow_reader/mod.rs
index d9729fdc77..404bebc05d 100644
--- a/parquet/tests/arrow_reader/mod.rs
+++ b/parquet/tests/arrow_reader/mod.rs
@@ -559,7 +559,8 @@ fn make_bytearray_batch(
.iter()
.map(|value| Some(value.as_slice()))
.collect::<Vec<_>>()
- .into();
+ .try_into()
+ .unwrap();
let service_large_binary: LargeBinaryArray =
large_binary_values.iter().map(Some).collect();
let schema = Schema::new(vec