westonpace opened a new pull request, #10627: URL: https://github.com/apache/datafusion/pull/10627
## Which issue does this PR close? Closes #8031 ## Rationale for this change See the motivating issue. ## What changes are included in this PR? The `MinAccumulator` and `MaxAccumlator` now use `ScalarValue::partial_cmp` to compare two float scalars instead of `f32::min` and `f32::max` (or `f64`). This is an approach that was already being taken for intervals. ## Are these changes tested? Yes, I added a unit test. I did not see any existing benchmarks for the accumulators. I'm not entirely certain if this approach is slower or not. However, I think a performance regression is unlikely since this only changes how we compare the intermediate results. I.e. given two arrays we will use the arrow kernels to find the min/max of the two arrays. Then we only use this path to compare the result of those two calculations. (e.g. we are in per-batch space and not per-row space) ## Are there any user-facing changes? No. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org For additional commands, e-mail: github-h...@datafusion.apache.org
