apache / datafusion

Apache DataFusion SQL Query Engine
https://datafusion.apache.org/
Apache License 2.0
5.48k stars 1.01k forks source link

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

Open alamb opened 3 days ago

alamb commented 3 days ago

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

Rachelint commented 3 days ago

take