[SPARK-37478][SQL][TESTS][FOLLOWUP] Unify v1 and v2 DROP NAMESPACE error #35007
Conversation
|
cc @cloud-fan @huaxingao. FYI ! |
There was a problem hiding this comment.
Change the error message of v1 command.
There was a problem hiding this comment.
Move cannotDropNonemptyNamespaceError from QueryExecutionErrors to QueryCompilationErrors as SPARK-33539
There was a problem hiding this comment.
Does CASCADE work for the v1 command? If yes, we should also mention it in the v1 error message.
There was a problem hiding this comment.
Yes, it works. I'll mention it in v1 error message. Thank you, @cloud-fan!
There was a problem hiding this comment.
Alias for namespace: database for v1 and namespace for v2
There was a problem hiding this comment.
Try-catch the error from hive catalog and re-throw it with a different error message that matches v1, v2
|
Kubernetes integration test starting |
|
Kubernetes integration test status failure |
|
Test build #146548 has finished for PR 35007 at commit
|
|
Kubernetes integration test starting |
|
Kubernetes integration test status failure |
|
Test build #146556 has finished for PR 35007 at commit
|
|
Please take a look if you have time @cloud-fan, @huaxingao . Many thanks! |
There was a problem hiding this comment.
Seems we can remove the details parameter now.
There was a problem hiding this comment.
Sorry, I missed the call cannotDropNonemptyDatabaseError(db, "tables") and cannotDropNonemptyDatabaseError(db, "functions") in InMemoryCatalog file. So I think keep original error message and also mention use CASCADE.
There was a problem hiding this comment.
Actually, I think One or more $details exist is a repeat of non-empty database, and it's OK to remove it, to keep consistent with v2.
dchvn
force-pushed
the
unify_dropnamespace_error
branch
from
2fa0bc8 to
941207d
Compare
| case NonFatal(exception) => | ||
| if (exception.getClass.getName.equals("org.apache.hadoop.hive.ql.metadata.HiveException") | ||
| && exception.getMessage.contains(s"Database $db is not empty.")) { | ||
| throw new AnalysisException(s"Cannot drop a non-empty database: $db.") |
There was a problem hiding this comment.
shall we use cannotDropNonemptyDatabaseError?
|
thanks, merging to master! |
3 participants
What changes were proposed in this pull request?
According to #cmt, unify the error of v1, v2 and hive external catalog in DROP NAMESPACE tests.
Why are the changes needed?
Currently, v1 and hive external catalog command throw
AnalysisException, while v2 command throwsSparkException. The error messages of v1 and hive catalog are also completely different from v2.So this PR is for unifying those errors to
AnalysisException. The error message will have one difference between v1/hive and v2:Cannot drop a non-empty database: ...vsCannot drop a non-empty namespace: ...Does this PR introduce any user-facing change?
No
How was this patch tested?
v1/v2 and Hive v1 DropNamespaceSuite:
$ build/sbt -Phive-2.3 -Phive-thriftserver "test:testOnly *DropNamespaceSuite"