mesejo commented on code in PR #12308:
URL: https://github.com/apache/datafusion/pull/12308#discussion_r1743548464


##########
datafusion/functions/src/core/coalesce.rs:
##########
@@ -60,12 +60,16 @@ impl ScalarUDFImpl for CoalesceFunc {
     }
 
     fn return_type(&self, arg_types: &[DataType]) -> Result<DataType> {
-        Ok(arg_types[0].clone())
+        Ok(arg_types
+            .iter()
+            .find_or_first(|d| !d.is_null())
+            .unwrap()
+            .clone())
     }
 
-    // If all the element in coalesce is non-null, the result is non-null
+    // If all the elements in coalesce are non-null, the result is non-null
     fn is_nullable(&self, args: &[Expr], schema: &dyn ExprSchema) -> bool {
-        args.iter().any(|e| e.nullable(schema).ok().unwrap_or(true))
+        args.iter().all(|e| e.nullable(schema).ok().unwrap_or(true))

Review Comment:
   Sorry, I was asking more about which function to use to coerce the types.
   I see that in the `coerce_types` function, there is the following code:
   ```rust
   if arg_types.is_empty() {
               return exec_err!("coalesce must have at least one argument");
           }
           let new_type = type_union_resolution(arg_types)
               .unwrap_or(arg_types.first().unwrap().clone());
   ``` 
   
   Should I follow the same pattern?



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