[SPARK-33074][SQL] Classify dialect exceptions in JDBC v2 Table Catalog #29952
Conversation
|
Kubernetes integration test starting |
|
Kubernetes integration test status success |
|
Test build #129439 has finished for PR 29952 at commit
|
|
@HyukjinKwon @cloud-fan @maropu @huaxingao Could you review this PR, please. |
|
I think of to continue with this after #29957 |
…eption # Conflicts: # sql/core/src/main/scala/org/apache/spark/sql/jdbc/JdbcDialects.scala # sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/v2/jdbc/JDBCTableCatalogSuite.scala
|
Kubernetes integration test starting |
|
Kubernetes integration test status success |
|
Test build #129502 has finished for PR 29952 at commit
|
|
Kubernetes integration test starting |
|
Kubernetes integration test status success |
|
@HyukjinKwon @cloud-fan @maropu @huaxingao I think this PR is ready for review. Please, have a look at it. |
| val writeOptions = new JdbcOptionsInWrite( | ||
| options.parameters + (JDBCOptions.JDBC_TABLE_NAME -> getTableName(ident))) | ||
| withConnection(JdbcUtils.tableExists(_, writeOptions)) | ||
| classifyException(s"Failed table existence check: $ident") { |
There was a problem hiding this comment.
shall we be consistent and always put classifyException inside withConnection?
There was a problem hiding this comment.
I don't think that mixing two conceptually different things will make them consistent.
| registerDialect(DerbyDialect) | ||
| registerDialect(OracleDialect) | ||
| registerDialect(TeradataDialect) | ||
| registerDialect(H2Dialect) |
There was a problem hiding this comment.
since we already have a testH2Dialect, how about we update testH2Dialect to implement classifyException, and use it in JDBCTableCatalogSuite? Then we don't need to have an official H2Dialect.
There was a problem hiding this comment.
If you look at testH2Dialect, it has test specific settings. Why should we have the settings in JDBCTableCatalogSuite?
Then we don't need to have an official
H2Dialect.
What is the problem to have built-in H2Dialect?
There was a problem hiding this comment.
Probably, not related to this. If we don't want to support H2 officially as a dialect, why do we test it so broadly in Spark.?Maybe it makes sense to switch all internal Spark tests to Derby?
|
Test build #129513 has finished for PR 29952 at commit
|
|
thanks, merging to master! |
3 participants
What changes were proposed in this pull request?
JdbcDialectclass -classifyException(). It converts dialect specific exception to Spark'sAnalysisExceptionor its sub-classes.org.h2.jdbc.JdbcSQLExceptioninJDBCTableCatalogSuitebyAnalysisException.H2DialectWhy are the changes needed?
Currently JDBC v2 Table Catalog implementation throws dialect specific exception and ignores exceptions defined in the
TableCataloginterface. This PR adds new method for converting dialect specific exception, and assumes that follow up PRs will implementclassifyException().Does this PR introduce any user-facing change?
Yes.
How was this patch tested?
By running existing test suites
JDBCTableCatalogSuiteandJDBCV2Suite.