[SPARK-33872][SQL] Throw more specific exceptions when relations/namespaces are not found #30878
Conversation
| case write: V2WriteCommand if write.table.isInstanceOf[UnresolvedRelation] => | ||
| val tblName = write.table.asInstanceOf[UnresolvedRelation].multipartIdentifier | ||
| write.table.failAnalysis(s"Table or view not found: ${tblName.quoted}") | ||
| throw new NoSuchTableException(s"Table or view not found: ${tblName.quoted}") |
There was a problem hiding this comment.
Do we want to throw this exception at a specific node, or is it not needed?
/** Fails the analysis at the point where a specific tree node was parsed. */
def failAnalysis(msg: String): Nothing = {
throw new AnalysisException(msg, t.origin.line, t.origin.startPosition)
}
|
@cloud-fan / @MaxGekk Do we also want to update the tests to catch more specific exceptions? There are many places, so I wanted to check with you first. Thanks! |
imback82
changed the title
|
Kubernetes integration test starting |
|
Test build #133182 has finished for PR 30878 at commit
|
|
Kubernetes integration test status failure |
|
|
||
| case u: UnresolvedNamespace => | ||
| u.failAnalysis(s"Namespace not found: ${u.multipartIdentifier.quoted}") | ||
| throw new NoSuchNamespaceException(s"Namespace not found: ${u.multipartIdentifier.quoted}") |
There was a problem hiding this comment.
One benefit of u.failAnalysis is that it can report the error position in SQL string. If we can't keep this feature with the new exception, I'd suggest we don't change the exception type.
There was a problem hiding this comment.
Should we close this PR or update NoSuchNamespaceException and NoSuchTableException to handle positions?
Btw, it looks like there is a bug in keeping the position. I will fix it separately.
There was a problem hiding this comment.
... update NoSuchNamespaceException and NoSuchTableException to handle positions?
If it's easy to do, let's go for it. Otherwise, we can close this PR as exception type seems not a big deal to me.
|
We don't need to update the tests. The concrete exception classes all extend |
|
Kubernetes integration test starting |
|
Kubernetes integration test status success |
|
We're closing this PR because it hasn't been updated in a while. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable. |
3 participants
What changes were proposed in this pull request?
This is to address a concerned raised in #30862 (comment)
Currently, when relations/namespaces are not found, a general
AnalysisExceptionis thrown inCheckAnalysis. This can be improved by throwing more specific (already existing) exceptions such asNoSuchNamespaceExceptionandNoSuchTableException.Why are the changes needed?
To use more specific exceptions.
Does this PR introduce any user-facing change?
Yes, a more specific exception will be thrown. Since these specific exceptions derive the
AnalysisException, no change is required if the client code already catchingAnalysisException.How was this patch tested?
Existing tests