MaxGekk commented on code in PR #56899: URL: https://github.com/apache/spark/pull/56899#discussion_r3498063670
########## sql/core/src/main/scala/org/apache/spark/sql/jdbc/PostgresDialect.scala: ########## @@ -259,8 +259,22 @@ private case class PostgresDialect() } // See https://www.postgresql.org/docs/current/errcodes-appendix.html + // Class 42 is "Syntax Error or Access Rule Violation", so it bundles genuine syntax errors + // together with authorization failures. 42501 is the insufficient_privilege state (e.g. + // "permission denied for table ..."), an access rule violation, not a syntax error. Additionally + // guard by message: some Postgres-compatible engines surface permission errors under the generic + // 42000 state, and access-control failures ("permission denied", "access denied", "unauthorized") + // must never be wrapped as JDBC_EXTERNAL_ENGINE_SYNTAX_ERROR. override def isSyntaxErrorBestEffort(exception: SQLException): Boolean = { - Option(exception.getSQLState).exists(_.startsWith("42")) + lazy val isAccessControlError = Option(exception.getMessage) + .map(_.toLowerCase(Locale.ROOT)) + .exists { message => + message.contains("permission denied") || + message.contains("access denied") || + message.contains("unauthorized") + } + Option(exception.getSQLState).exists(s => s.startsWith("42") && s != "42501") && Review Comment: The base contract (JdbcDialect Scaladoc) is *"if this method returns true, the exception is guaranteed to be a syntax error."* This denylist (`class 42 minus 42501 minus access-control messages`) still returns `true` for non-syntax class-42 codes — e.g. `42P01` (undefined_table), `42703` (undefined_column) — so the contract is only partially restored. It's not purely cosmetic: in `resolveTable` the preceding `isObjectNotFoundException` (Postgres matches `42P01`/`3F000`/`42704`) masks `42P01`, but the query-execution path (`JDBCRDD` ~L373) has no not-found pre-check, so there a `42P01`/`42703` is still wrapped as `JDBC_EXTERNAL_ENGINE_SYNTAX_ERROR.DURING_QUERY_EXECUTION`. For a genuine "true ⟹ syntax" guarantee, consider allowlisting the real syntax states (e.g. `42601`, `42P02`) rather than denylisting from class 42. Non-blocking — this extends @urosstan-db's thread about class-42 codes beyond `42501`. -- 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]
