uros-b commented on code in PR #58621:
URL: https://github.com/apache/spark/pull/58621#discussion_r3968170247
##########
connector/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/PostgresIntegrationSuite.scala:
##########
@@ -382,6 +385,34 @@ class PostgresIntegrationSuite extends
SharedJDBCIntegrationSuite {
assert(sql("select c1, c3 from queryOption").collect().toSet ==
expectedResult)
}
+ test("SPARK-59336: do not classify insufficient privilege as a syntax
error") {
+ val restrictedUser = "restricted_user"
+ val restrictedPassword = "restricted_password"
+ val restrictedJdbcUrl =
s"jdbc:postgresql://$dockerIp:$externalPort/postgres"
+
+ Using.resource(getConnection()) { conn =>
+ conn.prepareStatement(s"CREATE USER $restrictedUser PASSWORD
'$restrictedPassword'")
+ .executeUpdate()
+ }
+
+ Utils.tryWithSafeFinally {
+ val postgresError = intercept[SQLException] {
+ spark.read.format("jdbc")
+ .option("url", restrictedJdbcUrl)
+ .option("dbtable", "bar")
+ .option("user", restrictedUser)
+ .option("password", restrictedPassword)
+ .load()
+ }
+ assertResult("42501")(postgresError.getSQLState)
+ assertResult("ERROR: permission denied for table
bar")(postgresError.getMessage)
Review Comment:
The exact message match is a bit brittle in the Docker test.
```
assertResult("ERROR: permission denied for table
bar")(postgresError.getMessage)
```
PSQLException.getMessage often appends Position: / Detail:, and pre-15
servers say relation instead of table. The suite already allows
POSTGRES_DOCKER_IMAGE_NAME to vary. Assert SQLState == "42501" and
getMessage.contains("permission denied"), same style as the existing
SPARK-47886 checks in this suite.
Also use DROP USER IF EXISTS in cleanup. If CREATE USER fails, the finally
DROP USER can hide the original error; if a connection is still held, DROP USER
can fail after a green assertion.
##########
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"))
Review Comment:
Same startsWith("42") pattern is still in H2, Derby, DB2, Teradata, and
Databricks dialects. This might be out of scope in this particular PR, but
perhaps we can file follow-up tickets?
--
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]
