Jeffrey Vo

Thanks for checking this @carlosahs Sounds a bit complex, perhaps the easiest way forward is locally allow the allow, e.g. ```rust impl PhysicalOptimizerRule for AggregateStatistics { #[cfg_attr(feature = "recursive_protection", recursive::recursive)]...

> [@Jefffrey](https://github.com/Jefffrey) I looked into the reamining three and I think they don't require the changes. > > * `datafusion-proto`: We should keep the parquet `max_statistics_size` lint as `#[allow(deprecated)]` because...

> Interesting, I didn't realise negative scales aren't allowed. I assumed they were as arrow allows negative scales in decimal. https://github.com/apache/arrow-rs/blob/main/arrow-schema/src/datatype.rs#L359-L372 Negative scales are allowed; I believe any places in...

In the original PR that kicked off this effort (#17023) it converts the decimal128 to the native i128 representation before doing an integer log, as converting to f64 apparently causes...

> Ok, instead of converting to float I'll keep it as integers and perform and integer log. > > Just one thing though the `log` function in DuckDB and Clickhouse...

Checklist of things needed to be implemented. For reference existing implementation: https://github.com/apache/datafusion/blob/2a08013af3ccf703bee202c959b40bb0d35bdea1/datafusion/functions-aggregate/src/variance.rs Distinct accumulator for variance, see example for median: https://github.com/apache/datafusion/blob/2a08013af3ccf703bee202c959b40bb0d35bdea1/datafusion/functions-aggregate/src/median.rs#L511-L542 Make sure to wire distinct accumulator into the UDAF:...

One of the key requirements that we want here is that decimal inputs are not cast to floats, and the return type remains decimal. I think if you check the...

Unfortunately I don't currently have the capacity to try guide this PR along; I feel some of my previous comments have still not been addressed and a lot of the...

I took another quick look, and I think some of my prior comments were inaccurate, I apologize for that. I was mainly thrown by some parts of the code which...

Some pointers. Where to do implementation: https://github.com/apache/datafusion/blob/2a08013af3ccf703bee202c959b40bb0d35bdea1/datafusion/functions-aggregate/src/median.rs See the methods to implement from trait: https://github.com/apache/datafusion/blob/2a08013af3ccf703bee202c959b40bb0d35bdea1/datafusion/expr-common/src/accumulator.rs#L298-L313 Example retract_batch from other method: https://github.com/apache/datafusion/blob/79a2f5e110f2a3ddbe6d943f951bd6f518549b03/datafusion/functions-aggregate/src/average.rs#L554-L565 Add tests here: https://github.com/apache/datafusion/blob/79a2f5e110f2a3ddbe6d943f951bd6f518549b03/datafusion/sqllogictest/test_files/aggregate.slt - See suggested tests in...