cloud-fan commented on code in PR #57720:
URL: https://github.com/apache/spark/pull/57720#discussion_r3711445244


##########
sql/core/src/main/scala/org/apache/spark/sql/jdbc/MySQLDialect.scala:
##########
@@ -221,7 +221,35 @@ private case class MySQLDialect() extends JdbcDialect with 
SQLConfHelper with No
 
   // See 
https://dev.mysql.com/doc/mysql-errors/8.0/en/server-error-reference.html
   override def isSyntaxErrorBestEffort(exception: SQLException): Boolean = {
-    "42000".equals(exception.getSQLState)
+    "42000".equals(exception.getSQLState) &&
+    !isNonSyntaxErrorBestEffort(exception)

Review Comment:
   That is a fair concern; maintaining an exhaustive allowlist of roughly 80 
codes would not be attractive. I do not think it needs to be exhaustive, 
though. This API deliberately permits false negatives (it may fail to detect 
some syntax errors), while its contract forbids false positives (`true` must 
guarantee a syntax error). So I would start with a small allowlist of codes we 
are confident about, such as 1064, and add others only when needed with focused 
tests. Unknown `42000` codes would return `false`. That preserves the contract 
without requiring us to classify every MySQL error up front.



-- 
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]

Reply via email to