HyukjinKwon commented on code in PR #59408:
URL: https://github.com/apache/spark/pull/59408#discussion_r4270322773
##########
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/MathExpressionsSuite.scala:
##########
@@ -748,6 +749,185 @@ class MathExpressionsSuite extends SparkFunSuite with
ExpressionEvalHelper {
checkEvaluation(Truncate(Literal(1.23), Literal.create(null,
IntegerType)), null)
}
+ Seq("floor", "ceil").foreach { name =>
+ test(s"SPARK-60202: scaled $name preserves Double rounding boundaries") {
+ val cases = if (name == "floor") {
+ Seq(
+ (0.8999999999999999, 1, "0.8"),
+ (1.7999999999999998, 1, "1.7"),
+ (0.9999999999999999, 0, "0"),
+ (-0.9000000000000001, 1, "-1.0"))
+ } else {
+ Seq(
+ (0.9000000000000001, 1, "1.0"),
+ (-0.8999999999999999, 1, "-0.8"),
+ (-1.7999999999999998, 1, "-1.7"),
+ (-0.9999999999999999, 0, "0"))
+ }
+ cases.foreach { case (value, scale, expected) =>
+ val input = BoundReference(0, DoubleType, nullable = true)
+ val rounding = if (name == "floor") {
+ RoundFloor(input, Literal(scale))
+ } else {
+ RoundCeil(input, Literal(scale))
+ }
+ val expression = checkDataTypeAndCast(rounding)
+ assert(expression.dataType == DecimalType(16 + scale, scale))
+ checkEvaluation(expression, Decimal(BigDecimal(expected)),
InternalRow(value))
+ }
+ }
+ }
+
+ test("SPARK-60202: scaled floor/ceil preserves Float rounding boundaries") {
+ val cases = Seq(
+ (java.lang.Math.nextDown(0.1f), "0.0", "0.1"),
+ (java.lang.Math.nextUp(0.9f), "0.9", "1.0"),
+ (-java.lang.Math.nextDown(0.1f), "-0.1", "0.0"),
+ (-java.lang.Math.nextUp(0.9f), "-1.0", "-0.9"))
+ cases.foreach { case (value, down, up) =>
+ val input = BoundReference(0, FloatType, nullable = true)
+ checkEvaluation(checkDataTypeAndCast(RoundFloor(input, Literal(1))),
+ Decimal(BigDecimal(down)), InternalRow(value))
+ checkEvaluation(checkDataTypeAndCast(RoundCeil(input, Literal(1))),
+ Decimal(BigDecimal(up)), InternalRow(value))
+ }
+ }
+
+ test("SPARK-60202: scaled floor/ceil preserves result types and explicit
decimal casts") {
+ Seq(ByteType, ShortType, IntegerType, LongType, FloatType,
DoubleType).foreach { dt =>
+ Seq(-3, 0, 1, 15, 20).foreach { scale =>
+ val input = BoundReference(0, dt, nullable = true)
+ val decimal = Cast(input, DecimalType.forType(dt))
+ val actual = Seq(RoundFloor(input, Literal(scale)), RoundCeil(input,
Literal(scale)))
+ val previous = Seq(RoundFloor(decimal, Literal(scale)),
RoundCeil(decimal, Literal(scale)))
+ actual.zip(previous).foreach { case (rounding, control) =>
+ assert(checkDataTypeAndCast(rounding).dataType == control.dataType)
+ }
+ }
+ }
+ val input = BoundReference(0, DoubleType, nullable = true)
+ val explicitCast = Cast(input, DecimalType(30, 15))
+ checkEvaluation(RoundFloor(explicitCast, Literal(1)),
Decimal(BigDecimal("0.9")),
+ InternalRow(0.8999999999999999))
+ checkEvaluation(RoundCeil(explicitCast, Literal(1)),
Decimal(BigDecimal("0.9")),
+ InternalRow(0.9000000000000001))
+ }
+
+ test("SPARK-60202: scaled floor/ceil preserves null, non-finite and input
overflow behavior") {
+ for (ansi <- Seq(false, true); dt <- Seq(FloatType, DoubleType)) {
+ withSQLConf(SQLConf.ANSI_ENABLED.key -> ansi.toString) {
+ def toInput(value: Double): Any = if (dt == FloatType) value.toFloat
else value
+ val input = BoundReference(0, dt, nullable = true)
+ val expressions = Seq(RoundFloor(input, Literal(1)), RoundCeil(input,
Literal(1)))
+ .map(checkDataTypeAndCast)
+ expressions.foreach { expression =>
+ checkEvaluation(expression, null, InternalRow(null))
+ Seq(Double.NaN, Double.PositiveInfinity,
Double.NegativeInfinity).foreach { value =>
+ checkEvaluation(expression, null, InternalRow(toInput(value)))
+ }
+ Seq(0.0, -0.0).foreach { value =>
+ checkEvaluation(expression, Decimal(0),
InternalRow(toInput(value)))
+ }
+ val limit = if (dt == FloatType) 1e7 else 1e15
Review Comment:
**Nit (P3):** These `+/-limit` cases only cover the overflow side of the new
`inputLimit` threshold in `roundFloatingPoint`. No floor/ceil test evaluates a
large value inside the range, so if the limit were misderived (for example
`10^(p - s - 1)`) or a value just below it were misclassified, every
SPARK-60202 test would still pass while valid inputs in `[1e14, 1e15)` / `[1e6,
1e7)` returned NULL or raised `NUMERIC_VALUE_OUT_OF_RANGE`. Could you add
in-range cases next to these, e.g. `Math.nextDown(1e15)` and `9999999.0f` (plus
negations) in both ANSI modes? A scale-0 Double case where
`ceil(Math.nextDown(1e15), 0)` carries to `1000000000000000` would also pin the
extra integral digit of the result type.
**Verification:**
- **Regression:** Lowering or misderiving the input-range threshold (for
example using 10^(precision - scale - 1), or misclassifying values just below
the limit) makes the new in-range assertions fail in interpreted and generated
evaluation.
- **Behavior:** Near-limit in-range Float and Double inputs produce the
expected floor/ceil decimals with ANSI enabled and disabled, while the existing
exactly-at-limit cases keep their NULL/error results.
--
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]
