uros-b commented on code in PR #59781:
URL: https://github.com/apache/spark/pull/59781#discussion_r4376647093
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/aggregate/percentiles.scala:
##########
@@ -195,12 +209,20 @@ abstract class PercentileBase
*/
protected def getPercentile(
accumulatedCounts: Seq[(AnyRef, Long)],
- percentile: Double): Double = {
+ percentile: Double): Any = {
val position = (accumulatedCounts.last._2 - 1) * percentile
+ // A double position can land just below a whole number (e.g. 90 * 0.7),
which flooring a
+ // nanosecond result would turn into a whole step.
+ lazy val exactPosition = JBigDecimal.valueOf(accumulatedCounts.last._2 - 1)
+ .multiply(JBigDecimal.valueOf(percentile))
Review Comment:
A NaN percentage now fails nanos groups with a raw
`java.lang.NumberFormatException`.
- `checkInputDataTypes` only rejects `percentage < 0.0 || percentage > 1.0`,
so a foldable NaN (`CAST('NaN' AS DOUBLE)`, PySpark `float('nan')`) passes
analysis.
- The nanos branch forces `exactPosition`, and
`JBigDecimal.valueOf(Double.NaN)` throws "Character N is neither a decimal
digit number, decimal point, nor "e" notation exponential mark." (reproduced on
JDK 17) on any non-empty group. It is reachable via `percentile(c, NaN)` (also
inside an array), `percentile_cont(NaN) WITHIN GROUP (ORDER BY c)` and legacy
`percentile_disc`, and the task fails with no error class.
- Every other input type, and non-legacy `percentile_disc` over nanos,
instead returns the first value in sort order (`NaN.floor.toLong == 0`).
Fix: take the double-position path when `percentile.isNaN`, which keeps
nanos aligned with the other types, and add a test. Rejecting NaN as
`VALUE_OUT_OF_RANGE` at :115 is the cleaner long-term rule, but it also changes
the numeric NaN result and `ApproximatePercentile` has the same check, so it
fits better as its own JIRA.
--
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]
