[SPARK-55755][SQL][TESTS] Handle null ArithmeticException message on JDK 25#54553
[SPARK-55755][SQL][TESTS] Handle null ArithmeticException message on JDK 25#54553yaooqinn wants to merge 1 commit intoapache:masterfrom
Conversation
…JDK 25 On JDK 25, Math.multiplyExact/addExact may throw ArithmeticException without a message (null instead of "long overflow"). This causes NPE in tests that check the raw exception message. This PR: 1. Fixes MathUtils.withOverflow to provide a fallback message "Overflow" when the raw JDK exception message is null (both interpreted and codegen paths). 2. Makes test assertions null-safe for raw ArithmeticException message checks across DateExpressionsSuite, CatalystTypeConvertersSuite, DateTimeUtilsSuite, IntervalUtilsSuite, and TimestampFormatterSuite. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
BTW, did you split the scope disjointly from the following PR, @yaooqinn ? |
|
Oh, I didn't notice there's existing effort, just saw an error the master branch and proposed this fix. Shall I close this one? @dongjoon-hyun |
|
I'm not sure in this case because this one looks more concise. Does this make all tests pass? Could you talk with @pan3793 ? |
|
Let me merge this to fix some CI failures first, thank you @dongjoon-hyun |
|
Sounds good, @yaooqinn . Thanks. |
| throw ExecutionErrors.arithmeticOverflowError(e.getMessage, hint, context) | ||
| // On JDK 25+, Math.*Exact may throw ArithmeticException without a message | ||
| val message = if (e.getMessage != null) e.getMessage else "Overflow" | ||
| throw ExecutionErrors.arithmeticOverflowError(message, hint, context) |
There was a problem hiding this comment.
changing here causes duplication, why not do it in ExecutionErrors.arithmeticOverflowError like #54514?
in addition, better to use lower case "overflow" to match the original JDK message style
|
FYI, I have a draft PR #54091 which makes all GHA pipelines pass on JDK 25. You can check each commit to evaluate the remaining issues. |
|
@pan3793, my apologies, feel free to revert this one if you've gotten a better plan. |
|
Okay, reverted in 3f54ac5, JIRA is updated to "duplicate with SPARK-55714" |
3 participants
What changes were proposed in this pull request?
On JDK 25,
Math.multiplyExact/Math.addExactmay throwArithmeticExceptionwithout a message (null instead of"long overflow"). This causesNullPointerExceptionin tests that check the raw exception message via.getMessage.contains(...).This PR:
MathUtils.withOverflow: Provides a fallback message"Overflow"when the raw JDK exception message is null, preventing null propagation into Spark error conditions (both interpreted and codegen paths).ArithmeticExceptionmessage checks null-safe across:DateExpressionsSuite(line 1825)CatalystTypeConvertersSuite(line 305)DateTimeUtilsSuite(line 835)IntervalUtilsSuite(line 593)TimestampFormatterSuite(lines 451, 453)Why are the changes needed?
The JDK 25 scheduled CI build fails with:
See: https://github.com/apache/spark/actions/runs/22513372344/job/65226962993
Does this PR introduce any user-facing change?
No.
How was this patch tested?
Existing tests pass on JDK 17. The
MathUtils.withOverflowfix ensures non-null messages regardless of JDK version.Was this patch authored or co-authored using generative AI tooling?
Yes, co-authored with GitHub Copilot.