liamzwbao commented on code in PR #10377:
URL: https://github.com/apache/arrow-rs/pull/10377#discussion_r4059371009
##########
arrow-select/src/concat.rs:
##########
@@ -1889,6 +1896,27 @@ mod tests {
assert_eq!(&[10, 20, 30, 40], values.values());
}
+ #[test]
+ fn test_concat_run_array_length_overflows_run_end_type() {
+ // 20_000 + 20_000 exceeds i16::MAX.
+ let array1 = RunArray::<Int16Type>::try_new(
+ &Int16Array::from(vec![20_000]),
+ &Int16Array::from(vec![1]),
+ )
+ .unwrap();
+ let array2 = RunArray::<Int16Type>::try_new(
+ &Int16Array::from(vec![20_000]),
+ &Int16Array::from(vec![2]),
+ )
+ .unwrap();
+
+ let err = concat(&[&array1, &array2]).unwrap_err();
+ assert!(
+ err.to_string().contains("overflows the run-end type"),
+ "expected a run-end overflow error, got: {err}"
+ );
Review Comment:
I think `RunEndIndexOverflowError` makes sense but we need to fix the error
description a bit.
The old doc said “index bigger than the array length,” while the other
overflow errors (`DictionaryKeyOverflowError` or `OffsetOverflowError`) all
mean the value doesn’t fit in the integer type, which is what’s happening here,
so I feel like we should match it.
Plus, this error wasn't used anywhere before this PR, so it's safe to change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
