jayzhan211 commented on issue #9972: URL: https://github.com/apache/arrow-datafusion/issues/9972#issuecomment-2041652715
> > Not AggregateUDF because it does not have ordering info. > > Does it need the ordering info? Or could we keep the code that makes the new [`AggregateFunctionExpr`](https://github.com/alamb/arrow-datafusion/blob/dfd4442da8332116c7e1f9fcd9de7c6da856442b/datafusion/physical-plan/src/udaf.rs#L74-L87) in the physical optimizer? > > Doesn't reverse simply need to needs to know what function to use when if the sort order is "reversed"? > > ```rust > impl AggregateUDFImpl { > ... > /// returns the "reverse" of this aggregate. Reverse means the function to use when the input ordering is > /// reversed. For example, `first_value` can be reversed to `last_value` if the ordering is reversed. > /// if the function is its own reverse, > fn reverse(&self) -> Option<Arc<Aggregate>> > ``` > > This seems to mirror how the current AggregateExpr works: > > https://github.com/alamb/arrow-datafusion/blob/a1ae15826245097e7c12d4f0ed3425b25af6c431/datafusion/physical-expr/src/aggregate/average.rs#L144-L146 We create Last with reversed ordering. `First` is equal to `Last with reversed ordering`. > could we keep the code that makes the new [AggregateFunctionExpr](https://github.com/alamb/arrow-datafusion/blob/dfd4442da8332116c7e1f9fcd9de7c6da856442b/datafusion/physical-plan/src/udaf.rs#L74-L87) in the physical optimizer It is probably a good idea. move the physical-expr conversion to the physical-optimizer, similar to function rewrite in the logical optimizer. ```rust pub fn convert_to_last(self) -> LastValuePhysicalExpr { let name = if self.name.starts_with("FIRST") { format!("LAST{}", &self.name[5..]) } else { format!("LAST_VALUE({})", self.expr) }; let FirstValuePhysicalExpr { expr, input_data_type, ordering_req, order_by_data_types, .. } = self; LastValuePhysicalExpr::new( expr, name, input_data_type, reverse_order_bys(&ordering_req), order_by_data_types, ) } ``` -- 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]
