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:
[email protected]
