search

apache / datafusion

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

Move Accumulator trait to physical-expr #2349

Open yjshen opened 2 years ago

yjshen commented 2 years ago

Is your feature request related to a problem or challenge? Please describe what you are trying to do. Accumulator is the state of an aggregate function, and has nothing to do with a logical expression.

The current blocker is AggregateUDF which mixes both the function signature and the function implementation. AggregateUDF exists in datafusion-expr because of its signature parts.

Describe the solution you'd like

  1. Move Accumulator trait into datafusion-physical-expr.
  2. Split AggregateUDF into parts, one remains in datafusion-expr that represents AggregateUDF's signature, and the other in datafusion-physical-expr for its implementation.

Describe alternatives you've considered Don't move.

alamb commented 2 years ago

This makes sense to me

alamb commented 9 months ago

I am not sure this makes sense anymore given the UDFs are defined in datafusion_expr now and they need Acumulators