search

apache / datafusion

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

Convert `stddev` to udaf #10827

Closed jayzhan211 closed 3 weeks ago

jayzhan211 commented 3 weeks ago

Is your feature request related to a problem or challenge?

Similar to #10713 and others in #8708

  1. Move code from datafusion/physical-expr/src/aggregate/stddev.rs to datafusion/functions-aggregate/src/stddev.rs
  2. Add expression api in roundtrip_expr_api in datafusion/proto/tests/cases/roundtrip_logical_plan.rs
  3. Use lowercase for name

Describe the solution you'd like

No response

Describe alternatives you've considered

No response

Additional context

No response

goldmedal commented 3 weeks ago

take

goldmedal commented 3 weeks ago

Hi @jayzhan211, Although this issue only mentions stddev, I'm thinking about stddev_pop. Should I also convert it to UDAF? I found that #10713 only converted var_sample but left variance_pop. Is there any reason we didn't convert variance_pop too?

jayzhan211 commented 3 weeks ago

Hi @jayzhan211, Although this issue only mentions stddev, I'm thinking about stddev_pop. Should I also convert it to UDAF? I found that #10713 only converted var_sample but left variance_pop. Is there any reason we didn't convert variance_pop too?

We should also convert population function, variance pop is tracked at https://github.com/apache/datafusion/issues/10668. stddev_pop is also on the todo list in #8708.

goldmedal commented 3 weeks ago

Got it. I think I can convert them in the same PR. Thanks.