You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@spark.apache.org by ya...@apache.org on 2026/06/04 05:27:05 UTC

(spark) branch branch-4.x updated: [SPARK-57245][SQL] Simplify FormatNumber codegen by extracting a static Java helper

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

LuciferYang 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 49b521ba3728 [SPARK-57245][SQL] Simplify FormatNumber codegen by extracting a static Java helper
49b521ba3728 is described below

commit 49b521ba37280b8cf9e50e80a122fa8e7eecc225
Author: YangJie <ya...@baidu.com>
AuthorDate: Thu Jun 4 13:26:32 2026 +0800

    [SPARK-57245][SQL] Simplify FormatNumber codegen by extracting a static Java helper
    
    ### What changes were proposed in this pull request?
    
    This is a sub-task of [SPARK-56908](https://issues.apache.org/jira/browse/SPARK-56908) (reduce generated Java size in whole-stage codegen).
    
    For an integer `d`, `FormatNumber.doGenCode` inlined the number-pattern build (the default grouping pattern followed by `d` decimal places, applied to a `DecimalFormat`), an exact transliteration of the same block in `nullSafeEval`. The codegen also threaded a `pattern` `StringBuilder` through mutable state:
    
    ```java
    if (d >= 0) {
      pattern.delete(0, pattern.length());
      if (d != lastDValue) {
        pattern.append("#,###,###,###,###,###,##0");
        if (d > 0) {
          pattern.append(".");
          for (int i = 0; i < d; i++) {
            pattern.append("0");
          }
        }
        lastDValue = d;
        numberFormat.applyLocalizedPattern(pattern.toString());
      }
      ...
    }
    ```
    
    This PR moves the pattern build into a static helper `ExpressionImplUtils.applyNumberFormatScale(DecimalFormat, String defaultFormat, int scale)` that both `eval` and `doGenCode` call, mirroring the `ascii` helper added in SPARK-57208. The integer-`d` codegen becomes:
    
    ```java
    if (d >= 0) {
      if (d != lastDValue) {
        lastDValue = d;
        ExpressionImplUtils.applyNumberFormatScale(numberFormat, "#,###,###,###,###,###,##0", d);
      }
      ...
    }
    ```
    
    This removes the duplicated block, drops the `pattern` `StringBuilder` (an instance field in `eval` and a mutable-state slot in codegen), and removes a dead `pattern.delete(...)` that the string-pattern branch never read. The string-pattern path and the per-`d` caching (only rebuild when `d` changes) are unchanged.
    
    ### Why are the changes needed?
    
    To reduce the size of the generated Java in whole-stage codegen, as tracked by SPARK-56908. The integer-`d` branch shrinks to a single helper call, and the generated class drops one mutable-state field (the `pattern` `StringBuilder`). The same block is also deduplicated from `eval`.
    
    ### Does this PR introduce _any_ user-facing change?
    
    No. This is a codegen/refactor change; `eval`, `nullable`, `dataType`, and results are unchanged, so SQL output and golden files are unaffected.
    
    ### How was this patch tested?
    
    Existing tests, which exercise the changed path in both interpreted and codegen modes:
    - `StringExpressionsSuite` "format_number / FormatNumber" (catalyst) covers integer `d` with scale > 0, scale = 0, and scale < 0 (null) across all numeric input types, plus the string-pattern path and null cases.
    - `StringFunctionsSuite` "number format function" (sql/core).
    
    ### Was this patch authored or co-authored using generative AI tooling?
    
    Generated-by: Claude Code (Claude Opus 4.8)
    
    Closes #56299 from LuciferYang/formatnumber-codegen-helper.
    
    Authored-by: YangJie <ya...@baidu.com>
    Signed-off-by: yangjie01 <ya...@baidu.com>
    (cherry picked from commit d172a62127a6481eb5b9d07c608876307550966e)
    Signed-off-by: yangjie01 <ya...@baidu.com>
---
 .../catalyst/expressions/ExpressionImplUtils.java  | 19 ++++++++++
 .../catalyst/expressions/stringExpressions.scala   | 41 +++-------------------
 2 files changed, 23 insertions(+), 37 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 1a49cf6bb52b..d44922261674 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
@@ -22,6 +22,7 @@ import java.security.GeneralSecurityException;
 import java.security.SecureRandom;
 import java.security.spec.AlgorithmParameterSpec;
 import java.text.BreakIterator;
+import java.text.DecimalFormat;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Locale;
@@ -448,4 +449,22 @@ public class ExpressionImplUtils {
       return 0;
     }
   }
+
+  /**
+   * Builds the number pattern for the given scale (the default grouping pattern followed by
+   * {@code scale} decimal places) and applies it to the given {@link DecimalFormat}. Shared by the
+   * FormatNumber expression's eval and codegen paths so the generated Java is a single call and
+   * the pattern buffer does not need to be threaded through mutable state.
+   */
+  public static void applyNumberFormatScale(
+      DecimalFormat numberFormat, String defaultFormat, int scale) {
+    StringBuilder pattern = new StringBuilder(defaultFormat);
+    if (scale > 0) {
+      pattern.append('.');
+      for (int i = 0; i < scale; i++) {
+        pattern.append('0');
+      }
+    }
+    numberFormat.applyLocalizedPattern(pattern.toString());
+  }
 }
diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala
index bbc9341d8e6d..45ec0e6a8b30 100755
--- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala
+++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala
@@ -17,7 +17,6 @@
 
 package org.apache.spark.sql.catalyst.expressions
 
-import java.lang.{StringBuilder => JStringBuilder}
 import java.nio.{ByteBuffer, CharBuffer}
 import java.nio.charset.CharacterCodingException
 import java.text.{DecimalFormat, DecimalFormatSymbols}
@@ -3355,11 +3354,6 @@ case class FormatNumber(x: Expression, d: Expression)
   @transient
   private var lastDStringValue: Option[String] = None
 
-  // A cached DecimalFormat, for performance concern, we will change it
-  // only if the d value changed.
-  @transient
-  private lazy val pattern: JStringBuilder = new JStringBuilder()
-
   // SPARK-13515: US Locale configures the DecimalFormat object to use a dot ('.')
   // as a decimal separator.
   @transient
@@ -3377,31 +3371,15 @@ case class FormatNumber(x: Expression, d: Expression)
           case Some(last) if last == dValue =>
           // use the current pattern
           case _ =>
-            // construct a new DecimalFormat only if a new dValue
-            pattern.delete(0, pattern.length)
-            pattern.append(defaultFormat)
-
-            // decimal place
-            if (dValue > 0) {
-              pattern.append(".")
-
-              var i = 0
-              while (i < dValue) {
-                i += 1
-                pattern.append("0")
-              }
-            }
-
+            // construct a new pattern only if a new dValue
             lastDIntValue = Some(dValue)
-
-            numberFormat.applyLocalizedPattern(pattern.toString)
+            ExpressionImplUtils.applyNumberFormatScale(numberFormat, defaultFormat, dValue)
         }
       case _: StringType =>
         val dValue = dObject.asInstanceOf[UTF8String].toString
         lastDStringValue match {
           case Some(last) if last == dValue =>
           case _ =>
-            pattern.delete(0, pattern.length)
             lastDStringValue = Some(dValue)
             if (dValue.isEmpty) {
               numberFormat.applyLocalizedPattern(defaultFormat)
@@ -3433,7 +3411,6 @@ case class FormatNumber(x: Expression, d: Expression)
         }
       }
 
-      val sb = classOf[JStringBuilder].getName
       val df = classOf[DecimalFormat].getName
       val dfs = classOf[DecimalFormatSymbols].getName
       val l = classOf[Locale].getName
@@ -3445,24 +3422,14 @@ case class FormatNumber(x: Expression, d: Expression)
 
       right.dataType match {
         case IntegerType =>
-          val pattern = ctx.addMutableState(sb, "pattern", v => s"$v = new $sb();")
-          val i = ctx.freshName("i")
           val lastDValue =
             ctx.addMutableState(CodeGenerator.JAVA_INT, "lastDValue", v => s"$v = -100;")
+          val utils = classOf[ExpressionImplUtils].getName
           s"""
             if ($d >= 0) {
-              $pattern.delete(0, $pattern.length());
               if ($d != $lastDValue) {
-                $pattern.append("$defaultFormat");
-
-                if ($d > 0) {
-                  $pattern.append(".");
-                  for (int $i = 0; $i < $d; $i++) {
-                    $pattern.append("0");
-                  }
-                }
                 $lastDValue = $d;
-                $numberFormat.applyLocalizedPattern($pattern.toString());
+                $utils.applyNumberFormatScale($numberFormat, "$defaultFormat", $d);
               }
               ${ev.value} = UTF8String.fromString($numberFormat.format(${typeHelper(num)}));
             } else {


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@spark.apache.org
For additional commands, e-mail: commits-help@spark.apache.org