From 24bcb754bf1c6ab78366e777af3510b4d54ca437 Mon Sep 17 00:00:00 2001 From: Gengliang Wang Date: Sat, 30 May 2026 23:35:39 +0000 Subject: [PATCH] [SPARK-57177][SQL] Simplify Acosh codegen by extracting a static Java helper ### What changes were proposed in this pull request? Add `ExpressionImplUtils.acosh(double x)` (the fdlibm `e_acosh.c` algorithm) and route `Acosh`'s eval and codegen paths through it. `Acosh.doGenCode` previously emitted a 12-line inline five-branch if/else; it now emits a single `ExpressionImplUtils.acosh(...)` call via `defineCodeGen`, and the eval path (the `UnaryMathExpression` function) calls the same helper instead of an inline lambda copy of the algorithm. `acosh` returns a primitive `double` (NaN for out-of-domain input), so the helper is a clean drop-in for both paths with no boxing and no null handling. This is a plain (non-ANSI, non-try/catch) type-independent block, in line with the broadened goal of SPARK-56908 to move fixed generated-Java logic into static Java helpers. ### Why are the changes needed? Part of SPARK-56908 (umbrella). Collapsing the duplicated 12-line algorithm to a single call shrinks the generated Java for every stage that uses `acosh`, and removes the eval/codegen duplication of the same fdlibm algorithm. ### Does this PR introduce _any_ user-facing change? No. The compiled behavior is identical; only the emitted Java source text changes. ### How was this patch tested? ``` build/sbt "catalyst/testOnly *MathExpressionsSuite" ``` 54/54 pass, including `acosh` and `SPARK-56089: asinh/acosh fdlibm algorithm coverage` (exercised both with and without whole-stage codegen). ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Code (Opus 4.8) Co-authored-by: Isaac --- .../expressions/ExpressionImplUtils.java | 21 +++++++++++ .../expressions/mathExpressions.scala | 37 ++----------------- 2 files changed, 25 insertions(+), 33 deletions(-) diff --git a/sql/catalyst/src/main/java/org/apache/spark/sql/catalyst/expressions/ExpressionImplUtils.java b/sql/catalyst/src/main/java/org/apache/spark/sql/catalyst/expressions/ExpressionImplUtils.java index a5228edc33c83..ea8c14d565edc 100644 --- a/sql/catalyst/src/main/java/org/apache/spark/sql/catalyst/expressions/ExpressionImplUtils.java +++ b/sql/catalyst/src/main/java/org/apache/spark/sql/catalyst/expressions/ExpressionImplUtils.java @@ -342,4 +342,25 @@ public static UTF8String quote(UTF8String str) { String sp = str.toString().replaceAll(qtChar, qtCharRep); return UTF8String.fromString(qtChar + sp + qtChar); } + + /** + * Inverse hyperbolic cosine for the {@code acosh} expression, using the + * fdlibm {@code e_acosh.c} algorithm (returns {@code NaN} for {@code x < 1}). + * Shared by the eval and codegen paths so the generated Java is a single call + * rather than an inline five-branch if/else. + */ + public static double acosh(double x) { + if (x < 1.0) { + return Double.NaN; + } else if (x >= (1 << 28)) { + return StrictMath.log(x) + StrictMath.log(2.0); + } else if (x == 1.0) { + return 0.0; + } else if (x > 2.0) { + return StrictMath.log(2.0 * x - 1.0 / (x + Math.sqrt(x * x - 1.0))); + } else { + double t = x - 1.0; + return StrictMath.log1p(t + Math.sqrt(2.0 * t + t * t)); + } + } } 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 56fbb00131d57..35988def5afa5 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 @@ -418,40 +418,11 @@ case class Cosh(child: Expression) extends UnaryMathExpression(math.cosh, "COSH" since = "3.0.0", group = "math_funcs") case class Acosh(child: Expression) - extends UnaryMathExpression((x: Double) => { - // fdlibm e_acosh.c algorithm - if (x < 1.0) { - Double.NaN - } else if (x >= (1 << 28)) { - StrictMath.log(x) + StrictMath.log(2.0) - } else if (x == 1.0) { - 0.0 - } else if (x > 2.0) { - StrictMath.log(2.0 * x - 1.0 / (x + math.sqrt(x * x - 1.0))) - } else { - val t = x - 1.0 - StrictMath.log1p(t + math.sqrt(2.0 * t + t * t)) - } - }, "ACOSH") { + // fdlibm e_acosh.c algorithm, shared with codegen via ExpressionImplUtils.acosh. + extends UnaryMathExpression((x: Double) => ExpressionImplUtils.acosh(x), "ACOSH") { override def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = { - nullSafeCodeGen(ctx, ev, c => { - val sm = "java.lang.StrictMath" - val t = ctx.freshName("t") - s""" - |if ($c < 1.0) { - | ${ev.value} = java.lang.Double.NaN; - |} else if ($c >= ${1 << 28}.0) { - | ${ev.value} = $sm.log($c) + $sm.log(2.0); - |} else if ($c == 1.0) { - | ${ev.value} = 0.0; - |} else if ($c > 2.0) { - | ${ev.value} = $sm.log(2.0 * $c - 1.0 / ($c + java.lang.Math.sqrt($c * $c - 1.0))); - |} else { - | double $t = $c - 1.0; - | ${ev.value} = $sm.log1p($t + java.lang.Math.sqrt(2.0 * $t + $t * $t)); - |} - |""".stripMargin - }) + val utils = classOf[ExpressionImplUtils].getName + defineCodeGen(ctx, ev, c => s"$utils.acosh($c)") } override protected def withNewChildInternal(newChild: Expression): Acosh = copy(child = newChild) }