Raviston6296 opened a new pull request, #60024:
URL: https://github.com/apache/spark/pull/60024
### What changes were proposed in this pull request?
`percentile_disc(p)` returns the value with the smallest `cume_dist()` that
is greater than or equal to `p`. `PercentileDisc.getPercentile` found that row
as `ceil(total * p)`, computed in double. This PR keeps that as an estimate and
then corrects it by comparing `k / total >= p`, which is how `cume_dist()`
computes its value (`rowNumber.cast(DoubleType) / n.cast(DoubleType)`). The
result is the smallest row count `k` that `cume_dist()` would also pick.
The legacy calculation (`spark.sql.legacy.percentileDiscCalculation=true`)
is unchanged. A NaN percentage, which passes the range check, still returns the
first value.
### Why are the changes needed?
`total * p` can land just above a whole number in binary floating point, and
`ceil` then moves to the next row:
```sql
SELECT percentile_disc(0.28) WITHIN GROUP (ORDER BY id),
percentile_disc(0.28) WITHIN GROUP (ORDER BY id DESC)
FROM range(1, 26);
-- before: 8.0, 18.0
-- after: 7.0, 19.0
```
`25 * 0.28` is `7.000000000000001` as a double, so the ceiling is 8.
`cume_dist()` of 7 over 1..25 is `7 / 25`, which is the same double as `0.28`,
so 7 is the expected result, and Spark's own `cume_dist()` agrees. The same
happens for other pairs, such as `percentile_disc(0.56)` over 50 rows (29
instead of 28).
### Does this PR introduce _any_ user-facing change?
Yes. `percentile_disc` returns the correct value in these cases. Before, it
could return the next value in sort order when `n * p` is not exact in double.
### How was this patch tested?
- `PercentileSuite`: a new test checks the cases from the JIRA ticket,
ascending and descending, and compares `percentile_disc(p)` over `1..n` with
the first value whose `k / n >= p` for every `n` in 1..100 and `p` in
0.00..1.00. It fails without this change (`8.0 did not equal 7.0`).
- `percentiles.sql`: new queries for the cases above, plus a `cume_dist()`
query that gives the same answer. Golden files regenerated with
`SPARK_GENERATE_GOLDEN_FILES=1`.
- `build/sbt 'catalyst/testOnly *PercentileSuite'` and `build/sbt
"sql/testOnly org.apache.spark.sql.SQLQueryTestSuite -- -z percentiles.sql"`
pass.
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Claude Opus 5.5)
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
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]
