Jefffrey commented on code in PR #20208:
URL: https://github.com/apache/datafusion/pull/20208#discussion_r2778423798
##########
datafusion/functions-aggregate/src/percentile_cont.rs:
##########
@@ -58,17 +58,50 @@ use datafusion_macros::user_doc;
use crate::utils::validate_percentile_expr;
-/// Precision multiplier for linear interpolation calculations.
-///
-/// This value of 1,000,000 was chosen to balance precision with overflow
safety:
-/// - Provides 6 decimal places of precision for the fractional component
-/// - Small enough to avoid overflow when multiplied with typical numeric
values
-/// - Sufficient precision for most statistical applications
-///
-/// The interpolation formula: `lower + (upper - lower) * fraction`
-/// is computed as: `lower + ((upper - lower) * (fraction * PRECISION)) /
PRECISION`
-/// to avoid floating-point operations on integer types while maintaining
precision.
-const INTERPOLATION_PRECISION: usize = 1_000_000;
+/// Helper trait for safe interpolation on floating point native types
+/// (Float16 / Float32 / Float64).
+trait InterpolateNative: Copy {
Review Comment:
I feel there should be existing traits we can leverage for this
e.g.
-
https://docs.rs/half/latest/half/struct.f16.html#impl-AsPrimitive%3Cf64%3E-for-f16
-
https://docs.rs/half/latest/half/struct.f16.html#impl-AsPrimitive%3Cf16%3E-for-f64
##########
datafusion/sqllogictest/test_files/aggregate.slt:
##########
@@ -3926,7 +3926,7 @@ SELECT percentile_cont(0.95) WITHIN GROUP (ORDER BY c3
DESC) FROM aggregate_test
query R
SELECT percentile_cont(0.05) WITHIN GROUP (ORDER BY c3 DESC) FROM
aggregate_test_100
----
-118.099998
+118.1
Review Comment:
Not sure if this precision loss for other float types is desirable as part
of these changes
--
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]
