cloud-fan commented on code in PR #57333:
URL: https://github.com/apache/spark/pull/57333#discussion_r3603809808
##########
sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/v2/jdbc/JDBCTableCatalogSuite.scala:
##########
@@ -228,6 +229,12 @@ class JDBCTableCatalogSuite extends SharedSparkSession {
}
}
+ test("DatabricksDialect recognizes syntax errors in messages") {
+ val exception = new SQLException("[parse_syntax_error] Syntax error at or
near 'SQL'", "07000")
Review Comment:
Non-blocking: the test asserts only the returns-true path. A complementary
case — a plain `SQLException` with neither a class-42 SQLState nor
`SYNTAX_ERROR` in its message returning `false` — would guard the best-effort
substring match against becoming too broad.
##########
sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/v2/jdbc/JDBCTableCatalogSuite.scala:
##########
@@ -228,6 +229,12 @@ class JDBCTableCatalogSuite extends SharedSparkSession {
}
}
+ test("DatabricksDialect recognizes syntax errors in messages") {
Review Comment:
Non-blocking: this is a pure dialect unit test — it constructs a
`SQLException`, calls `JdbcDialects.get(...).isSyntaxErrorBestEffort(...)`, and
asserts, using none of this suite's H2 catalog, `SharedSparkSession`, or
`beforeAll` table fixtures. The other `DatabricksDialect` unit tests (URL
handling, catalyst/JDBC type mapping) all live in `JDBCSuite.scala` (the
`SPARK-45139: DatabricksDialect ...` tests). Consider placing this one there
for consistency and to avoid paying the table-catalog fixture cost for a check
that doesn't need it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
