Skip to content

Commit

Permalink
Implementing feedback from code review
Browse files Browse the repository at this point in the history
  • Loading branch information
edmondop committed Aug 1, 2024
1 parent c7deaa8 commit ec95a77
Show file tree
Hide file tree
Showing 6 changed files with 106 additions and 187 deletions.
25 changes: 0 additions & 25 deletions datafusion/expr/src/type_coercion/aggregates.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ use arrow::datatypes::{
DataType, TimeUnit, DECIMAL128_MAX_PRECISION, DECIMAL128_MAX_SCALE,
DECIMAL256_MAX_PRECISION, DECIMAL256_MAX_SCALE,
};
use std::ops::Deref;

use datafusion_common::{internal_err, plan_err, Result};

Expand Down Expand Up @@ -142,22 +141,6 @@ pub fn check_arg_count(
Ok(())
}

pub fn get_min_max_result_type(input_types: &[DataType]) -> Result<Vec<DataType>> {
// make sure that the input types only has one element.
assert_eq!(input_types.len(), 1);
// min and max support the dictionary data type
// unpack the dictionary to get the value
match &input_types[0] {
DataType::Dictionary(_, dict_value_type) => {
// TODO add checker, if the value type is complex data type
Ok(vec![dict_value_type.deref().clone()])
}
// TODO add checker for datatype which min and max supported
// For example, the `Struct` and `Map` type are not supported in the MIN and MAX function
_ => Ok(input_types.to_vec()),
}
}

/// function return type of a sum
pub fn sum_return_type(arg_type: &DataType) -> Result<DataType> {
match arg_type {
Expand Down Expand Up @@ -327,14 +310,6 @@ pub fn coerce_avg_type(func_name: &str, arg_types: &[DataType]) -> Result<Vec<Da
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn test_get_min_max_return_type_coerce_dictionary() -> Result<()> {
let data_type =
DataType::Dictionary(Box::new(DataType::Utf8), Box::new(DataType::Int32));
let result = get_min_max_result_type(&[data_type])?;
assert_eq!(result, vec![DataType::Int32]);
Ok(())
}

#[test]
fn test_variance_return_data_type() -> Result<()> {
Expand Down
2 changes: 0 additions & 2 deletions datafusion/functions-aggregate/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,9 +112,7 @@ pub mod expr_fn {
pub use super::grouping::grouping;
pub use super::median::median;
pub use super::min_max::max;
pub use super::min_max::max_distinct;
pub use super::min_max::min;
pub use super::min_max::min_distinct;
pub use super::regr::regr_avgx;
pub use super::regr::regr_avgy;
pub use super::regr::regr_count;
Expand Down
Loading

0 comments on commit ec95a77

Please sign in to comment.