ashniku commented on code in PR #6412:
URL: https://github.com/apache/hive/pull/6412#discussion_r3593830047
##########
jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java:
##########
@@ -398,20 +401,83 @@ private TGetOperationStatusResp waitForResultSetStatus()
throws SQLException {
return statusResp;
}
+ /**
+ * Returns the timeout message for a {@code TIMEDOUT_STATE} response. The
server is authoritative
+ * when the SQL state is {@code HYT00} ("timeout expired") and the message
is usable: it reflects
+ * the effective operation timeout (e.g. the minimum of session
+ * {@code hive.query.timeout.seconds} and {@link #setQueryTimeout(int)}).
Otherwise (e.g. an older
+ * server, or a placeholder {@code after 0 seconds} message) falls back to
the per-statement
+ * {@link #setQueryTimeout(int)}.
+ */
+ private String sqlTimeoutMessageForTimedOutState(String serverMessage,
String sqlState) {
+ if ("HYT00".equals(sqlState) &&
isUsableServerTimeoutMessage(serverMessage)) {
+ return serverMessage;
+ }
+ if (queryTimeout > 0) {
+ return "Query timed out after " + queryTimeout + " seconds";
+ }
+ return "Query timed out";
+ }
+
+ /**
+ * @return whether {@code serverMessage} is safe to pass through as the JDBC
timeout text
+ */
+ static boolean isUsableServerTimeoutMessage(String serverMessage) {
Review Comment:
@deniskuzZ
Thanks for the review — addressed all feedback:
1. Removed the "after 0 seconds" filter and isUsableServerTimeoutMessage().
Agreed it was a client-side hack; the bug was HiveStatement always using local
queryTimeout (default 0) for the JDBC message when only
hive.query.timeout.seconds was set via SET/URL.
2. Server is authoritative: SQLOperation sets HYT00 with the effective
timeout seconds before transitioning to TIMEDOUT. The client passes through any
non-blank HYT00 server message.
3. Client fallback only: server HYT00 → setQueryTimeout(N) → generic "Query
timed out". No client-side session timeout tracking or URL parsing.
4. Guard centralized in Operation.setOperationException() (don’t overwrite
terminal outcomes except ERROR); timeout handler skips if already terminal.
Integration tests cover JDBC URL, SET hive.query.timeout.seconds,
setQueryTimeout(), and message persistence across statements. Latest CI run is
green.
Could you please take another look when you have a chance
--
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]
