datafusion icon indicating copy to clipboard operation
datafusion copied to clipboard

Remove `Min`/`Max` references from `AggregateExec::get_minmax_descr`

Open alamb opened this issue 1 year ago • 1 comments

Is your feature request related to a problem or challenge?

Part of https://github.com/apache/datafusion/issues/11151 (where we are removing special case uses of Min/Max)

Describe the solution you'd like

Remove the code here: https://github.com/apache/datafusion/blob/f58df32753a06dd1b67597a12cdf68007e249338/datafusion/physical-plan/src/aggregates/mod.rs#L485-L494

Specifically, the idea is to remove the pattern

       expr.as_any().downcast_ref::<Max>()

and

       expr.as_any().downcast_ref::<Min>()

Describe alternatives you've considered

Perhaps we can add function like this to AggregteExpr and implement it for Min and Max:

impl AggregateExpr {
    /// If this function is max, return (output_field, true)
    /// if the function is min, return (output_field, false)
    /// otherwise return None (the default)
    ///
    /// output_field is the name of the column produced by this aggregate
    ///
    /// Note: this is used to use special aggregate implementations in certain conditions
    pub fn get_min_max(&self) -> Option<(Field, bool)> { None }
...
}

Additional context

No response

alamb avatar Jun 28 '24 00:06 alamb

take

Rachelint avatar Jun 28 '24 02:06 Rachelint