Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SPARK-47045][SQL] Replace IllegalArgumentException by SparkIllegalArgumentException in sql/api #45098

Closed

Conversation

MaxGekk
Copy link
Member

@MaxGekk MaxGekk commented Feb 14, 2024

What changes were proposed in this pull request?

In the PR, I propose to replace all IllegalArgumentException by SparkIllegalArgumentException in sql/api code base, and introduce new legacy error classes with the _LEGACY_ERROR_TEMP_ prefix.

Why are the changes needed?

To unify Spark SQL exception, and port Java exceptions on Spark exceptions with error classes.

Does this PR introduce any user-facing change?

Yes, it can if user's code assumes some particular format of IllegalArgumentException messages.

How was this patch tested?

By running existing test suites like:

$ build/sbt "core/testOnly *SparkThrowableSuite"

Was this patch authored or co-authored using generative AI tooling?

No.

@github-actions github-actions bot added the R label Feb 14, 2024
@MaxGekk MaxGekk changed the title [WIP][SQL] Replace IllegalArgumentException by SparkIllegalArgumentException in sql/api [SPARK-47045][SQL] Replace IllegalArgumentException by SparkIllegalArgumentException in sql/api Feb 14, 2024
@MaxGekk MaxGekk marked this pull request as ready for review February 14, 2024 16:44
@MaxGekk
Copy link
Member Author

MaxGekk commented Feb 14, 2024

@HyukjinKwon @LuciferYang @panbingkun @srielau @hvanhovell @beliefer Could you review this PR, please.

},
"_LEGACY_ERROR_TEMP_3257" : {
"message" : [
"All week-based patterns are unsupported since Spark 3.0, detected: <c>, Please use the SQL function EXTRACT instead"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: punctuation in detected: <c>, Please could be improved, feel free to ignore though :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I preserved punctuation of the original error intentionally:
https://github.com/apache/spark/pull/45098/files#diff-1ada8897c412e27c3f73c8f5449f62f1fdc805b979cc7cfa35fcf8ad031529bbL313
to don't break existing tests. We should improve errors while assigning proper error class names, and writing tests for the errors.

@MaxGekk
Copy link
Member Author

MaxGekk commented Feb 15, 2024

Merging to master. Thank you, @srielau @xinrong-meng for review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants