Skip to content

Conversation

@MaxGekk
Copy link
Member

@MaxGekk MaxGekk commented Nov 21, 2022

What changes were proposed in this pull request?

In the PR, I propose new error class FAILED_FUNCTION_CALL for errors that might occur while preparing functions class, for instance during creation of expression instances. Also, the PR propagates SparkThrowable to users in handling any exceptions coming from preparations of functions calls.

Why are the changes needed?

To improve user experience with Spark SQL, in particular, the PR makes errors more clear.

Does this PR introduce any user-facing change?

Yes, it affects the user-facing errors.

How was this patch tested?

By running the modified test suites:

$ build/sbt "core/testOnly *SparkThrowableSuite"
$ PYSPARK_PYTHON=python3 build/sbt "sql/testOnly org.apache.spark.sql.SQLQueryTestSuite"

@MaxGekk MaxGekk changed the title [WIP][SPARK-41217][SQL] Add the error class FAILED_FUNCTION_CALL [SPARK-41217][SQL] Add the error class FAILED_FUNCTION_CALL Nov 21, 2022
@MaxGekk MaxGekk requested a review from cloud-fan November 21, 2022 15:54
@MaxGekk MaxGekk marked this pull request as ready for review November 21, 2022 15:54
@MaxGekk
Copy link
Member Author

MaxGekk commented Nov 21, 2022

@panbingkun @LuciferYang @itholic @cloud-fan @srielau Could you review this PR, please.


def funcBuildError(funcName: String, cause: Exception): Throwable = {
cause.getCause match {
case st: SparkThrowable with Throwable => st
Copy link
Member Author

@MaxGekk MaxGekk Nov 21, 2022

Choose a reason for hiding this comment

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

If some Spark's exception happens during preparation of a function call, we just propagate it to users AS IS otherwise (something we didn't catch) we wrap it by AnalysisException(errorClass = FAILED_FUNCTION_CALL)

@MaxGekk
Copy link
Member Author

MaxGekk commented Nov 21, 2022

@panbingkun @LuciferYang Your PRs are related to this one. I would propose to merge this first of all to properly propagate AnalysisExceptions w/ error classes:

@cloud-fan We bypass SparkThrowable in other places already. In this PR, I propose to do the same instead of additionally wrap them and increase levels of nesting.

Copy link
Contributor

@LuciferYang LuciferYang left a comment

Choose a reason for hiding this comment

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

+1, LGTM

I think we can merge this as soon as possible

@panbingkun
Copy link
Contributor

+1, LGTM

@MaxGekk
Copy link
Member Author

MaxGekk commented Nov 22, 2022

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

@MaxGekk MaxGekk closed this in 40b7d29 Nov 22, 2022
@LuciferYang
Copy link
Contributor

OK, I will rebase my pr

@MaxGekk
Copy link
Member Author

MaxGekk commented Nov 22, 2022

@panbingkun Please, update your PRs.

SandishKumarHN pushed a commit to SandishKumarHN/spark that referenced this pull request Dec 12, 2022
### What changes were proposed in this pull request?
In the PR, I propose new error class `FAILED_FUNCTION_CALL` for errors that might occur while preparing functions class, for instance during creation of expression instances. Also, the PR propagates `SparkThrowable` to users in handling any exceptions coming from preparations of functions calls.

### Why are the changes needed?
To improve user experience with Spark SQL, in particular, the PR makes errors more clear.

### Does this PR introduce _any_ user-facing change?
Yes, it affects the user-facing errors.

### How was this patch tested?
By running the modified test suites:
```
$ build/sbt "core/testOnly *SparkThrowableSuite"
$ PYSPARK_PYTHON=python3 build/sbt "sql/testOnly org.apache.spark.sql.SQLQueryTestSuite"
```

Closes apache#38744 from MaxGekk/failed-builtin-func.

Authored-by: Max Gekk <[email protected]>
Signed-off-by: Max Gekk <[email protected]>
beliefer pushed a commit to beliefer/spark that referenced this pull request Dec 15, 2022
### What changes were proposed in this pull request?
In the PR, I propose new error class `FAILED_FUNCTION_CALL` for errors that might occur while preparing functions class, for instance during creation of expression instances. Also, the PR propagates `SparkThrowable` to users in handling any exceptions coming from preparations of functions calls.

### Why are the changes needed?
To improve user experience with Spark SQL, in particular, the PR makes errors more clear.

### Does this PR introduce _any_ user-facing change?
Yes, it affects the user-facing errors.

### How was this patch tested?
By running the modified test suites:
```
$ build/sbt "core/testOnly *SparkThrowableSuite"
$ PYSPARK_PYTHON=python3 build/sbt "sql/testOnly org.apache.spark.sql.SQLQueryTestSuite"
```

Closes apache#38744 from MaxGekk/failed-builtin-func.

Authored-by: Max Gekk <[email protected]>
Signed-off-by: Max Gekk <[email protected]>
beliefer pushed a commit to beliefer/spark that referenced this pull request Dec 18, 2022
### What changes were proposed in this pull request?
In the PR, I propose new error class `FAILED_FUNCTION_CALL` for errors that might occur while preparing functions class, for instance during creation of expression instances. Also, the PR propagates `SparkThrowable` to users in handling any exceptions coming from preparations of functions calls.

### Why are the changes needed?
To improve user experience with Spark SQL, in particular, the PR makes errors more clear.

### Does this PR introduce _any_ user-facing change?
Yes, it affects the user-facing errors.

### How was this patch tested?
By running the modified test suites:
```
$ build/sbt "core/testOnly *SparkThrowableSuite"
$ PYSPARK_PYTHON=python3 build/sbt "sql/testOnly org.apache.spark.sql.SQLQueryTestSuite"
```

Closes apache#38744 from MaxGekk/failed-builtin-func.

Authored-by: Max Gekk <[email protected]>
Signed-off-by: Max Gekk <[email protected]>
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.

4 participants