Blizzara commented on code in PR #10615:
URL: https://github.com/apache/datafusion/pull/10615#discussion_r1610205145


##########
datafusion/substrait/src/logical_plan/consumer.rs:
##########
@@ -1278,6 +1279,45 @@ pub(crate) fn from_substrait_literal(lit: &Literal) -> 
Result<ScalarValue> {
                 s,
             )
         }
+        Some(LiteralType::List(l)) => {
+            let elements = l
+                .values
+                .iter()
+                .map(from_substrait_literal)
+                .collect::<Result<Vec<_>>>()?;
+            if elements.is_empty() {
+                return substrait_err!(
+                    "Empty list must be encoded as EmptyList literal type, not 
List"
+                );
+            }
+            let element_type = elements[0].data_type();
+            match lit.type_variation_reference {
+                DEFAULT_CONTAINER_TYPE_REF => 
ScalarValue::List(ScalarValue::new_list(
+                    elements.as_slice(),
+                    &element_type,
+                )),
+                LARGE_CONTAINER_TYPE_REF => ScalarValue::LargeList(
+                    ScalarValue::new_large_list(elements.as_slice(), 
&element_type),
+                ),
+                others => {
+                    return substrait_err!("Unknown type variation reference 
{others}");
+                }
+            }
+        }
+        Some(LiteralType::EmptyList(l)) => {
+            let element_type = 
from_substrait_type(l.r#type.clone().unwrap().as_ref())?;

Review Comment:
   Hm, without the type specified we don't know what it should be - I guess we 
could default to NullType (which I think is what DataFusion does if you just do 
a "SELECT [] FROM ..", do you think that'd make sense? 
   
   I feel like Substrait probably intends this field to always exist, though 
I'm not sure, but e.g. in the Java library they have it as required: 
https://github.com/substrait-io/substrait-java/blob/79decd20e85d6a1a5623890042ebcf1415cf784a/core/src/main/java/io/substrait/expression/Expression.java#L451



-- 
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]

Reply via email to