This is an automated email from the ASF dual-hosted git repository.
gengliangwang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/spark.git
The following commit(s) were added to refs/heads/master by this push:
new bbe1b19aa17b [SPARK-58040][SQL] Extract a MathUtils.doubleToLong
helper to simplify Ceil/Floor overflow codegen
bbe1b19aa17b is described below
commit bbe1b19aa17bb723975f9b20006007aa7b78f03e
Author: Gengliang Wang <[email protected]>
AuthorDate: Fri Jul 10 11:42:28 2026 -0700
[SPARK-58040][SQL] Extract a MathUtils.doubleToLong helper to simplify
Ceil/Floor overflow codegen
### What changes were proposed in this pull request?
`Ceil` and `Floor` (ANSI mode, `DOUBLE -> LONG`) check for overflow before
casting the rounded
double to long. That check is currently implemented twice, in a private
`CeilFloor` helper object in
`mathExpressions.scala`:
- `CeilFloor.doubleToLong` for the interpreted path, and
- `CeilFloor.doubleToLongCode`, which re-emits the bound check as ~9 lines
of Java into every
`Ceil`/`Floor` codegen stage.
This PR replaces both with a single shared `MathUtils.doubleToLong(value,
context)` utility that the
interpreted and generated code both call, and deletes the `CeilFloor`
object. The generated code
collapses from the inline block:
```java
double roundedValue = java.lang.Math.ceil(input);
if (!java.lang.Double.isNaN(roundedValue) &&
(roundedValue < (double) java.lang.Long.MIN_VALUE ||
roundedValue >= (double) java.lang.Long.MAX_VALUE)) {
throw QueryExecutionErrors.arithmeticOverflowError("long overflow", "",
errCtx);
}
result = (long) roundedValue;
```
to a single call:
```java
result = org.apache.spark.sql.catalyst.util.MathUtils.doubleToLong(
java.lang.Math.ceil(input), errCtx);
```
`MathUtils` is the natural home: it already hosts the other
arithmetic-overflow helpers
(`addExact`, `negateExact`, `withOverflow`) and follows the same "shared by
the eval and codegen
paths so the two never diverge" convention already used for `pmod`. Placing
`doubleToLong` there also
means one fewer copy of the bound check emitted per stage (part of the
broader effort to shrink
whole-stage-codegen output, SPARK-56908).
### Why are the changes needed?
The overflow check is type-independent bookkeeping that does not need to be
re-emitted into each
generated stage. Consolidating it into `MathUtils`:
- removes the duplicated logic (the `CeilFloor.doubleToLong` eval copy and
the
`doubleToLongCode` string builder collapse into one method), so the two
paths cannot drift, and
- shrinks the generated code for every ANSI `CEIL`/`FLOOR` over a `DOUBLE`
from the multi-line
bound check to a single call.
### Does this PR introduce _any_ user-facing change?
No. The check, the error (`ARITHMETIC_OVERFLOW`), and the query context are
identical; `defineCodeGen`
applies the same null-safety wrapper the previous `nullSafeCodeGen` did.
This is a pure
code-organization change.
### How was this patch tested?
Existing tests, run with whole-stage codegen both on and off:
- `MathExpressionsSuite` (`ceil` and `floor`, including the ANSI overflow
cases and
`checkConsistencyBetweenInterpretedAndCodegenAllowingException` for
`DoubleType`), and
- regenerated `postgreSQL/float8.sql.out` (unchanged, confirming the
behavior is preserved).
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Opus 4.8)
Closes #57143 from gengliangwang/SPARK-58040-mathutils-doubletolong.
Authored-by: Gengliang Wang <[email protected]>
Signed-off-by: Gengliang Wang <[email protected]>
---
.../apache/spark/sql/catalyst/util/MathUtils.scala | 12 ++++++
.../sql/catalyst/expressions/mathExpressions.scala | 48 ++++------------------
2 files changed, 20 insertions(+), 40 deletions(-)
diff --git
a/sql/api/src/main/scala/org/apache/spark/sql/catalyst/util/MathUtils.scala
b/sql/api/src/main/scala/org/apache/spark/sql/catalyst/util/MathUtils.scala
index a4c6c75358f4..e318a60ce99a 100644
--- a/sql/api/src/main/scala/org/apache/spark/sql/catalyst/util/MathUtils.scala
+++ b/sql/api/src/main/scala/org/apache/spark/sql/catalyst/util/MathUtils.scala
@@ -123,6 +123,18 @@ object MathUtils {
if (r < 0) (r + n) % n else r
}
+ // Casts a rounded double (the result of Math.ceil/Math.floor) to long,
throwing an arithmetic
+ // overflow error when the value cannot be represented as a long. NaN is
passed through to the
+ // JVM cast (which yields 0), matching the previous behavior. Shared by the
eval and codegen
+ // paths of `Ceil`/`Floor` so the two never diverge.
+ def doubleToLong(value: Double, context: QueryContext): Long = {
+ if (!value.isNaN &&
+ (value < Long.MinValue.toDouble || value >= Long.MaxValue.toDouble)) {
+ throw ExecutionErrors.arithmeticOverflowError("long overflow", context =
context)
+ }
+ value.toLong
+ }
+
def withOverflow[A](f: => A, hint: String = "", context: QueryContext =
null): A = {
try {
f
diff --git
a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala
b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala
index 7a76968018c0..81b576146abe 100644
---
a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala
+++
b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala
@@ -252,34 +252,6 @@ case class Cbrt(child: Expression) extends
UnaryMathExpression(math.cbrt, "CBRT"
override protected def withNewChildInternal(newChild: Expression): Cbrt =
copy(child = newChild)
}
-private object CeilFloor {
- def doubleToLong(value: Double, context: QueryContext): Long = {
- if (!value.isNaN &&
- (value < Long.MinValue.toDouble || value >= Long.MaxValue.toDouble)) {
- throw QueryExecutionErrors.arithmeticOverflowError("long overflow",
context = context)
- }
- value.toLong
- }
-
- def doubleToLongCode(
- input: String,
- funcName: String,
- result: String,
- roundedValue: String,
- errorContext: String): String = {
- s"""
- |double $roundedValue = java.lang.Math.$funcName($input);
- |if (!java.lang.Double.isNaN($roundedValue) &&
- | ($roundedValue < (double) java.lang.Long.MIN_VALUE ||
- | $roundedValue >= (double) java.lang.Long.MAX_VALUE)) {
- | throw QueryExecutionErrors.arithmeticOverflowError(
- | "long overflow", "", $errorContext);
- |}
- |$result = (long) $roundedValue;
- |""".stripMargin
- }
-}
-
case class Ceil(child: Expression, failOnError: Boolean =
SQLConf.get.ansiEnabled)
extends UnaryMathExpression(math.ceil, "CEIL") with SupportQueryContext {
override def dataType: DataType = child.dataType match {
@@ -299,7 +271,7 @@ case class Ceil(child: Expression, failOnError: Boolean =
SQLConf.get.ansiEnable
protected override def nullSafeEval(input: Any): Any = child.dataType match {
case LongType => input.asInstanceOf[Long]
case DoubleType if failOnError =>
- CeilFloor.doubleToLong(f(input.asInstanceOf[Double]), getContextOrNull())
+ MathUtils.doubleToLong(f(input.asInstanceOf[Double]), getContextOrNull())
case DoubleType => f(input.asInstanceOf[Double]).toLong
case DecimalType.Fixed(_, _) => input.asInstanceOf[Decimal].ceil
}
@@ -311,11 +283,9 @@ case class Ceil(child: Expression, failOnError: Boolean =
SQLConf.get.ansiEnable
defineCodeGen(ctx, ev, c => s"$c.ceil()")
case LongType => defineCodeGen(ctx, ev, c => s"$c")
case DoubleType if failOnError =>
- nullSafeCodeGen(ctx, ev, c => {
- val roundedValue = ctx.freshName("roundedValue")
- val errorContext = getContextOrNullCode(ctx)
- CeilFloor.doubleToLongCode(c, funcName, ev.value, roundedValue,
errorContext)
- })
+ val mathUtils = MathUtils.getClass.getCanonicalName.stripSuffix("$")
+ defineCodeGen(ctx, ev, c =>
+ s"$mathUtils.doubleToLong(java.lang.Math.$funcName($c),
${getContextOrNullCode(ctx)})")
case _ => defineCodeGen(ctx, ev, c =>
s"(long)(java.lang.Math.${funcName}($c))")
}
}
@@ -591,7 +561,7 @@ case class Floor(child: Expression, failOnError: Boolean =
SQLConf.get.ansiEnabl
protected override def nullSafeEval(input: Any): Any = child.dataType match {
case LongType => input.asInstanceOf[Long]
case DoubleType if failOnError =>
- CeilFloor.doubleToLong(f(input.asInstanceOf[Double]), getContextOrNull())
+ MathUtils.doubleToLong(f(input.asInstanceOf[Double]), getContextOrNull())
case DoubleType => f(input.asInstanceOf[Double]).toLong
case DecimalType.Fixed(_, _) => input.asInstanceOf[Decimal].floor
}
@@ -603,11 +573,9 @@ case class Floor(child: Expression, failOnError: Boolean =
SQLConf.get.ansiEnabl
defineCodeGen(ctx, ev, c => s"$c.floor()")
case LongType => defineCodeGen(ctx, ev, c => s"$c")
case DoubleType if failOnError =>
- nullSafeCodeGen(ctx, ev, c => {
- val roundedValue = ctx.freshName("roundedValue")
- val errorContext = getContextOrNullCode(ctx)
- CeilFloor.doubleToLongCode(c, funcName, ev.value, roundedValue,
errorContext)
- })
+ val mathUtils = MathUtils.getClass.getCanonicalName.stripSuffix("$")
+ defineCodeGen(ctx, ev, c =>
+ s"$mathUtils.doubleToLong(java.lang.Math.$funcName($c),
${getContextOrNullCode(ctx)})")
case _ => defineCodeGen(ctx, ev, c =>
s"(long)(java.lang.Math.${funcName}($c))")
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]