pan3793 commented on code in PR #56341:
URL: https://github.com/apache/spark/pull/56341#discussion_r3361703087
##########
sql/connect/client/jdbc/src/main/scala/org/apache/spark/sql/connect/client/jdbc/SparkConnectStatement.scala:
##########
@@ -140,11 +147,17 @@ class SparkConnectStatement(conn: SparkConnectConnection)
extends Statement {
override def getQueryTimeout: Int = {
checkOpen()
- 0
+ queryTimeout
}
- override def setQueryTimeout(seconds: Int): Unit =
- throw new SQLFeatureNotSupportedException
+ // stored as a hint and echoed back; Spark Connect has no client-side timeout
+ override def setQueryTimeout(seconds: Int): Unit = {
+ checkOpen()
+ if (seconds < 0) {
+ throw new SQLException("Query timeout must be zero or a positive
integer.")
+ }
+ queryTimeout = seconds
Review Comment:
IIRC, DataGrip fails immediately when calling this method, because the JDBC
API javadoc does not say it may throw `SQLFeatureNotSupportedException`, so the
fix is indeed required.
For the implementation, I feel we should not store a dummy state, the API
javadoc does not say the `get*` method must return the same value accepted by
the `set*` the, so `getQueryTimeout` should always return 0 to reflect the real
state.
So the suggestion here is: keep the `checkOpen()` and `if (seconds < 0)`
check, silent drop the value, and keep existing `getQueryTimeout`
--
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]
