joelrobin18 opened a new pull request, #58408:
URL: https://github.com/apache/spark/pull/58408
### 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`/`%a` conversions accept only
`float`, `Float`, `double`, `Double` and `java.math.BigDecimal` — so they
reject Spark's wrapper.
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 `DecimalType` branch now emits
`.toJavaBigDecimal()`, preserving null handling.
`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 — floating-point conversions now work on decimals.** 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 prints 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)
```
`%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.
### 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` and `%e` 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)`;
- a NULL decimal formatting as `null`;
- the negative case — `%f` on an int still throwing
`IllegalFormatConversionException` with `f != java.lang.Integer`.
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
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
