vbarua commented on code in PR #15854:
URL: https://github.com/apache/datafusion/pull/15854#discussion_r2069115514


##########
datafusion/substrait/src/logical_plan/producer.rs:
##########
@@ -1590,6 +1590,21 @@ pub fn from_cast(
     schema: &DFSchemaRef,
 ) -> Result<Expression> {
     let Cast { expr, data_type } = cast;
+    // since substrait Null must be typed, so if we see a cast(null, dt), we 
make it a typed null
+    if let Expr::Literal(lit) = expr.as_ref() {
+        if lit.is_null() {

Review Comment:
   I don't really grok the `is_null` logic for compound types super well.
   
   I did test locally with
   ```rust
           let sv = ScalarValue::List(Arc::new(GenericListArray::new_null(
               Field::new_list_field(DataType::Float32, true).into(),
               1,
           )));
           let target_type = DataType::List(
               Field::new_list_field(DataType::Int32, true).into()
           );
           let expr = Expr::Literal(sv)
               .cast_to(&target_type, &empty_schema)
               .unwrap();
   ```
   and was able to trigger the folding behaviour, which is undesirable... I 
think?
   
   @discord9 it might be good to capture that we _shouldn't_ fold this as a 
test.



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

Reply via email to