[CALCITE-7162] AggregateMergeRule type mismatch on MIN/MAX

[gianm]

Fixes CALCITE-7162 by taking the type from the top aggregator rather than the bottom aggregator when merging. A similar fix was made to AbstractSumSplitter in CALCITE-6557.

This patch also updates CountSplitter with a similar change— just in case— even though I'm not sure if that is necessary to fix any actual bug. I believe that in general using the top aggregator type is the correct thing to do, since the merged aggregator is meant to stand in for the top aggregator.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[xiedeyantu]

If you have also updated the CountSplitter, I hope you can add a relevant test case.

[mihaibudiu]

This looks fine, but it would be great if @julianhyde could confirm.

[julianhyde]

Can we have a generic solution that works for all aggregate functions?

[gianm]

Can we have a generic solution that works for all aggregate functions?

In this patch I updated all existing SqlSplittableAggFunction implementations. If there is something more generic you had in mind, please let me know.

If you have also updated the CountSplitter, I hope you can add a relevant test case.

I tried for a bit to find a case where the current CountSplitter breaks, but couldn't, so I didn't add a test case. There are already a lot of test cases for COUNT splitting. It's possible the current code is "fine" because COUNT splits into COUNT and SUM0, which won't have different nullability. I updated the CountSplitter anyway just to keep all the Splitter implementations harmonious.

[gianm]

Thanks all for having a look. I will commit it since I believe I've addressed the comments as best as they can be.