jonahgao commented on code in PR #10531:
URL: https://github.com/apache/datafusion/pull/10531#discussion_r1608408244
##########
datafusion/substrait/src/logical_plan/consumer.rs:
##########
@@ -1277,7 +1404,84 @@ pub(crate) fn from_substrait_literal(lit: &Literal) ->
Result<ScalarValue> {
s,
)
}
- Some(LiteralType::Null(ntype)) => from_substrait_null(ntype)?,
+ Some(LiteralType::List(l)) => {
+ let element_dt: Option<&DataType> = match dt {
+ Some(DataType::List(l)) => Ok(Some(l.data_type())),
Review Comment:
I am wondering if `dt` is redundant here and can be removed. We still need
to check whether it is a `LargeList`, just as we check
`type_variation_reference` below.
Is it possible to only extract `element_dt` from `elements[0]`? I guess
that `elements` are not empty because substrait has a special type called
EmptyList.
```rust
pub struct List {
/// A homogeneously-typed list of one or more expressions that
form the
/// list entries. To specify an empty list, use
Literal.empty_list
/// (otherwise type information would be missing).
#[prost(message, repeated, tag="1")]
pub values: ::prost::alloc::vec::Vec<super::super::Expression>,
}
```
--
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: github-unsubscr...@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org
