cloud-fan commented on code in PR #58621: URL: https://github.com/apache/spark/pull/58621#discussion_r3995048855
########## sql/core/src/main/scala/org/apache/spark/sql/jdbc/PostgresDialect.scala: ########## @@ -260,7 +260,10 @@ private case class PostgresDialect() // See https://www.postgresql.org/docs/current/errcodes-appendix.html override def isSyntaxErrorBestEffort(exception: SQLException): Boolean = { - Option(exception.getSQLState).exists(_.startsWith("42")) + exception.getSQLState match { + case "42000" | "42601" => true Review Comment: 42000 can indeed represent syntax, but the two mistakes are asymmetric here: if we return false, Spark preserves the original PostgreSQL SQLException; if we return true for an access-rule 42000, Spark changes the top-level error class to JDBC_EXTERNAL_ENGINE_SYNTAX_ERROR. Since this API explicitly permits missed syntax errors but guarantees that every true result is syntax, the false positive is worse. PostgreSQL also provides 42601 for syntax, so I still think we should match only 42601 and leave 42000 unwrapped unless we have another reliable discriminator. <!-- SPARK_DEV_REVIEW_REPLY {"feedback_id":"inline:3964791615","thread_id":"inline:3964791615","verdict_sha256":"36b1a36b7e1f42d6f72f87cb98de058bc572c88bf4bfa2e1baefbfc33cc86db7"} --> -- 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]
