Document and enforce compatibility between an aggregate's accumulators

[masonh22]

Which issue does this PR close?

• Closes Document and enforce compatibility of the intermediate state produced by an AggregateUDFImpl's Accumulator and GroupsAccumulator #25660.

Rationale for this change

See #25660.

This change only documents/enforces this requirement for built-in aggregate UDF functions. Anyone who implements their own aggregate UDFs do not need to adhere to this requirement since vanilla datafusion doesn't require it. This does add a public test that user's can use to make sure this requirement is satisfied if they desire.

What changes are included in this PR?

This change adds:

• Internal documentation explaining this requirement and how to check for it using the provided test function.
• datafusion_functions_aggregate::testing::check_state_compatibility(): A generic test that identifies cases where the Accumulator and GroupsAccumulator of an AggregateUDF differ. This is exposed as a public function that can be used to test this requirement on AggregateUDFImpls implemented by downstream consumers. The test is run on all built-in default aggregation functions.
• Fixes for approx_distinct() and avg(), which both violated this requirement
  • This takes the changes from Allow the HLLAccumulator used by approx_distinct() to consume state produced by HllGroupsAccumulator #25659 to fix approx_distinct()
  • The Accumulator for avg() would return 0 for the count if it hasn't seen any non-null values. The GroupsAccumulator would return null instead. This change updates the Accumulators to also return null for the count in this case.

What is the testing strategy for this PR?

This adds a test to enforce this new requirement in a generic way.

Are there any user-facing changes?

This changes the intermediate state of the avg() accumulator to match the avg() groups accumulator, which some users may be affected by.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 75.80645% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.79%. Comparing base (8248a57) to head (5273fe3).
⚠️ Report is 71 commits behind head on main.

Files with missing lines	Patch %	Lines
...afusion/functions-aggregate/src/approx_distinct.rs	77.77%	7 Missing and 3 partials ⚠️
datafusion/functions-aggregate/src/average.rs	70.58%	4 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #25710      +/-   ##
==========================================
+ Coverage   82.66%   82.79%   +0.12%     
==========================================
  Files        1147     1147              
  Lines      446542   450917    +4375     
  Branches   446542   450917    +4375     
==========================================
+ Hits       369154   373318    +4164     
+ Misses      54986    54880     -106     
- Partials    22402    22719     +317     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[masonh22]

@alamb tagging you because you might be interested in the optimization this enables, given that github considers you a contributor to Datafusion Query Cache

[alamb]

Thank you @masonh22 and @thinkharderdev and @Dandandan

This looks like a nice improvement code and testing wise to me, though I am a little worried about the somewhat non standard testing feature (mostly that it add some tiny more amount of complexity to a system that is already pretty complicated).

I left some comments, but I also think this PR could be merged as is

Thank you for the cleanup 🙏

[alamb]

Thank you @masonh22