Skip to content

fix: preserve floating-point percentile interpolation precision - #25275

Open
aoto-tech wants to merge 2 commits into
apache:mainfrom
aoto-tech:fix-percentile-cont-float-interpolation
Open

aoto-tech wants to merge 2 commits into
apache:mainfrom
aoto-tech:fix-percentile-cont-float-interpolation

Conversation

@aoto-tech

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Rationale for this change

For floating-point inputs, percentile_cont currently truncates the interpolation weight to six decimal places. A small but nonzero percentile such as 2.5e-7 therefore returns the lower input value instead of an interpolated result.

What changes are included in this PR?

I removed the integer quantization from the float path and apply the f64 interpolation fraction directly. Decimal interpolation still uses the existing fixed precision; I renamed the constant to make that boundary explicit.

I also use separately weighted terms when finite endpoints have opposite signs. The usual difference-based form stays in place otherwise, but it would overflow before applying the fraction for a range such as -f64::MAX..=f64::MAX.

What is the testing strategy for this PR?

I added a unit test that exercises the reported sub-millionth weight through calculate_percentile for Float16, Float32, and Float64. There is also a focused test for the opposite-extreme overflow path, plus an sqllogictest covering the three float types. Three existing expected values change because they previously included the six-decimal truncation error.

I ran:

  • cargo test --locked -p datafusion-functions-aggregate --lib -j 4 (234 passed)
  • cargo test --locked -j 4 --test sqllogictests -- aggregate.slt
  • cargo fmt --all -- --check

I also ran Clippy for all targets in datafusion-functions-aggregate. With Rust 1.97 I had to allow the pre-existing unnecessary_semicolon warning in datafusion/common/src/rounding.rs:257; otherwise Clippy stops in that dependency before reaching this crate.

Are there any user-facing changes?

Yes. percentile_cont now preserves the full interpolation weight for floating-point inputs. There is no public API change.

@github-actions github-actions Bot added sqllogictest SQL Logic Tests (.slt) functions Changes to functions implementation labels Sep 13, 2026
@aoto-tech
aoto-tech marked this pull request as ready for review September 13, 2026 16:16
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.91%. Comparing base (85d4cbb) to head (9568827).

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #25275   +/-   ##
=======================================
  Coverage   81.91%   81.91%           
=======================================
  Files        1134     1134           
  Lines      425631   425673   +42     
  Branches   425631   425673   +42     
=======================================
+ Hits       348647   348685   +38     
- Misses      56304    56308    +4     
  Partials    20680    20680           

☔ 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

functions Changes to functions implementation sqllogictest SQL Logic Tests (.slt)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

percentile_cont truncates Float64 interpolation weights to six decimal places

2 participants