You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "LuciferYang (via GitHub)" <gi...@apache.org> on 2026/06/03 11:51:32 UTC
[PR] [SPARK-57245][SQL] Simplify FormatNumber codegen by extracting a static Java helper [spark]
LuciferYang opened a new pull request, #56299:
URL: https://github.com/apache/spark/pull/56299
### 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)
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-57245][SQL] Simplify FormatNumber codegen by extracting a static Java helper [spark]
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on PR #56299:
URL: https://github.com/apache/spark/pull/56299#issuecomment-4619268789
Merged into master/branch-4.x. Thanks @gengliangwang
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-57245][SQL] Simplify FormatNumber codegen by extracting a static Java helper [spark]
Posted by "gengliangwang (via GitHub)" <gi...@apache.org>.
gengliangwang commented on PR #56299:
URL: https://github.com/apache/spark/pull/56299#issuecomment-4619231786
@LuciferYang Thanks for working on this!
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-57245][SQL] Simplify FormatNumber codegen by extracting a static Java helper [spark]
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang closed pull request #56299: [SPARK-57245][SQL] Simplify FormatNumber codegen by extracting a static Java helper
URL: https://github.com/apache/spark/pull/56299
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-57245][SQL] Simplify FormatNumber codegen by extracting a static Java helper [spark]
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on PR #56299:
URL: https://github.com/apache/spark/pull/56299#issuecomment-4616996865
cc @gengliangwang
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-57245][SQL] Simplify FormatNumber codegen by extracting a static Java helper [spark]
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on PR #56299:
URL: https://github.com/apache/spark/pull/56299#issuecomment-4618461164
@gengliangwang This should comply with the preceding criteria.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org