[GitHub] [arrow] andygrove commented on a change in pull request #7988: ARROW-9783: [Rust] [DataFusion] Remove aggregate expression data type

[GitBox <gi...@apache.org>]

andygrove commented on a change in pull request #7988:
URL: https://github.com/apache/arrow/pull/7988#discussion_r472217735



##########
File path: rust/datafusion/src/logicalplan.rs
##########
@@ -379,7 +349,48 @@ impl Expr {
             Expr::Literal(l) => l.get_datatype(),
             Expr::Cast { data_type, .. } => Ok(data_type.clone()),
             Expr::ScalarFunction { return_type, .. } => Ok(return_type.clone()),
-            Expr::AggregateFunction { return_type, .. } => Ok(return_type.clone()),
+            Expr::AggregateFunction { name, args, .. } => {
+                match name.to_uppercase().as_str() {
+                    "MIN" | "MAX" => args[0].get_type(schema),
+                    "SUM" => match args[0].get_type(schema)? {
+                        DataType::Int8
+                        | DataType::Int16
+                        | DataType::Int32
+                        | DataType::Int64 => Ok(DataType::Int64),
+                        DataType::UInt8
+                        | DataType::UInt16
+                        | DataType::UInt32
+                        | DataType::UInt64 => Ok(DataType::UInt64),
+                        DataType::Float32 => Ok(DataType::Float32),

Review comment:
       I don't know. I just copied over the logic from the physical implementation of these expressions for consistency. Maybe the next step here is for me to create a PR to unify the code somehow between the logical and physical expressions.




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org