jonahgao commented on code in PR #10531:
URL: https://github.com/apache/datafusion/pull/10531#discussion_r1609180143
##########
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:
Thank you for your explanation @Blizzara . My concern is that using two
types for List simultaneously is a bit verbose unless it is necessary.
I tested the following two cases: the first one panicked, and the second one
also failed.
```rust
#[tokio::test]
async fn roundtrip_list_iteral() -> Result<()> {
roundtrip("SELECT [] from data").await
}
#[tokio::test]
async fn roundtrip_struct_iteral() -> Result<()> {
roundtrip("select struct(1 as name0, 3.14 as name1, 'e', true as name3)
from data")
.await
}
```
```sh
---- cases::roundtrip_logical_plan::roundtrip_list_iteral stdout ----
thread 'cases::roundtrip_logical_plan::roundtrip_list_iteral' panicked at
consumer.rs:1425:41:
index out of bounds: the len is 0 but the index is 0
```
I think lists and structs are more complex than the virtual table, so it's
better for them to have separate PRs.
--
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
