alamb commented on code in PR #10874:
URL: https://github.com/apache/datafusion/pull/10874#discussion_r1635332004
##########
datafusion/substrait/src/logical_plan/consumer.rs:
##########
@@ -1272,7 +1272,9 @@ fn from_substrait_type(
})?;
let field = Arc::new(Field::new_list_field(
from_substrait_type(inner_type, dfs_names, name_idx)?,
- is_substrait_type_nullable(inner_type)?,
+ // We ignore Substrait's nullability here to match
to_substrait_literal
Review Comment:
👍
##########
datafusion/substrait/src/logical_plan/producer.rs:
##########
@@ -2309,14 +2309,12 @@ mod test {
round_trip_type(DataType::Decimal128(10, 2))?;
round_trip_type(DataType::Decimal256(30, 2))?;
- for nullable in [true, false] {
- round_trip_type(DataType::List(
- Field::new_list_field(DataType::Int32, nullable).into(),
- ))?;
- round_trip_type(DataType::LargeList(
- Field::new_list_field(DataType::Int32, nullable).into(),
- ))?;
- }
+ round_trip_type(DataType::List(
Review Comment:
Should we also add a test here showing that the List becomes nullable after
roundtrip? That might additionally document that this is an intended behavior
rather than a bug.
I do see you have added a comment which I think is probably enought
##########
datafusion/common/src/utils/mod.rs:
##########
@@ -366,7 +366,7 @@ pub fn array_into_list_array(arr: ArrayRef) -> ListArray {
pub fn array_into_large_list_array(arr: ArrayRef) -> LargeListArray {
let offsets = OffsetBuffer::from_lengths([arr.len()]);
LargeListArray::new(
- Arc::new(Field::new("item", arr.data_type().to_owned(), true)),
+ Arc::new(Field::new_list_field(arr.data_type().to_owned(), true)),
Review Comment:
👍
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
