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]

Reply via email to