Change Accumulator::evaluate and Accumulator::state to take &mut self

[alamb]

Which issue does this PR close?

Closes #8934
Part of #5472

Rationale for this change

I want to make a fast COUNT DISTINCT accumulator for strings, and a key optimization is to avoid copying the strings more than once. However, the current signature of Accumulator makes this tough

What changes are included in this PR?

1. Change Accumulator::evaluate and Accumulator::state to take &mut self
2. Update all uses
3. Fix median and distinct accumulators to avoid a copy

Alternate Considered: Accumulator::evaluate and Accumulator::state take self

Given the Accumulator is never used after calls to evaluate and merge a consuming API might make more sense -- and in fact I tried making these APIs consuming like this:

trait Accumulator {
...
    fn evaluate(self) -> Result<ScalarValue>;
...
}

However, the aggregator actually has Box<dyn Accumulator>:
https://github.com/apache/arrow-datafusion/blob/70ffb2fd3df7fbc744587ce3299ffb52c8ca6cbd/datafusion/physical-expr/src/aggregate/groups_accumulator/adapter.rs#L55-L63

I get a compiler error about about them not being sized when I try to call

let boxed_accumulator: Box<dyn Accumulator> = ...;
boxed_accumulator.evaluate()?

I think we could work around this, as described here by adding a evaluate_boxed and state_boxed variants, for example

trait Accumulator {
...
    fn evaluate(self) -> Result<ScalarValue>;
    fn evaluate_boxed(self: Box<Self>) -> Result<ScalarValue>;
...
}

However, I think that would just make the trait harder to use and &mut self is enough to implement zero copy semantics for those that want it

Are these changes tested?

Covered by existing tests

Are there any user-facing changes?

Yes, the signature of user defined Accumulators needs to be changed to take &mut self

[alamb]

This is the core change in this PR -- make evaluate and state get a &mut reference so the Accumulator can avoid copies

This is technically possible today (with "inner mutability", for example a Mutex) but that is both not documented and quite unobvious

[alamb]

This is an example of what DataFusion users have to do if this PR is merged -- they have to change the signatures to take mut

[alamb]

🎉 here is an example showing why this API change is good -- you can avoid this copy

This change this might actually improve the performance of median for large inputs measurably (though I have not measured it)

[alamb]

While this is an API change I think it is a minimally invasive one and will not cause too much pain downstream, and thus I plan to merge it tomorrow Tue Jan 22 2024 unless others would like more chance to review

Thank you for the PR review @Dandandan