Skip to content

Commit 844a573

Browse files
committed
add try_new_with_length constructor to FixedSizeList
Also fixes existing tests and adds some more test cases for degenerate `FixedSizeList`s. Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
1 parent 751b082 commit 844a573

File tree

1 file changed

+127
-24
lines changed

1 file changed

+127
-24
lines changed

arrow-array/src/array/fixed_size_list_array.rs

Lines changed: 127 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,15 @@ pub struct FixedSizeListArray {
125125
}
126126

127127
impl FixedSizeListArray {
128-
/// Create a new [`FixedSizeListArray`] with `size` element size, panicking on failure
128+
/// Create a new [`FixedSizeListArray`] with `size` element size, panicking on failure.
129+
///
130+
/// Note that if `size == 0` and `nulls` is `None` (a degenerate, non-nullable
131+
/// `FixedSizeListArray`), this function will set the length of the array to 0.
132+
///
133+
/// If you would like to have a degenerate, non-nullable `FixedSizeListArray` with arbitrary
134+
/// length, use the [`try_new_with_length()`] constructor.
135+
///
136+
/// [`try_new_with_length()`]: Self::try_new_with_length
129137
///
130138
/// # Panics
131139
///
@@ -134,12 +142,20 @@ impl FixedSizeListArray {
134142
Self::try_new(field, size, values, nulls).unwrap()
135143
}
136144

137-
/// Create a new [`FixedSizeListArray`] from the provided parts, returning an error on failure
145+
/// Create a new [`FixedSizeListArray`] from the provided parts, returning an error on failure.
146+
///
147+
/// Note that if `size == 0` and `nulls` is `None` (a degenerate, non-nullable
148+
/// `FixedSizeListArray`), this function will set the length of the array to 0.
149+
///
150+
/// If you would like to have a degenerate, non-nullable `FixedSizeListArray` with arbitrary
151+
/// length, use the [`try_new_with_length()`] constructor.
152+
///
153+
/// [`try_new_with_length()`]: Self::try_new_with_length
138154
///
139155
/// # Errors
140156
///
141157
/// * `size < 0`
142-
/// * `values.len() / size != nulls.len()`
158+
/// * `values.len() != nulls.len() * size` if `nulls` is `Some`
143159
/// * `values.data_type() != field.data_type()`
144160
/// * `!field.is_nullable() && !nulls.expand(size).contains(values.logical_nulls())`
145161
pub fn try_new(
@@ -152,23 +168,81 @@ impl FixedSizeListArray {
152168
ArrowError::InvalidArgumentError(format!("Size cannot be negative, got {size}"))
153169
})?;
154170

155-
let len = match s {
156-
0 => nulls.as_ref().map(|x| x.len()).unwrap_or_default(),
157-
_ => {
158-
let len = values.len() / s.max(1);
159-
if let Some(n) = nulls.as_ref() {
160-
if n.len() != len {
161-
return Err(ArrowError::InvalidArgumentError(format!(
162-
"Incorrect length of null buffer for FixedSizeListArray, expected {} got {}",
163-
len,
164-
n.len(),
165-
)));
166-
}
171+
let len = if s == 0 {
172+
// Note that for degenerate and non-nullable `FixedSizeList`s, we will set the length to
173+
// 0 (`_or_default`).
174+
nulls.as_ref().map(|x| x.len()).unwrap_or_default()
175+
} else {
176+
// Note that `s` might not divide `values.len()`, so we need to check this
177+
let len = values.len() / s;
178+
179+
// Check that the null buffer length is correct (if it exists).
180+
if let Some(null_buffer) = &nulls {
181+
if s * null_buffer.len() != values.len() {
182+
return Err(ArrowError::InvalidArgumentError(format!(
183+
"Incorrect length of values buffer for FixedSizeListArray, expected {} got {}",
184+
s * null_buffer.len(),
185+
values.len(),
186+
)));
167187
}
168-
len
169188
}
189+
190+
len
170191
};
171192

193+
Self::try_new_with_length(field, size, values, nulls, len)
194+
}
195+
196+
/// Create a new [`FixedSizeListArray`] from the provided parts, returning an error on failure.
197+
///
198+
/// This method exists to allow the construction of arbitrary length degenerate (`size == 0`)
199+
/// and non-nullable `FixedSizeListArray`s. If you want a nullable `FixedSizeListArray`, then
200+
/// you can use [`try_new()`] instead.
201+
///
202+
/// [`try_new()`]: Self::try_new
203+
///
204+
/// # Errors
205+
///
206+
/// * `size < 0`
207+
/// * `nulls.len() != len` if `nulls` is `Some`
208+
/// * `values.len() != len * size`
209+
/// * `values.data_type() != field.data_type()`
210+
/// * `!field.is_nullable() && !nulls.expand(size).contains(values.logical_nulls())`
211+
pub fn try_new_with_length(
212+
field: FieldRef,
213+
size: i32,
214+
values: ArrayRef,
215+
nulls: Option<NullBuffer>,
216+
len: usize,
217+
) -> Result<Self, ArrowError> {
218+
let s = size.to_usize().ok_or_else(|| {
219+
ArrowError::InvalidArgumentError(format!("Size cannot be negative, got {size}"))
220+
})?;
221+
222+
if let Some(null_buffer) = &nulls
223+
&& null_buffer.len() != len
224+
{
225+
return Err(ArrowError::InvalidArgumentError(format!(
226+
"Invalid null buffer for FixedSizeListArray, expected {len} found {}",
227+
null_buffer.len()
228+
)));
229+
}
230+
231+
if s == 0 && !values.is_empty() {
232+
return Err(ArrowError::InvalidArgumentError(format!(
233+
"An degenerate FixedSizeListArray should have no underlying values, found {} values",
234+
values.len()
235+
)));
236+
}
237+
238+
if values.len() != len * s {
239+
return Err(ArrowError::InvalidArgumentError(format!(
240+
"Incorrect length of values buffer for FixedSizeListArray, expected {} got {}",
241+
len * s,
242+
values.len(),
243+
)));
244+
}
245+
172246
if field.data_type() != values.data_type() {
173247
return Err(ArrowError::InvalidArgumentError(format!(
174248
"FixedSizeListArray expected data type {} got {} for {:?}",
@@ -673,23 +747,28 @@ mod tests {
673747
let list = FixedSizeListArray::new(field.clone(), 2, values.clone(), Some(nulls));
674748
assert_eq!(list.len(), 3);
675749

676-
let list = FixedSizeListArray::new(field.clone(), 4, values.clone(), None);
677-
assert_eq!(list.len(), 1);
750+
let list = FixedSizeListArray::new(field.clone(), 3, values.clone(), None);
751+
assert_eq!(list.len(), 2);
752+
753+
let err =
754+
FixedSizeListArray::try_new_with_length(field.clone(), 1, values.clone(), None, 4)
755+
.unwrap_err();
756+
assert_eq!(
757+
err.to_string(),
758+
"Invalid argument error: Incorrect length of values buffer for FixedSizeListArray, expected 4 got 6"
759+
);
678760

679761
let err = FixedSizeListArray::try_new(field.clone(), -1, values.clone(), None).unwrap_err();
680762
assert_eq!(
681763
err.to_string(),
682764
"Invalid argument error: Size cannot be negative, got -1"
683765
);
684766

685-
let list = FixedSizeListArray::new(field.clone(), 0, values.clone(), None);
686-
assert_eq!(list.len(), 0);
687-
688767
let nulls = NullBuffer::new_null(2);
689768
let err = FixedSizeListArray::try_new(field, 2, values.clone(), Some(nulls)).unwrap_err();
690769
assert_eq!(
691770
err.to_string(),
692-
"Invalid argument error: Incorrect length of null buffer for FixedSizeListArray, expected 3 got 2"
771+
"Invalid argument error: Incorrect length of values buffer for FixedSizeListArray, expected 4 got 6"
693772
);
694773

695774
let field = Arc::new(Field::new_list_field(DataType::Int32, false));
@@ -712,11 +791,35 @@ mod tests {
712791
}
713792

714793
#[test]
715-
fn empty_fixed_size_list() {
794+
fn degenerate_fixed_size_list() {
716795
let field = Arc::new(Field::new_list_field(DataType::Int32, true));
717796
let nulls = NullBuffer::new_null(2);
718797
let values = new_empty_array(&DataType::Int32);
719-
let list = FixedSizeListArray::new(field.clone(), 0, values, Some(nulls));
798+
let list = FixedSizeListArray::new(field.clone(), 0, values.clone(), Some(nulls.clone()));
720799
assert_eq!(list.len(), 2);
800+
801+
// Test invalid null buffer length.
802+
let err = FixedSizeListArray::try_new_with_length(
803+
field.clone(),
804+
0,
805+
values.clone(),
806+
Some(nulls),
807+
5,
808+
)
809+
.unwrap_err();
810+
assert_eq!(
811+
err.to_string(),
812+
"Invalid argument error: Invalid null buffer for FixedSizeListArray, expected 5 found 2"
813+
);
814+
815+
// Test non-empty values for degenerate list.
816+
let non_empty_values = Arc::new(Int32Array::from(vec![1, 2, 3]));
817+
let err =
818+
FixedSizeListArray::try_new_with_length(field.clone(), 0, non_empty_values, None, 3)
819+
.unwrap_err();
820+
assert_eq!(
821+
err.to_string(),
822+
"Invalid argument error: An degenerate FixedSizeListArray should have no underlying values, found 3 values"
823+
);
721824
}
722825
}

0 commit comments

Comments
 (0)