alamb commented on code in PR #7369:
URL: https://github.com/apache/arrow-datafusion/pull/7369#discussion_r1301653120
##########
datafusion/optimizer/tests/optimizer_integration.rs:
##########
@@ -70,7 +70,7 @@ fn subquery_filter_with_cast() -> Result<()> {
\n Inner Join: Filter: CAST(test.col_int32 AS Float64) >
__scalar_sq_1.AVG(test.col_int32)\
\n TableScan: test projection=[col_int32]\
\n SubqueryAlias: __scalar_sq_1\
- \n Aggregate: groupBy=[[]], aggr=[[AVG(test.col_int32)]]\
+ \n Aggregate: groupBy=[[]], aggr=[[AVG(CAST(test.col_int32 AS
Float64))]]\
Review Comment:
it is actually nice to see the explicit cast I think
##########
datafusion/expr/src/type_coercion/aggregates.rs:
##########
@@ -357,11 +377,9 @@ fn get_min_max_result_type(input_types: &[DataType]) ->
Result<Vec<DataType>> {
/// function return type of a sum
pub fn sum_return_type(arg_type: &DataType) -> Result<DataType> {
match arg_type {
- arg_type if SIGNED_INTEGERS.contains(arg_type) => Ok(DataType::Int64),
- arg_type if UNSIGNED_INTEGERS.contains(arg_type) =>
Ok(DataType::UInt64),
- // In the
https://www.postgresql.org/docs/current/functions-aggregate.html doc,
- // the result type of floating-point is FLOAT64 with the double
precision.
- DataType::Float64 | DataType::Float32 => Ok(DataType::Float64),
+ DataType::Int64 => Ok(DataType::Int64),
+ DataType::UInt64 => Ok(DataType::UInt64),
+ DataType::Float64 => Ok(DataType::Float64),
Review Comment:
It looked like we lost support for Float32 -- but then I see the test below
for it, so 👍 (presumably what happens is the arguments are coerced first and
`arg_type` refers to the *post* coerced type)
##########
datafusion/expr/src/type_coercion/aggregates.rs:
##########
@@ -357,11 +377,9 @@ fn get_min_max_result_type(input_types: &[DataType]) ->
Result<Vec<DataType>> {
/// function return type of a sum
pub fn sum_return_type(arg_type: &DataType) -> Result<DataType> {
match arg_type {
- arg_type if SIGNED_INTEGERS.contains(arg_type) => Ok(DataType::Int64),
- arg_type if UNSIGNED_INTEGERS.contains(arg_type) =>
Ok(DataType::UInt64),
- // In the
https://www.postgresql.org/docs/current/functions-aggregate.html doc,
- // the result type of floating-point is FLOAT64 with the double
precision.
- DataType::Float64 | DataType::Float32 => Ok(DataType::Float64),
+ DataType::Int64 => Ok(DataType::Int64),
+ DataType::UInt64 => Ok(DataType::UInt64),
+ DataType::Float64 => Ok(DataType::Float64),
Review Comment:
It looked like we lost support for Float32 -- but then I see the test below
for it, so 👍 (presumably what happens is the arguments are coerced first and
`arg_type` refers to the *post* coerced type)
--
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]
