j1wonpark commented on code in PR #57009:
URL: https://github.com/apache/spark/pull/57009#discussion_r3695747364
##########
sql/connect/client/jdbc/src/main/scala/org/apache/spark/sql/connect/client/jdbc/util/JdbcErrorUtils.scala:
##########
@@ -53,4 +59,65 @@ private[jdbc] object JdbcErrorUtils {
case _ =>
throw new IllegalArgumentException(s"Invalid fetch direction:
$direction")
}
+
+ // SQLState class 08 is "connection exception"; HYT00 is the conventional
+ // (ODBC-derived) state for an elapsed timeout.
+ private val CONNECTION_DOES_NOT_EXIST = "08003"
+ private val CONNECTION_FAILURE = "08006"
+ private val TIMEOUT_EXPIRED = "HYT00"
+
+ private val SESSION_GONE_CONDITION_PREFIX = "INVALID_HANDLE.SESSION_"
+
+ /**
+ * Maps the unchecked exceptions raised by the Spark Connect client (a
+ * [[SparkThrowable]] once GrpcExceptionConverter has converted the gRPC
error) to
+ * the [[SQLException]] a JDBC method is required to throw:
+ *
+ * - `INVALID_HANDLE.SESSION_*` means the server-side session is gone (e.g.
it
+ * timed out); the connection is unusable and retrying on it is
pointless, so it
+ * maps to a [[SQLNonTransientConnectionException]] with SQLState 08003
+ * ("connection does not exist"). The operation-level subconditions
+ * (`OPERATION_*`, `FORMAT`) concern a single operation on a healthy
session
+ * and are deliberately not treated as connection errors.
+ * - a gRPC UNAVAILABLE (e.g. a server restart or a network blip) maps to a
+ * [[SQLTransientConnectionException]] with SQLState 08006 ("connection
+ * failure"), since a fresh connection can succeed.
+ * - a gRPC DEADLINE_EXCEEDED means the RPC deadline elapsed, which a slow
query
+ * fires on a perfectly healthy connection, so it maps to a
+ * [[SQLTimeoutException]] rather than a connection error.
+ * - any other error keeps the server-provided SQLState when one is
available.
+ *
+ * The gRPC status is read from the [[StatusRuntimeException]] that
+ * GrpcExceptionConverter preserves in the cause chain, never from message
text,
+ * so a server-side error merely quoting a gRPC exception cannot be mistaken
for
+ * a transport failure. SQLState class 08 is how connection pools and BI
tools
+ * detect a dead connection and reconnect.
+ */
Review Comment:
Thanks for raising this. I looked into whether we could assign `08003`
directly in `error-conditions.json`.
The current error framework cannot assign a SQLSTATE to an individual
subclass. `ErrorSubInfo` has no `sqlState` field, and
`ErrorClassesJsonReader.getSqlState` resolves only the top-level condition.
Therefore, assigning `08003` only to `INVALID_HANDLE.SESSION_*` while
preserving the existing condition names would require a broader extension to
the common error framework.
Moving the session subclasses to a new top-level condition would avoid that
limitation, but it would be a compatibility-breaking rename. Existing Scala and
Python Connect clients match identifiers such as
`INVALID_HANDLE.SESSION_NOT_FOUND` to retry `ExecutePlan` transparently, and
`ResponseValidator` matches `INVALID_HANDLE.SESSION_CHANGED` to invalidate the
session. Updating the clients in this repository would not protect already
released clients communicating with a newer server.
For those reasons, would it make sense to keep the JDBC-layer mapping in
this PR and consider subclass-level SQLSTATE support as a follow-up? When an
`INVALID_HANDLE.SESSION_*` condition propagates to JDBC, `08003` is more
specific than the inherited `HY000`. The JDBC adapter would still be needed to
select the appropriate `SQLException` subtype and handle transport-level
`UNAVAILABLE` and `DEADLINE_EXCEEDED` errors.
--
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]
