ODriscollBoys opened a new pull request, #26176: URL: https://github.com/apache/datafusion/pull/26176
## Which issue does this PR close? - Closes #24900. ## Rationale for this change `approx_percentile_cont` (and `approx_percentile_cont_with_weight`) accept any positive integer literal as the `centroids` argument, which becomes the t-digest `max_size`. `TDigest::merge_sorted_f64` and `TDigest::merge_digests` preallocate the compressed centroid list with `Vec::with_capacity(max_size)`, so ```sql SELECT approx_percentile_cont(0.5, 9223372036854775807) WITHIN GROUP (ORDER BY x) FROM (VALUES (1.0), (2.0), (3.0)) t(x); ``` panics with `capacity overflow`, and large but representable values can abort the process when the allocation fails. #24901 proposed essentially the same change and was closed together with a batch of other PRs rather than on its merits; this PR redoes it on current `main`. ## What changes are included in this PR? The compressed digest can never contain more centroids than the centroids and values being merged, so the preallocation is bounded by that input size as well as by `max_size`: - `merge_sorted_f64`: `max_size.min(self.centroids.len() + sorted_values.len())` - `merge_digests`: `max_size.min(centroids.len())` The capacity is only a hint, so the compression logic itself is unchanged. As a side effect, small inputs with the default `max_size` of 100 no longer allocate room for 100 centroids. ## What is the testing strategy for this PR? - New unit test `test_large_max_size` in `tdigest.rs` that merges values and digests with `max_size = usize::MAX` (panics on `main`). - New `aggregate.slt` cases using `centroids = 9223372036854775807`, both for the query above and for a `GROUP BY` over `aggregate_test_100`, which also goes through the partial/final `merge_digests` path. The results match what the same query returns with `centroids = 10000` and with the default. ## Are there any user-facing changes? No panic for large `centroids` arguments; results are unchanged. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
