alamb commented on code in PR #9960:
URL: https://github.com/apache/arrow-datafusion/pull/9960#discussion_r1554552119
##########
datafusion/functions-aggregate/src/first_last.rs:
##########
@@ -96,7 +96,45 @@ impl AggregateUDFImpl for FirstValue {
}
fn accumulator(&self, acc_args: AccumulatorArgs) -> Result<Box<dyn
Accumulator>> {
- (self.accumulator)(acc_args)
+ let mut all_sort_orders = vec![];
+
+ // Construct PhysicalSortExpr objects from Expr objects:
+ let mut sort_exprs = vec![];
Review Comment:
👍
##########
datafusion/functions-aggregate/src/lib.rs:
##########
@@ -82,3 +92,260 @@ pub fn register_all(registry: &mut dyn FunctionRegistry) ->
Result<()> {
Ok(())
}
+
+/// Creates a physical expression of the UDAF, that includes all necessary
type coercion.
+/// This function errors when `args`' can't be coerced to a valid argument
type of the UDAF.
+pub fn create_aggregate_expr(
+ fun: &AggregateUDF,
+ input_phy_exprs: &[Arc<dyn PhysicalExpr>],
+ sort_exprs: &[Expr],
+ ordering_req: &[PhysicalSortExpr],
+ schema: &Schema,
+ name: impl Into<String>,
+ ignore_nulls: bool,
+) -> Result<Arc<dyn AggregateExpr>> {
+ let input_exprs_types = input_phy_exprs
+ .iter()
+ .map(|arg| arg.data_type(schema))
+ .collect::<Result<Vec<_>>>()?;
+
+ let ordering_types = ordering_req
+ .iter()
+ .map(|e| e.expr.data_type(schema))
+ .collect::<Result<Vec<_>>>()?;
+
+ let ordering_fields = ordering_fields(ordering_req, &ordering_types);
+
+ Ok(Arc::new(AggregateFunctionExpr {
+ fun: fun.clone(),
+ args: input_phy_exprs.to_vec(),
+ data_type: fun.return_type(&input_exprs_types)?,
+ name: name.into(),
+ schema: schema.clone(),
+ sort_exprs: sort_exprs.to_vec(),
+ ordering_req: ordering_req.to_vec(),
+ ignore_nulls,
+ ordering_fields,
+ }))
+}
+
+/// An aggregate expression that:
+/// * knows its resulting field
+/// * knows how to create its accumulator
+/// * knows its accumulator's state's field
+/// * knows the expressions from whose its accumulator will receive values
+///
+/// Any implementation of this trait also needs to implement the
+/// `PartialEq<dyn Any>` to allows comparing equality between the
+/// trait objects.
+pub trait AggregateExpr: Send + Sync + Debug + PartialEq<dyn Any> {
Review Comment:
> I also move AggregateFunctionExpr and create_aggregate_expr to
functions-aggregate from physical-expr-common
While I agree that these two structures are aggregate specific, I still
think they should be in `datafusion-physical-expr-common` because that would
allow people to use DataFusion and create their own aggregate functions
*without* having to take DataFusion's built in aggregate functions
I see real value in keeping the APIs required to use datafusion separate
from the actual implementations
--
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]
