(spark) branch branch-4.x updated: [SPARK-58040][SQL] Extract a MathUtils.doubleToLong helper to simplify Ceil/Floor overflow codegen

[ge...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

gengliangwang pushed a commit to branch branch-4.x
in repository https://gitbox.apache.org/repos/asf/spark.git


The following commit(s) were added to refs/heads/branch-4.x by this push:
     new 36c8ac0187c5 [SPARK-58040][SQL] Extract a MathUtils.doubleToLong helper to simplify Ceil/Floor overflow codegen
36c8ac0187c5 is described below

commit 36c8ac0187c5a0f727707700a07b9f8b9b5da956
Author: Gengliang Wang <ge...@apache.org>
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 <ge...@apache.org>
    Signed-off-by: Gengliang Wang <ge...@apache.org>
    (cherry picked from commit bbe1b19aa17bb723975f9b20006007aa7b78f03e)
    Signed-off-by: Gengliang Wang <ge...@apache.org>
---
 .../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: commits-unsubscribe@spark.apache.org
For additional commands, e-mail: commits-help@spark.apache.org