[SPARK-56911][SQL] Simplify Cast to decimal codegen under ANSI mode#55936
Open
gengliangwang wants to merge 2 commits into
Open
[SPARK-56911][SQL] Simplify Cast to decimal codegen under ANSI mode#55936gengliangwang wants to merge 2 commits into
gengliangwang wants to merge 2 commits into
Conversation
Member
Author
This was referenced May 17, 2026
0beac87 to
f699004
Compare
f699004 to
8f72067
Compare
Member
Author
|
Audited this PR for the same lessons surfaced by @viirya and @cloud-fan on #55938 (and applied to #55934 / #55939):
So no changes needed on this PR for that review. |
Extend `CastUtils.java` with helpers for `byte` and `short` ANSI cast
targets and use them from `Cast.scala`. Drops the byte/short-target
dispatch (and the now-unused `lowerAndUpperBound` Scala helper) added
in SPARK-56909 -- after this PR, all integral and fractional narrowing
ANSI casts share the same `CastUtils.<...>Exact` one-line codegen.
Helpers added:
* `shortToByteExact(short)`, `intToByteExact(int)`, `longToByteExact(long)`
* `intToShortExact(int)`, `longToShortExact(long)`
* `floatToByteExact(float)`, `doubleToByteExact(double)`
* `floatToShortExact(float)`, `doubleToShortExact(double)`
`Cast.scala` changes:
* `castIntegralTypeToIntegralTypeExactCode` / `castFractionToIntegralTypeCode`
no longer dispatch on target type -- the helper-name pattern
`${integralPrefix(from)}To${target.capitalize}Exact` covers all four
target types.
* Eval paths for `castToByte` and `castToShort` add ANSI cases for
`ShortType` / `IntegerType` / `LongType` / `FloatType` / `DoubleType`
source types that delegate to the new helpers; the existing
`exactNumeric.toInt(b) + bounds-check` fallback now only handles the
remaining `Decimal` source.
Part of SPARK-56908 (umbrella). The original byte/short ANSI cast bodies
were 5 lines each across 8 call sites; this PR collapses them to one
line per call site, matching the int/long target work from SPARK-56909.
No. The compiled behavior is identical; only the emitted Java source
text changes.
```
build/sbt "catalyst/testOnly *CastSuite *CastWithAnsiOnSuite \
*CastWithAnsiOffSuite *AnsiCastSuite *TryCastSuite \
*ExpressionClassIdentitySuite"
```
312/312 pass.
Generated-by: Cursor 1.x
Extend `CastUtils.java` with two helpers for decimal precision adjustment and use them from `Cast.changePrecision` (both the eval and codegen implementations). The new helpers mutate the input `Decimal` in place (matching the behavior of the existing inline codegen), so they're safe to call on the temporary produced by `Decimal.fromString(...)` / `Decimal.apply(...)` / decimal-arithmetic results. Helpers added: * `changePrecisionExact(Decimal, int, int, QueryContext)`: ANSI throw on overflow, preserves the per-call-site `QueryContext` so error messages keep their query-origin info. * `changePrecisionOrNull(Decimal, int, int)`: non-ANSI, returns `null` on overflow (no `QueryContext` needed). `Cast.scala` changes: * `changePrecision` eval method dispatches on `nullOnOverflow` and delegates to the appropriate helper. * `changePrecision` codegen method has three branches now: the existing `canNullSafeCast` fast path (unchanged), a `nullOnOverflow` branch (inline), and the ANSI throw branch which now emits a one-line `CastUtils.changePrecisionExact(...)` call instead of the 5-line `if/else` overflow block. Part of SPARK-56908 (umbrella). The ANSI throw branch of `Cast.changePrecision` is hit by every cast to decimal that may overflow (very common in TPC-DS, where `cast(int as decimal(7,2))` is widespread). Collapsing the 5-line inline body to one line shrinks the generated Java source for those plans. No. The compiled behavior is identical; only the emitted Java source text changes. ``` build/sbt "catalyst/testOnly *CastSuite *CastWithAnsiOnSuite \ *CastWithAnsiOffSuite *AnsiCastSuite *TryCastSuite *DecimalSuite \ *ExpressionClassIdentitySuite" ``` 337/337 pass. Generated-by: Cursor 1.x
8f72067 to
62ce0cd
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
Extend
CastUtils.javawith two helpers for decimal precision adjustment and use them fromCast.changePrecision(both eval and codegen). The new helpers mutate the inputDecimalin place (matching the in-place semantics of the existing inline codegen), so they're safe to call on the temporary produced byDecimal.fromString(...)/Decimal.apply(...)/ decimal-arithmetic results.Helpers added:
changePrecisionExact(Decimal, int, int, QueryContext): ANSI throw on overflow, preserves the per-call-siteQueryContextso error messages keep their query-origin info.changePrecisionOrNull(Decimal, int, int): non-ANSI, returnsnullon overflow (noQueryContextneeded).Cast.scalachanges:changePrecisioneval method dispatches onnullOnOverflowand delegates to the appropriate helper.changePrecisioncodegen method has three branches now: the existingcanNullSafeCastfast path (unchanged), anullOnOverflowbranch (inline), and the ANSI throw branch which now emits a one-lineCastUtils.changePrecisionExact(...)call instead of the 5-lineif/elseoverflow block.Why are the changes needed?
Part of SPARK-56908 (umbrella). The ANSI throw branch of
Cast.changePrecisionis hit by every cast to decimal that may overflow (very common in TPC-DS, wherecast(int as decimal(7,2))is widespread). Collapsing the 5-line inline body to one line shrinks the generated Java source for those plans.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?
332/332 pass.
Was this patch authored or co-authored using generative AI tooling?
Generated-by: Cursor 1.x