alamb commented on code in PR #10742:
URL: https://github.com/apache/datafusion/pull/10742#discussion_r1622967865
##########
datafusion/physical-expr-common/src/aggregate/mod.rs:
##########
@@ -185,6 +185,40 @@ pub trait AggregateExpr: Send + Sync + Debug +
PartialEq<dyn Any> {
fn create_sliding_accumulator(&self) -> Result<Box<dyn Accumulator>> {
not_impl_err!("Retractable Accumulator hasn't been implemented for
{self:?} yet")
}
+
+ /// Returns all expressions used in the [`AggregateExpr`].
+ /// These expressions are (1)function arguments, (2) order by expressions.
+ fn all_expressions(&self) -> AggregatePhysicalExpressions {
+ let args = self.expressions();
+ let order_bys = self.order_bys().unwrap_or(&[]);
+ let order_by_exprs = order_bys
+ .iter()
+ .map(|sort_expr| sort_expr.expr.clone())
+ .collect::<Vec<_>>();
+ AggregatePhysicalExpressions {
+ args,
+ order_by_exprs,
+ }
+ }
+
+ /// Rewrites [`AggregateExpr`], with new expressions given. The argument
should be consistent
+ /// with the return value of the [`AggregateExpr::all_expressions`] method.
+ /// Returns `Some(Arc<dyn AggregateExpr>)` if re-write is supported,
otherwise returns `None`.
+ fn with_new_expressions(
Review Comment:
The only thing that might be worth considering is that this API forces
cloning. However, since everything is `Arc`d that seems relatively minor. Maybe
something to keep in mind for the future.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
