HIVE-28265: Fix JDBC timeout message for hive.query.timeout.seconds

itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcDriver2.java

@@ -42,6 +42,7 @@
 import org.apache.hive.service.cli.operation.ClassicTableTypeMapping.ClassicTableTypes;
 import org.apache.hive.service.cli.operation.HiveTableTypeMapping;
 import org.apache.hive.service.cli.operation.TableTypeMappingFactory.TableTypeMappings;
+import org.junit.After;
 import org.junit.AfterClass;
 import org.junit.BeforeClass;
 import org.junit.Rule;
@@ -131,6 +132,25 @@ public class TestJdbcDriver2 {
   @Rule public ExpectedException thrown = ExpectedException.none();
   @Rule public final TestName testName = new TestName();
 
+  /**
+   * {@code SET hive.query.timeout.seconds} applies to the whole HS2 session. Tests such as
+   * {@link #testQueryTimeoutMessageUsesHiveConf()} must not leave a short limit on the shared
+   * {@link #con}, or unrelated tests will see {@link SQLTimeoutException}.
+   */
+  @After
+  public void resetHiveSessionQueryTimeout() {
+    try {
+      if (con == null || con.isClosed()) {
+        return;
+      }
+      try (Statement st = con.createStatement()) {
+        st.execute("set hive.query.timeout.seconds=0s");
+      }
+    } catch (SQLException e) {
+      LOG.warn("Could not reset hive.query.timeout.seconds after {}", testName.getMethodName(), e);
+    }
+  }
+
   private static Connection getConnection(String prefix, String postfix) throws SQLException {
     Connection con1;
     String connString = "jdbc:hive2:///" + prefix + "?" + conf.getOverlayOptionsAsQueryString()
@@ -2662,6 +2682,10 @@ public void testQueryTimeout() throws Exception {
       fail("Expecting SQLTimeoutException");
     } catch (SQLTimeoutException e) {
       assertNotNull(e);
+      assertTrue("Message should reflect JDBC query timeout (1s): " + e.getMessage(),
+          e.getMessage().contains("1"));
+      assertFalse("Message should not claim 0 seconds: " + e.getMessage(),
+          e.getMessage().contains("after 0 seconds"));
       System.err.println(e.toString());
     } catch (SQLException e) {
       fail("Expecting SQLTimeoutException, but got SQLException: " + e);
@@ -2680,6 +2704,37 @@ public void testQueryTimeout() throws Exception {
     stmt.close();
   }
 
+  /**
+   * HIVE-28265: hive.query.timeout.seconds drives the server-side timer, but the JDBC client
+   * must not report "0 seconds" when Statement#setQueryTimeout was not used.
+   */
+  @Test
+  public void testQueryTimeoutMessageUsesHiveConf() throws Exception {
+    String udfName = SleepMsUDF.class.getName();
+    Statement stmt1 = con.createStatement();
+    stmt1.execute("create temporary function sleepMsUDF as '" + udfName + "'");
+    stmt1.close();
+
+    Statement stmt = con.createStatement();
+    stmt.execute("set hive.query.timeout.seconds=1s");
+
+    try {
+      stmt.executeQuery("select sleepMsUDF(t1.under_col, 5) as u0, t1.under_col as u1, "
+          + "t2.under_col as u2 from " + tableName + " t1 join " + tableName
+          + " t2 on t1.under_col = t2.under_col");
+      fail("Expecting SQLTimeoutException");
+    } catch (SQLTimeoutException e) {
+      assertNotNull(e);
+      assertTrue("Message should include session timeout (1s): " + e.getMessage(),
+          e.getMessage().contains("1"));
+      assertFalse("Message should not claim 0 seconds (HIVE-28265): " + e.getMessage(),
+          e.getMessage().contains("after 0 seconds"));
+    } catch (SQLException e) {
+      fail("Expecting SQLTimeoutException, but got SQLException: " + e);
+    }
+    stmt.close();
+  }
+
   /**
    * Test the non-null value of the Yarn ATS GUID.
    * We spawn 2 threads - one running the query and

jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java

@@ -155,6 +155,7 @@
 import org.apache.thrift.transport.TTransportException;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+import java.util.concurrent.atomic.AtomicLong;
 import java.util.function.Supplier;
 
 /**
@@ -163,6 +164,12 @@
  */
 public class HiveConnection implements java.sql.Connection {
   private static final Logger LOG = LoggerFactory.getLogger(HiveConnection.class);
+
+  /**
+   * Sentinel: no {@code SET hive.query.timeout.seconds} has been observed on this connection yet.
+   */
+  static final long SESSION_QUERY_TIMEOUT_NOT_TRACKED = -1L;
+  private final AtomicLong sessionQueryTimeoutSeconds = new AtomicLong(SESSION_QUERY_TIMEOUT_NOT_TRACKED);
   private String jdbcUriString;
   private String host;
   private int port;
@@ -190,6 +197,21 @@ public class HiveConnection implements java.sql.Connection {
 
   public TCLIService.Iface getClient() { return client; }
 
+  /**
+   * Records the effective {@code hive.query.timeout.seconds} (in seconds) after a successful
+   * {@code SET hive.query.timeout.seconds=...} on this connection. Used for JDBC timeout messages.
+   */
+  void recordSessionQueryTimeoutFromSet(long seconds) {
+    sessionQueryTimeoutSeconds.set(seconds);
+  }
+
+  /**
+   * @return seconds from the last client-tracked SET, or {@link #SESSION_QUERY_TIMEOUT_NOT_TRACKED} if none
+   */
+  long getSessionQueryTimeoutSecondsTracked() {
+    return sessionQueryTimeoutSeconds.get();
+  }
+
   /**
    * Get all direct HiveServer2 URLs from a ZooKeeper based HiveServer2 URL
    * @param zookeeperBasedHS2Url

jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java

@@ -19,6 +19,7 @@
 package org.apache.hive.jdbc;
 
 import org.apache.commons.lang3.StringUtils;
+import org.apache.hadoop.hive.conf.HiveConf;
 import org.apache.hadoop.hive.common.classification.InterfaceAudience.LimitedPrivate;
 import org.apache.hive.jdbc.logs.InPlaceUpdateStream;
 import org.apache.hive.service.cli.RowSet;
@@ -57,6 +58,9 @@
 import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
+import java.util.concurrent.TimeUnit;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 
 import static org.apache.hadoop.hive.ql.ErrorMsg.CLIENT_POLLING_OPSTATUS_INTERRUPTED;
 
@@ -70,6 +74,10 @@ public class HiveStatement implements java.sql.Statement {
 
   public static final String QUERY_CANCELLED_MESSAGE = "Query was cancelled.";
 
+  /** Last assignment wins if multiple appear (e.g. multi-line script). Uses find(), not full-string match. */
+  private static final Pattern SET_HIVE_QUERY_TIMEOUT_SECONDS = Pattern.compile(
+      "(?i)set\\s+hive\\.query\\.timeout\\.seconds\\s*=\\s*([^;\\n]+)");
+
   private final HiveConnection connection;
   private TCLIService.Iface client;
   private Optional<TOperationHandle> stmtHandle;
@@ -298,6 +306,7 @@ public void closeOnCompletion() throws SQLException {
   public boolean execute(String sql) throws SQLException {
     runAsyncOnServer(sql);
     TGetOperationStatusResp status = waitForOperationToComplete();
+    trackSessionQueryTimeoutIfSet(sql);
 
     // The query should be completed by now
     if (!status.isHasResultSet() && stmtHandle.isPresent() && !stmtHandle.get().isHasResultSet()) {
@@ -398,6 +407,70 @@ private TGetOperationStatusResp waitForResultSetStatus() throws SQLException {
     return statusResp;
   }
 
+  /**
+   * When {@code SET hive.query.timeout.seconds=...} succeeds, remember the effective value on the
+   * connection so {@code TIMEDOUT_STATE} can report it if the server omits {@code errorMessage}
+   * (HIVE-28265).
+   */
+  private void trackSessionQueryTimeoutIfSet(String sql) {
+    if (sql == null) {
+      return;
+    }
+    Matcher m = SET_HIVE_QUERY_TIMEOUT_SECONDS.matcher(sql);
+    Long lastSec = null;
+    while (m.find()) {
+      try {
+        HiveConf conf = new HiveConf();
+        conf.set(HiveConf.ConfVars.HIVE_QUERY_TIMEOUT_SECONDS.varname, m.group(1).trim());
+        long sec = HiveConf.getTimeVar(conf, HiveConf.ConfVars.HIVE_QUERY_TIMEOUT_SECONDS, TimeUnit.SECONDS);
+        lastSec = sec;
+      } catch (Exception e) {
+        LOG.debug("Could not parse session query timeout fragment: {}", m.group(0), e);
+      }
+    }
+    if (lastSec != null) {
+      connection.recordSessionQueryTimeoutFromSet(lastSec);
+    }
+  }
+
+  /**
+   * HIVE-28265: Prefer server error text unless it is empty or the known-broken "0 seconds" case;
+   * otherwise derive seconds from JDBC {@link #setQueryTimeout(int)} or last session SET.
+   */
+  private String sqlTimeoutMessageForTimedOutState(String serverMessage) {
+    if (!needsLocalTimeoutMessageForTimedOut(serverMessage)) {
+      return serverMessage;
+    }
+    long effectiveSec = resolveEffectiveTimeoutSecondsForMessage();
+    if (effectiveSec > 0) {
+      return "Query timed out after " + effectiveSec + " seconds";
+    }
+    return "Query timed out";
+  }
+
+  private boolean needsLocalTimeoutMessageForTimedOut(String timeoutMsg) {
+    return StringUtils.isBlank(timeoutMsg)
+        || StringUtils.containsIgnoreCase(timeoutMsg, "after 0 seconds");
+  }
+
+  private long resolveEffectiveTimeoutSecondsForMessage() {
+    if (queryTimeout > 0) {
+      return queryTimeout;
+    }
+    long tracked = connection.getSessionQueryTimeoutSecondsTracked();
+    if (tracked > 0) {
+      return tracked;
+    }
+    return 0L;
+  }
+
+  private SQLException sqlExceptionForCanceledState(TGetOperationStatusResp statusResp) {
+    final String errMsg = statusResp.getErrorMessage();
+    final String fullErrMsg =
+        (errMsg == null || errMsg.isEmpty()) ? QUERY_CANCELLED_MESSAGE : QUERY_CANCELLED_MESSAGE + " " + errMsg;
+    return new SQLException(fullErrMsg, "01000");
+  }
+
   TGetOperationStatusResp waitForOperationToComplete() throws SQLException {
     TGetOperationStatusResp statusResp = null;
 
@@ -436,13 +509,9 @@ TGetOperationStatusResp waitForOperationToComplete() throws SQLException {
             isLogBeingGenerated = false;
             break;
           case CANCELED_STATE:
-            // 01000 -> warning
-            final String errMsg = statusResp.getErrorMessage();
-            final String fullErrMsg =
-                (errMsg == null || errMsg.isEmpty()) ? QUERY_CANCELLED_MESSAGE : QUERY_CANCELLED_MESSAGE + " " + errMsg;
-            throw new SQLException(fullErrMsg, "01000");
+            throw sqlExceptionForCanceledState(statusResp);
           case TIMEDOUT_STATE:
-            throw new SQLTimeoutException("Query timed out after " + queryTimeout + " seconds");
+            throw new SQLTimeoutException(sqlTimeoutMessageForTimedOutState(statusResp.getErrorMessage()));
           case ERROR_STATE:
             // Get the error details from the underlying exception
             throw new SQLException(statusResp.getErrorMessage(), statusResp.getSqlState(),

service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java

@@ -179,6 +179,11 @@ private void prepare(QueryState queryState) throws HiveSQLException {
           try {
             final String queryId = queryState.getQueryId();
             log.info("Query timed out after: {} seconds. Cancelling the execution now: {}", queryTimeout, queryId);
+            setOperationException(new HiveSQLException(
+                "Query timed out after " + queryTimeout + " seconds",
+                "HYT00",
+                0,
+                queryId));
             SQLOperation.this.cancel(OperationState.TIMEDOUT);
           } catch (HiveSQLException e) {
             log.error("Error cancelling the query after timeout: {} seconds", queryTimeout, e);
@@ -334,7 +339,9 @@ public Object run() throws HiveSQLException {
             runQuery();
           } catch (HiveSQLException e) {
             // TODO: why do we invent our own error path op top of the one from Future.get?
-            setOperationException(e);
+            if (getState() != OperationState.TIMEDOUT) {
+              setOperationException(e);
+            }
             log.error("Error running hive query", e);
           } finally {
             if (!embedded) {
@@ -353,7 +360,9 @@ public Object run() throws HiveSQLException {
       try {
         currentUGI.doAs(doAsAction);
       } catch (Exception e) {
-        setOperationException(new HiveSQLException(e));
+        if (getState() != OperationState.TIMEDOUT) {
+          setOperationException(new HiveSQLException(e));
+        }
         log.error("Error running hive query as user : {}", currentUGI.getShortUserName(), e);
       } finally {
         /**