Skip to content

Conversation

@MaxGekk
Copy link
Member

@MaxGekk MaxGekk commented Jan 16, 2024

What changes were proposed in this pull request?

In the PR, I propose to add new error class for unsupported method calls, and remove similar legacy error classes. New apply() method of SparkUnsupportedOperationException extracts method and class name from stack traces automatically, and places them to error class parameters.

Why are the changes needed?

To improve code maintenance by avoid boilerplate code (extract class and method names automatically), and to clean up error-classes.json.

Does this PR introduce any user-facing change?

Yes, it can if user's code depends on the error class or message format of SparkUnsupportedOperationException.

How was this patch tested?

By running new test:

$ build/sbt "test:testOnly *QueryCompilationErrorsSuite"

and the affected test suites:

$ build/sbt "core/testOnly *SparkThrowableSuite"
$ build/sbt "test:testOnly *ShuffleSpecSuite"

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

No.

private[spark] object SparkUnsupportedOperationException {
def apply(): SparkUnsupportedOperationException = {
val stackTrace = Thread.currentThread().getStackTrace
val messageParameters = if (stackTrace.length >= 4) {
Copy link
Member Author

Choose a reason for hiding this comment

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

The stack trace looks like:
Screenshot 2024-01-16 at 19 55 35

@MaxGekk MaxGekk changed the title [WIP][SQL] Add the error class UNSUPPORTED_CALL [WIP][SPARK-46739][SQL] Add the error class UNSUPPORTED_CALL Jan 17, 2024
@MaxGekk MaxGekk marked this pull request as ready for review January 17, 2024 01:52
@MaxGekk MaxGekk changed the title [WIP][SPARK-46739][SQL] Add the error class UNSUPPORTED_CALL [SPARK-46739][SQL] Add the error class UNSUPPORTED_CALL Jan 17, 2024
@MaxGekk MaxGekk requested a review from cloud-fan January 17, 2024 08:50
@MaxGekk
Copy link
Member Author

MaxGekk commented Jan 17, 2024

@cloud-fan @beliefer @LuciferYang @panbingkun Could you take a look at the PR, please.

@MaxGekk
Copy link
Member Author

MaxGekk commented Jan 17, 2024

Merging to master. Thank you, @cloud-fan for review.

@MaxGekk MaxGekk closed this in bccbdb7 Jan 17, 2024
@LuciferYang
Copy link
Contributor

late LGTM

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