-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-31144][SQL]Wrap Error with QueryExecutionException to notify QueryExecutionListener #27907
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
Conversation
|
Thank you, @zsxwing . |
|
Test build #119769 has finished for PR 27907 at commit
|
|
Could you fix |
|
Test build #119770 has finished for PR 27907 at commit
|
dongjoon-hyun
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1, LGTM. Merged to master/3.0.
Thank you, @zsxwing !
…QueryExecutionListener ### What changes were proposed in this pull request? This PR manually reverts changes in #25292 and then wraps java.lang.Error with `QueryExecutionException` to notify `QueryExecutionListener` to send it to `QueryExecutionListener.onFailure` which only accepts `Exception`. The bug fix PR for 2.4 is #27904. It needs a separate PR because the touched codes were changed a lot. ### Why are the changes needed? Avoid API changes and fix a bug. ### Does this PR introduce any user-facing change? Yes. Reverting an API change happening in 3.0. QueryExecutionListener APIs will be the same as 2.4. ### How was this patch tested? The new added test. Closes #27907 from zsxwing/SPARK-31144. Authored-by: Shixiong Zhu <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]> (cherry picked from commit 1ddf44d) Signed-off-by: Dongjoon Hyun <[email protected]>
|
@dongjoon-hyun thanks! |
HyukjinKwon
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 LGTM!
…QueryExecutionListener ### What changes were proposed in this pull request? This PR manually reverts changes in apache#25292 and then wraps java.lang.Error with `QueryExecutionException` to notify `QueryExecutionListener` to send it to `QueryExecutionListener.onFailure` which only accepts `Exception`. The bug fix PR for 2.4 is apache#27904. It needs a separate PR because the touched codes were changed a lot. ### Why are the changes needed? Avoid API changes and fix a bug. ### Does this PR introduce any user-facing change? Yes. Reverting an API change happening in 3.0. QueryExecutionListener APIs will be the same as 2.4. ### How was this patch tested? The new added test. Closes apache#27907 from zsxwing/SPARK-31144. Authored-by: Shixiong Zhu <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]>
What changes were proposed in this pull request?
This PR manually reverts changes in #25292 and then wraps java.lang.Error with
QueryExecutionExceptionto notifyQueryExecutionListenerto send it toQueryExecutionListener.onFailurewhich only acceptsException.The bug fix PR for 2.4 is #27904. It needs a separate PR because the touched codes were changed a lot.
Why are the changes needed?
Avoid API changes and fix a bug.
Does this PR introduce any user-facing change?
Yes. Reverting an API change happening in 3.0. QueryExecutionListener APIs will be the same as 2.4.
How was this patch tested?
The new added test.