sunchao commented on a change in pull request #9425:
URL: https://github.com/apache/arrow/pull/9425#discussion_r571463897
##########
File path: rust/arrow/src/array/array_list.rs
##########
@@ -139,7 +166,19 @@ impl<OffsetSize: OffsetSizeTrait> From<ArrayDataRef> for
GenericListArray<Offset
1,
"ListArray should contain a single child array (values array)"
);
- let values = make_array(data.child_data()[0].clone());
+
+ let values = data.child_data()[0].clone();
+
+ if let Some(child) = Self::get_type(data.data_type()) {
+ assert_eq!(values.data_type(), child, "[Large]ListArray's child
datatype does not correspond to the List's datatype");
Review comment:
nit: long line? I remember we enforce a limit of 100 characters.
##########
File path: rust/arrow/src/array/array_list.rs
##########
@@ -31,12 +31,19 @@ use crate::datatypes::*;
/// trait declaring an offset size, relevant for i32 vs i64 array types.
pub trait OffsetSizeTrait: ArrowNativeType + Num + Ord + std::ops::AddAssign {
+ fn is_large() -> bool;
+
fn prefix() -> &'static str;
fn to_isize(&self) -> isize;
}
impl OffsetSizeTrait for i32 {
+ #[inline]
+ fn is_large() -> bool {
+ false
+ }
+
fn prefix() -> &'static str {
Review comment:
maybe we won't need `prefix` anymore with the new `is_large`.
##########
File path: rust/arrow/src/array/array_list.rs
##########
@@ -139,7 +166,19 @@ impl<OffsetSize: OffsetSizeTrait> From<ArrayDataRef> for
GenericListArray<Offset
1,
"ListArray should contain a single child array (values array)"
);
- let values = make_array(data.child_data()[0].clone());
+
+ let values = data.child_data()[0].clone();
+
+ if let Some(child) = Self::get_type(data.data_type()) {
Review comment:
can we have a few tests to cover this? checking error message and all.
Also I'm not sure if `assert_eq` is good here: IMO assertion should only be
used for checking internal logic that developer should follow and which are not
exposed to the library users, but in this case it appears not. It's just a nit
though since this is already used in multiple places before.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
