You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@spark.apache.org by we...@apache.org on 2026/09/17 16:12:45 UTC

(spark) branch branch-4.x updated: [SPARK-59098][SQL] Format decimal arguments in format_string/printf as java.math.BigDecimal

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

cloud-fan 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 22304816188b [SPARK-59098][SQL] Format decimal arguments in format_string/printf as java.math.BigDecimal
22304816188b is described below

commit 22304816188bc4945b0483bb5794e5652032f217
Author: Joel Robin P <jo...@gmail.com>
AuthorDate: Thu Sep 17 16:12:03 2026 +0000

    [SPARK-59098][SQL] Format decimal arguments in format_string/printf as java.math.BigDecimal
    
    ### What changes were proposed in this pull request?
    
    `FormatString` passes Catalyst's internal values straight to `java.util.Formatter`, so a `DecimalType` argument arrives as `org.apache.spark.sql.types.Decimal`. `Formatter` dispatches on the runtime class of each argument, and its `%f`/`%e`/`%g` conversions accept only `float`, `Float`, `double`, `Double` and `java.math.BigDecimal` — so they reject Spark's wrapper. (`%a` is narrower still: it accepts only `float` and `double`, so it does not support decimals either before or after thi [...]
    
    This PR converts `Decimal` to `java.math.BigDecimal` on the way into the formatter, in both evaluation paths:
    
    - **Interpreted** (`eval`): a new `toFormatterArg` helper maps `Decimal` to `toJavaBigDecimal` and passes everything else through unchanged.
    - **Codegen** (`doGenCode`): decimals previously fell into the pass-through branch because `CodeGenerator.boxedType` and `CodeGenerator.javaType` are both `Decimal`, so no boxing was applied. A decimal branch now emits `.toJavaBigDecimal()`, preserving null handling. Because `inputTypes` admits any type, the branch is chosen from `CodeGenerator.javaClass(dataType)` rather than by matching `DecimalType`, in two steps. When that class is exactly `Decimal` — `DecimalType`, or a UDT stori [...]
    
    `java.math.BigDecimal` keeps the exact value, unlike a cast to `double`.
    
    ### Why are the changes needed?
    
    `format_string`/`printf` fail on any decimal argument with a floating-point conversion, which is a natural thing to write:
    
    ```
    spark-sql> SELECT printf('%.2f', 1.5);
    java.lang.IllegalArgumentException: f != org.apache.spark.sql.types.Decimal
    ```
    
    Decimal literals get `DecimalType`, so `%.2f` — the common way to format money — throws instead of formatting. The error surfaces an internal class name, giving no hint that a workaround is to cast to `double` (which would also lose precision).
    
    ### Does this PR introduce _any_ user-facing change?
    
    Yes, three ways.
    
    **1. Fixed — the `%f`, `%e` and `%g` conversions now work on decimals.** (`%a` is unaffected and still rejects them.) Compared to released versions (reproduced on 4.0.0) and to master:
    
    | Query | Before | After |
    |---|---|---|
    | `SELECT printf('%.2f', 1.5)` | `IllegalFormatConversionException` | `1.50` |
    | `SELECT printf('%,.2f', CAST(1234.5 AS DECIMAL(10,2)))` | `IllegalFormatConversionException` | `1,234.50` |
    | `SELECT printf('%e', 1.5)` | `IllegalFormatConversionException` | `1.500000e+00` |
    
    **2. Unchanged — `%s` and NULL.** `%s` output is identical, because `Decimal.toString` is already defined as `toBigDecimal.toString()` and `toBigDecimal`/`toJavaBigDecimal` wrap the same unscaled value and scale. A NULL decimal still formats as `null`. Non-decimal arguments are untouched: `%f` on an int still throws `f != java.lang.Integer`, and `%d` on a decimal still throws, now naming `java.math.BigDecimal`.
    
    **3. Behavior change to be aware of — `%h` on a decimal can print a different hash.** `%h` formats `Integer.toHexString(arg.hashCode())`. `Decimal.hashCode` delegates to `scala.math.BigDecimal.hashCode`, which is kept consistent with `Double` hashing, whereas `java.math.BigDecimal.hashCode` is based on the unscaled value and scale. For `CAST(1.5 AS DECIMAL(10,2))`:
    
    ```
    %h before: 3fc00000   (Float.floatToIntBits(1.5f), via Scala's unified primitive hashing)
    %h after:  122c       (31 * 150 + 2, from java.math.BigDecimal's unscaled value and scale)
    ```
    
    The two hashes coincide for some values — `CAST(0 AS DECIMAL(1, 0))` prints `0` either way — so this is not a change for every decimal. `%h` on a decimal seems very unlikely to be relied on, and its previous value was a side effect of the same bug this PR fixes, but it is a real change, so it is noted in `docs/sql-migration-guide.md` under "Upgrading from Spark SQL 4.3 to 4.4" and pinned by a `checkEvaluation` assertion.
    
    ### How was this patch tested?
    
    New unit test `FormatString with decimal arguments` in `StringExpressionsSuite`. It goes through `checkEvaluation`, which exercises both the interpreted and the codegen path, and covers:
    
    - `%f`, `%.2f`, `%,.2f`, `%e` and `%g` on decimals;
    - exactness — `%.20f` on `0.10000000000000000001` round-trips every digit, which a `double` conversion would round;
    - `%s` still producing `1.2E-9` for `Decimal(12, 18, 10)`;
    - the documented `%h` effect — `Decimal("1.50")` prints `122c`, which is `java.math.BigDecimal.hashCode` (scale sensitive: `1.5` would give `1d2`) and not `Decimal.hashCode`, so a regression that passed a `Decimal` for `%h` again would fail here;
    - a NULL decimal formatting as `null`;
    - the negative cases — `%f` on an int still throwing `IllegalFormatConversionException` with `f != java.lang.Integer`, and `%a` on a decimal still throwing with `a != java.math.BigDecimal`, so the unsupported conversion cannot be advertised by accident.
    
    A second test, `FormatString with a decimal-backed UDT argument`, covers the UDT unwrap: it formats a UDT whose `sqlType` is `DecimalType` with `%f`, `%s` and a NULL. Reverting the `javaClass` check makes it fail with `f != org.apache.spark.sql.types.Decimal` under codegen while the interpreted path still succeeds, which is the mismatch it guards against.
    
    A third test, `FormatString with an object-typed decimal argument`, covers the `ObjectType` form: it asserts `Literal.fromObject(Decimal("1.5")).dataType` is `ObjectType(classOf[Decimal])` and then formats it with `%f` and `%s`. It fails the same way if the branch matches `DecimalType` instead of the generated value's class.
    
    A fourth test, `FormatString with a decimal under a broader object type`, covers the `instanceof` step: a `BoundReference` declared `ObjectType(classOf[Object])` formats a row holding a `Decimal` with `%f` and `%s`, a row holding a string, and a null row. Removing the runtime check makes only the decimal rows fail, with `f != org.apache.spark.sql.types.Decimal` under codegen; the string and null rows guard against the check converting values it should leave alone.
    
    `StringExpressionsSuite` passes locally: 70 tests run, 70 succeeded. `CodeGenerationSuite`, `GenerateUnsafeProjectionSuite`, `LiteralExpressionSuite`, `DecimalExpressionSuite` and `ObjectExpressionsSuite` were run alongside it to check the predicate change does not affect other codegen paths: 192 tests run, 192 succeeded.
    
    The fix was also verified against released Spark 4.0.0 by compiling the v4.0.0 source with this change and loading it ahead of `spark-catalyst` on the classpath; `FormatString.eval` and `FormatString.doGenCode` are identical between v4.0.0 and master.
    
    ### Was this patch authored or co-authored using generative AI tooling?
    
    Generated-by: Claude Code
    
    Closes #58408 from joelrobin18/format-string-decimal-bigdecimal.
    
    Authored-by: Joel Robin P <jo...@gmail.com>
    Signed-off-by: Wenchen Fan <we...@databricks.com>
    (cherry picked from commit f699ef19776079be828da2bbae8c89415bf3c64a)
    Signed-off-by: Wenchen Fan <we...@databricks.com>
---
 docs/sql-migration-guide.md                        |  1 +
 .../catalyst/expressions/stringExpressions.scala   | 21 ++++++-
 .../expressions/StringExpressionsSuite.scala       | 72 ++++++++++++++++++++++
 3 files changed, 92 insertions(+), 2 deletions(-)

diff --git a/docs/sql-migration-guide.md b/docs/sql-migration-guide.md
index 2f908e149c33..7b0d73ce7a0d 100644
--- a/docs/sql-migration-guide.md
+++ b/docs/sql-migration-guide.md
@@ -29,6 +29,7 @@ license: |
 - Since Spark 4.4, the optimizer rule `SimplifyCaseConversionExpressions` no longer simplifies mixed case conversions `LOWER(UPPER(str))` to `LOWER(str)` or `UPPER(LOWER(str))` to `UPPER(str)`. This preserves standard Unicode case mapping semantics for non-ASCII characters such as Latin small letter dotless i (`ı` U+0131) and micro sign (`µ` U+00B5).
 - Since Spark 4.4, the Oracle JDBC datasource maps Oracle DATE and TIMESTAMP to TimestampNTZType unconditionally, ignoring the JDBC read option `preferTimestampNTZ` (DATE only when the driver default `oracle.jdbc.mapDateToTimestamp=true` surfaces it as TIMESTAMP; otherwise DATE stays DateType). In 4.3 and earlier these were read as TimestampType, or as TimestampNTZType only when `preferTimestampNTZ=true`. This faithfully represents these zoneless Oracle types. Unaffected: Oracle DATE rea [...]
 - Since Spark 4.4, a nondeterministic common expression that a conditional branch reads more than once is evaluated once, and every read sees that one value. `BETWEEN` and `NULLIF` read their input twice, so an expression such as `CASE WHEN c THEN monotonically_increasing_id() BETWEEN 3 AND 5 END` previously gave each read its own value and could return a different result. Queries whose results depended on the earlier behaviour change, and an optimized or physical plan may now show a `wi [...]
+- Since Spark 4.4, `format_string` and `printf` pass a decimal argument to `java.util.Formatter` as a `java.math.BigDecimal`, so the floating-point conversions `%f`, `%e`, and `%g` format decimals instead of failing with `IllegalFormatConversionException`. `%a` still rejects a decimal, because `Formatter` supports it only for `float` and `double`. As a side effect, `%h` on a decimal now hashes the `java.math.BigDecimal` (unscaled value and scale) rather than Spark's internal `Decimal` (w [...]
 - Since Spark 4.4, for storage-partitioned joins, `spark.sql.requireAllClusterKeysForCoPartition` requires every join key to be covered by some partition key instead of matching the partition keys positionally. As a result, a join-key column partitioned by more than one transform no longer prevents shuffle elimination, and `spark.sql.sources.v2.bucketing.allowKeysSubsetOfPartitionKeys.enabled` no longer additionally requires `spark.sql.requireAllClusterKeysForCoPartition` to be `false` w [...]
 - Since Spark 4.4, the CSV `extension` option must be non-empty and contain only letters. In Spark 4.3 and earlier, due to a bug in the extension validation check, empty strings and three-character suffixes containing non-letters (for example `ab1`) were accepted. This affects both writes and reads of CSV tables whose persisted `OPTIONS` contain such values; update the option to a non-empty letters-only suffix to restore access.
 - Since Spark 4.4, when `array_repeat` or `array_insert` is asked to build an array larger than the maximum supported array length, generated code raises the same error as interpreted evaluation. `array_repeat` now fails with `COLLECTION_SIZE_LIMIT_EXCEEDED.PARAMETER` instead of the internal error `_LEGACY_ERROR_TEMP_2176`, and `array_insert` fails with `COLLECTION_SIZE_LIMIT_EXCEEDED.FUNCTION` instead of `COLLECTION_SIZE_LIMIT_EXCEEDED.PARAMETER`, which named a `count` parameter that `a [...]
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 763e5dbe8648..b918591a43ac 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
@@ -2407,21 +2407,38 @@ case class FormatString(children: Expression*) extends Expression with ImplicitC
       null
     } else {
       val formatter = new java.util.Formatter(Locale.US)
-      val arglist = children.tail.map(_.eval(input).asInstanceOf[AnyRef])
+      val arglist = children.tail.map(child => toFormatterArg(child.eval(input)))
       UTF8String.fromString(
         formatter.format(pattern.asInstanceOf[UTF8String].toString, arglist: _*).toString)
     }
   }
 
+  // java.util.Formatter dispatches on the runtime class of its argument and has no case for
+  // Catalyst's Decimal, so %f/%e/%g reject it. java.math.BigDecimal is accepted and stays exact.
+  // %a stays unsupported: Formatter accepts it only for float and double.
+  private def toFormatterArg(value: Any): AnyRef = value match {
+    case d: Decimal => d.toJavaBigDecimal
+    case other => other.asInstanceOf[AnyRef]
+  }
+
   override def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
     val pattern = children.head.genCode(ctx)
 
     val argListGen = children.tail.map(x => (x.dataType, x.genCode(ctx)))
     val argList = ctx.freshName("argLists")
     val numArgLists = argListGen.length
+    val decimalClass = classOf[Decimal].getName
     val argListCode = argListGen.zipWithIndex.map { case(v, index) =>
+      val argClass = CodeGenerator.javaClass(v._1)
       val value =
-        if (CodeGenerator.boxedType(v._1) != CodeGenerator.javaType(v._1)) {
+        if (argClass == classOf[Decimal]) {
+          // Keep in sync with toFormatterArg in the interpreted path above.
+          s"(${v._2.isNull}) ? null : ${v._2.value}.toJavaBigDecimal()"
+        } else if (argClass.isAssignableFrom(classOf[Decimal])) {
+          // Keep in sync with toFormatterArg in the interpreted path above.
+          s"(${v._2.isNull}) ? null : ((${v._2.value} instanceof $decimalClass) ? " +
+            s"(($decimalClass) ${v._2.value}).toJavaBigDecimal() : ${v._2.value})"
+        } else if (CodeGenerator.boxedType(v._1) != CodeGenerator.javaType(v._1)) {
           // Java primitives get boxed in order to allow null values.
           s"(${v._2.isNull}) ? (${CodeGenerator.boxedType(v._1)}) null : " +
             s"new ${CodeGenerator.boxedType(v._1)}(${v._2.value})"
diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/StringExpressionsSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/StringExpressionsSuite.scala
index bf2ddddd6778..4ff0a9788d99 100644
--- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/StringExpressionsSuite.scala
+++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/StringExpressionsSuite.scala
@@ -18,6 +18,7 @@
 package org.apache.spark.sql.catalyst.expressions
 
 import java.math.{BigDecimal => JavaBigDecimal}
+import java.util.IllegalFormatConversionException
 
 import org.apache.spark.{SPARK_DOC_ROOT, SparkFunSuite, SparkIllegalArgumentException, SparkRuntimeException}
 import org.apache.spark.sql.AnalysisException
@@ -1140,6 +1141,63 @@ class StringExpressionsSuite extends SparkFunSuite with ExpressionEvalHelper {
     GenerateUnsafeProjection.generate(FormatString(Literal("\"quote"), Literal("\"quote")) :: Nil)
   }
 
+  test("FormatString with decimal arguments") {
+    checkEvaluation(FormatString(Literal("%f"), Literal(Decimal("1.5"))), "1.500000")
+    checkEvaluation(FormatString(Literal("%.2f"), Literal(Decimal("1234.5"))), "1234.50")
+    checkEvaluation(FormatString(Literal("%,.2f"), Literal(Decimal("1234.5"))), "1,234.50")
+    checkEvaluation(FormatString(Literal("%e"), Literal(Decimal("1.5"))), "1.500000e+00")
+    checkEvaluation(FormatString(Literal("%g"), Literal(Decimal("1.5"))), "1.50000")
+
+    // %a is not supported for decimals: Formatter accepts it only for float and double.
+    checkExceptionInExpression[IllegalFormatConversionException](
+      FormatString(Literal("%a"), Literal(Decimal("1.5"))), "a != java.math.BigDecimal")
+
+    // Not routed through a double, so digits beyond double precision survive.
+    checkEvaluation(
+      FormatString(Literal("%.20f"), Literal(Decimal("0.10000000000000000001"))),
+      "0.10000000000000000001")
+
+    // %s is unchanged, because Decimal.toString already delegates to the same BigDecimal string.
+    checkEvaluation(FormatString(Literal("%s"), Literal(Decimal(12, 18, 10))), "1.2E-9")
+
+    // %h hashes whatever object reaches Formatter, so it pins the conversion down: this is
+    // java.math.BigDecimal.hashCode (31 * unscaledValue + scale, scale sensitive), not
+    // Decimal.hashCode, which follows Double hashing and would print 3fc00000 here.
+    checkEvaluation(FormatString(Literal("%h"), Literal(Decimal("1.50"))), "122c")
+
+    checkEvaluation(
+      FormatString(Literal("%f"), Literal.create(null, DecimalType(10, 2))), "null")
+
+    checkExceptionInExpression[IllegalFormatConversionException](
+      FormatString(Literal("%f"), Literal(1)), "f != java.lang.Integer")
+  }
+
+  test("FormatString with a decimal-backed UDT argument") {
+    // The generated row accessors use the UDT's underlying sqlType, so codegen sees a Decimal
+    // just like interpreted evaluation does. Both paths must convert it.
+    val udt = new DecimalWrapperUDT
+    val arg = Literal.create(udt.serialize(DecimalWrapper(Decimal("1.5"))), udt)
+    checkEvaluation(FormatString(Literal("%f"), arg), "1.500000")
+    checkEvaluation(FormatString(Literal("%s"), arg), "1.5")
+    checkEvaluation(FormatString(Literal("%f"), Literal.create(null, udt)), "null")
+  }
+
+  test("FormatString with an object-typed decimal argument") {
+    val arg = Literal.fromObject(Decimal("1.5"))
+    assert(arg.dataType === ObjectType(classOf[Decimal]))
+    checkEvaluation(FormatString(Literal("%f"), arg), "1.500000")
+    checkEvaluation(FormatString(Literal("%s"), arg), "1.5")
+  }
+
+  test("FormatString with a decimal under a broader object type") {
+    val objType = ObjectType(classOf[Object])
+    val ref = BoundReference(0, objType, nullable = true)
+    checkEvaluation(FormatString(Literal("%f"), ref), "1.500000", create_row(Decimal("1.5")))
+    checkEvaluation(FormatString(Literal("%s"), ref), "1.5", create_row(Decimal("1.5")))
+    checkEvaluation(FormatString(Literal("%s"), ref), "abc", create_row("abc"))
+    checkEvaluation(FormatString(Literal("%f"), ref), "null", create_row(null))
+  }
+
   test("SPARK-22603: FormatString should not generate codes beyond 64KB") {
     val N = 4500
     val args = (1 to N).map(i => Literal.create(i.toString, StringType))
@@ -2494,3 +2552,17 @@ class StringExpressionsSuite extends SparkFunSuite with ExpressionEvalHelper {
         "form" -> "'NFE'"))
   }
 }
+
+private case class DecimalWrapper(d: Decimal)
+
+private class DecimalWrapperUDT extends UserDefinedType[DecimalWrapper] {
+  override def sqlType: DataType = DecimalType(10, 2)
+
+  override def serialize(obj: DecimalWrapper): Any = obj.d
+
+  override def deserialize(datum: Any): DecimalWrapper = datum match {
+    case d: Decimal => DecimalWrapper(d)
+  }
+
+  override def userClass: Class[DecimalWrapper] = classOf[DecimalWrapper]
+}


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