uros-b commented on code in PR #58621:
URL: https://github.com/apache/spark/pull/58621#discussion_r3972713650
##########
connector/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/PostgresIntegrationSuite.scala:
##########
@@ -382,6 +385,46 @@ class PostgresIntegrationSuite extends
SharedJDBCIntegrationSuite {
assert(sql("select c1, c3 from queryOption").collect().toSet ==
expectedResult)
}
+ test("SPARK-59336: do not classify missing table as a syntax error") {
+ val postgresError = intercept[SQLException] {
+ spark.read.format("jdbc")
+ .option("url", jdbcUrl)
+ .option("query", "SELECT * FROM table_that_does_not_exist")
+ .load()
+ }
+ assertResult("42P01")(postgresError.getSQLState)
+ }
+
+ 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'")
Review Comment:
CREATE USER sits outside tryWithSafeFinally. If that statement fails (role
already exists from a previous keepContainer run), cleanup never runs.
Put both CREATE and DROP inside the same tryWithSafeFinally, and DROP USER
IF EXISTS before CREATE.
##########
sql/core/src/test/scala/org/apache/spark/sql/jdbc/PostgresDialectSuite.scala:
##########
@@ -82,6 +82,14 @@ class PostgresDialectSuite extends SparkFunSuite with
MockitoSugar {
verify(conn).setAutoCommit(false)
}
+ test("SPARK-59336: classify only syntax error SQLSTATEs as syntax errors") {
+ assert(dialect.isSyntaxErrorBestEffort(new SQLException("syntax error",
"42000")))
+ assert(dialect.isSyntaxErrorBestEffort(new SQLException("syntax error",
"42601")))
+ assert(!dialect.isSyntaxErrorBestEffort(new SQLException("permission
denied", "42501")))
+ assert(!dialect.isSyntaxErrorBestEffort(new SQLException("undefined
table", "42P01")))
+ assert(!dialect.isSyntaxErrorBestEffort(new SQLException("error without
SQLSTATE")))
+ }
Review Comment:
Unit coverage is thin for the new contract. The suite checks 42501 and
42P01. Adding 42703 (undefined column) and 42883 (undefined function) would
lock the other common class-42 false positives the PR description calls out.
Cheap and they always run, unlike Docker tests.
--
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]
