gengliangwang opened a new pull request, #57143:
URL: https://github.com/apache/spark/pull/57143
### What changes were proposed in this pull request?
`Ceil` and `Floor` (ANSI mode, `DOUBLE -> LONG`) check for overflow before
casting the rounded
double to long. That check is currently implemented twice, in a private
`CeilFloor` helper object in
`mathExpressions.scala`:
- `CeilFloor.doubleToLong` for the interpreted path, and
- `CeilFloor.doubleToLongCode`, which re-emits the bound check as ~9 lines
of Java into every
`Ceil`/`Floor` codegen stage.
This PR replaces both with a single shared `MathUtils.doubleToLong(value,
context)` utility that the
interpreted and generated code both call, and deletes the `CeilFloor`
object. The generated code
collapses from the inline block:
```java
double roundedValue = java.lang.Math.ceil(input);
if (!java.lang.Double.isNaN(roundedValue) &&
(roundedValue < (double) java.lang.Long.MIN_VALUE ||
roundedValue >= (double) java.lang.Long.MAX_VALUE)) {
throw QueryExecutionErrors.arithmeticOverflowError("long overflow", "",
errCtx);
}
result = (long) roundedValue;
```
to a single call:
```java
result = org.apache.spark.sql.catalyst.util.MathUtils.doubleToLong(
java.lang.Math.ceil(input), errCtx);
```
`MathUtils` is the natural home: it already hosts the other
arithmetic-overflow helpers
(`addExact`, `negateExact`, `withOverflow`) and follows the same "shared by
the eval and codegen
paths so the two never diverge" convention already used for `pmod`. Placing
`doubleToLong` there also
means one fewer copy of the bound check emitted per stage (part of the
broader effort to shrink
whole-stage-codegen output, SPARK-56908).
### Why are the changes needed?
The overflow check is type-independent bookkeeping that does not need to be
re-emitted into each
generated stage. Consolidating it into `MathUtils`:
- removes the duplicated logic (the `CeilFloor.doubleToLong` eval copy and
the
`doubleToLongCode` string builder collapse into one method), so the two
paths cannot drift, and
- shrinks the generated code for every ANSI `CEIL`/`FLOOR` over a `DOUBLE`
from the multi-line
bound check to a single call.
### Does this PR introduce _any_ user-facing change?
No. The check, the error (`ARITHMETIC_OVERFLOW`), and the query context are
identical; `defineCodeGen`
applies the same null-safety wrapper the previous `nullSafeCodeGen` did.
This is a pure
code-organization change.
### How was this patch tested?
Existing tests, run with whole-stage codegen both on and off:
- `MathExpressionsSuite` (`ceil` and `floor`, including the ANSI overflow
cases and
`checkConsistencyBetweenInterpretedAndCodegenAllowingException` for
`DoubleType`), and
- regenerated `postgreSQL/float8.sql.out` (unchanged, confirming the
behavior is preserved).
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code (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: [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]
