[SPARK-20459][SQL] JdbcUtils throws IllegalStateException: Cause already initialized after getting SQLException #17800
Conversation
|
Test build #76265 has finished for PR 17800 at commit
|
| try { | ||
| e.initCause(cause) | ||
| } catch { | ||
| // cause may have been explicitly initialized to null, in which case this |
There was a problem hiding this comment.
Based on my understanding, IllegalStateException is not caused by the value of cause.
@throws IllegalStateException if this throwable was
created with {@link #Throwable(Throwable)} or
{@link #Throwable(String,Throwable)}, or this method has already
been called on this throwable.There was a problem hiding this comment.
No the issue is simply whether the cause has been initialized at all. If it has been initialized to null, then e.getCause returns null, but the current code takes that to mean the cause is uninitialized (because it would return null) in that case. If it were initialized to an actual exception it already would call addSuppressed instead.
There was a problem hiding this comment.
@gatorsmile oh are you suggesting the comment is a bit misleading? yeah I can fix that to be clearer about the scenario
There was a problem hiding this comment.
Yeah. Please update the comment. Thanks!
|
LGTM |
|
Test build #76354 has finished for PR 17800 at commit
|
|
Thanks! Merging to master/2.2 |
…ady initialized after getting SQLException ## What changes were proposed in this pull request? Avoid failing to initCause on JDBC exception with cause initialized to null ## How was this patch tested? Existing tests Author: Sean Owen <[email protected]> Closes #17800 from srowen/SPARK-20459. (cherry picked from commit af726cd) Signed-off-by: Xiao Li <[email protected]>
|
@gatorsmile do you think this should go back into 2.1? seems like a clean fix to a non-trivial problem. I don't particularly care either way; you may have more context about the problem. |
3 participants
What changes were proposed in this pull request?
Avoid failing to initCause on JDBC exception with cause initialized to null
How was this patch tested?
Existing tests