[client-v2, jdbc-v2] Make 159 TIMEOUT_EXCEEDED non-retryable + honour setQueryTimeout

CHANGELOG.md

@@ -171,6 +171,16 @@
 
 ### Bug Fixes 
 
+- **[client-v2]** `ServerException` with code `159 TIMEOUT_EXCEEDED` is no longer reported as retryable. The server
+  raises it once the query has already consumed its whole `max_execution_time` budget, so every automatic retry spends
+  that budget again and multiplies the load that caused the timeout. The fix was released in `0.9.9` but never reached
+  `main`, so `0.10.0` shipped the original behaviour.
+  (https://github.com/ClickHouse/clickhouse-java/issues/3136)
+- **[jdbc-v2]** Fixed `Statement#setQueryTimeout` being ignored. The client executes a query in the calling thread
+  unless asynchronous operations are enabled, and the future timeout then has no effect, so the value is applied as the
+  `max_execution_time` server setting instead. An execution timeout is reported as `SQLTimeoutException` carrying the
+  server error code as its vendor code. The fix was released in `0.9.9` but never reached `main`, so `0.10.0` shipped
+  the original behaviour. (https://github.com/ClickHouse/clickhouse-java/issues/3136)
 - **[jdbc-v2]** Added the non-reserved keywords `AGGREGATE`, `BOUNDED`, `EXTEND`, `HANDLER`, `IDLE`, `PROTOCOL`,
   `RECENT`, `TIMEOUT` and `UNORDERED` (ClickHouse `26.8+`; `IDLE`, `TIMEOUT` and `RECENT` come from the multi-word
   keywords `IDLE TIMEOUT` and `RECENT SAMPLES`) to the list of keywords allowed in identifier positions. The server

client-v2/src/main/java/com/clickhouse/client/api/ServerException.java

@@ -8,6 +8,8 @@ public class ServerException extends ClickHouseException {
 
     public static final int UNKNOWN_SETTING = 115;
 
+    public static final int EXECUTION_TIMEOUT = 159;
+
     private final int code;
 
     private final int transportProtocolCode;
@@ -56,10 +58,9 @@ public String getQueryId() {
     private boolean discoverIsRetryable(int code, String message, int transportProtocolCode) {
         // Let's check if we have a ServerException to reference the error code
         // https://github.com/ClickHouse/ClickHouse/blob/master/src/Common/ErrorCodes.cpp
-        switch (code) { // UNEXPECTED_END_OF_FILE
+        switch (code) {
             case 3: // UNEXPECTED_END_OF_FILE
             case 107: // FILE_DOESNT_EXIST
-            case 159: // TIMEOUT_EXCEEDED
             case 164: // READONLY
             case 202: // TOO_MANY_SIMULTANEOUS_QUERIES
             case 203: // NO_FREE_CONNECTION

client-v2/src/test/java/com/clickhouse/client/api/ServerExceptionTest.java

@@ -0,0 +1,32 @@
+package com.clickhouse.client.api;
+
+import org.testng.Assert;
+import org.testng.annotations.DataProvider;
+import org.testng.annotations.Test;
+
+public class ServerExceptionTest {
+
+    @DataProvider(name = "retryableCodes")
+    public static Object[][] retryableCodes() {
+        return new Object[][] {
+                // Execution timeout means the server already spent the whole time budget on the query,
+                // so retrying it can only spend it again.
+                {159, "TIMEOUT_EXCEEDED", false},
+                {60, "UNKNOWN_TABLE", false},
+                {62, "SYNTAX_ERROR", false},
+                {241, "MEMORY_LIMIT_EXCEEDED", true},
+                {209, "SOCKET_TIMEOUT", true},
+                {210, "NETWORK_ERROR", true},
+                {999, "KEEPER_EXCEPTION", true},
+        };
+    }
+
+    @Test(groups = {"unit"}, dataProvider = "retryableCodes")
+    public void testIsRetryable(int code, String codeName, boolean expected) {
+        ServerException exception = new ServerException(code, "DB::Exception: " + codeName, 500, "query-id");
+
+        Assert.assertEquals(exception.isRetryable(), expected,
+                "Unexpected retryability for code " + code + " (" + codeName + ")");
+        Assert.assertEquals(exception.getCode(), code);
+    }
+}

client-v2/src/test/java/com/clickhouse/client/api/internal/HttpAPIClientHelperTest.java

@@ -341,8 +341,8 @@ public void testExecuteRequestThrowsConnectExceptionOn503() throws Exception {
 
     @DataProvider(name = "serverExceptionRetryCases")
     public static Object[][] serverExceptionRetryCases() {
-        // Server code 159 (TIMEOUT_EXCEEDED) is retryable; code 60 (TABLE_NOT_FOUND) is not.
-        ServerException retryable = new ServerException(159, "TIMEOUT_EXCEEDED", 500, "q1");
+        // Server code 209 (SOCKET_TIMEOUT) is retryable; code 60 (TABLE_NOT_FOUND) is not.
+        ServerException retryable = new ServerException(209, "SOCKET_TIMEOUT", 500, "q1");
         ServerException nonRetryable = new ServerException(60, "TABLE_NOT_FOUND", 404, "q2");
         return new Object[][]{
                 // ServerException thrown directly (behaviour that already worked; pinned as contrast).

docs/features.md

@@ -79,7 +79,7 @@ Compatibility-sensitive traits:
 - Schema and database context: Supports database selection through URL, `setSchema`, `USE`, and statement-level settings.
 - Non-transactional operation: Exposes ClickHouse-appropriate transaction behavior with auto-commit semantics and unsupported transactional features.
 - Statement execution: Supports `execute`, `executeQuery`, `executeUpdate`, large update counts, and forward-only/read-only statements.
-- Query cancellation and timeout: Supports JDBC query timeout handling and query cancellation through server-side `KILL QUERY`, with optional JDBC `cluster_name` property support to add `ON CLUSTER '<name>'` for cluster-wide cancellation.
+- Query cancellation and timeout: Supports JDBC query timeout handling and query cancellation through server-side `KILL QUERY`, with optional JDBC `cluster_name` property support to add `ON CLUSTER '<name>'` for cluster-wide cancellation. `Statement#setQueryTimeout` is applied as the `max_execution_time` server setting, because without asynchronous operations the query runs in the calling thread and a client-side future timeout cannot interrupt it. Exceeding the timeout raises `SQLTimeoutException`, whose vendor code is the ClickHouse error code when the server reported the timeout.
 - Batch execution: Supports batched statements and prepared-statement batches, including multi-row rewrite for eligible `INSERT ... VALUES` statements.
 - Prepared statements: Supports `?` parameters through client-side SQL rendering and validates that all parameters are bound before execution.
 - SQL parsing and classification: Classifies SQL to distinguish queries, updates, inserts, `USE`, and role-changing statements, with selectable parser backends.

jdbc-v2/src/main/java/com/clickhouse/jdbc/StatementImpl.java

@@ -1,6 +1,7 @@
 package com.clickhouse.jdbc;
 
 import com.clickhouse.client.api.ClientConfigProperties;
+import com.clickhouse.client.api.ServerException;
 import com.clickhouse.client.api.data_formats.ClickHouseFormatReader;
 import com.clickhouse.client.api.data_formats.JSONEachRowFormatReader;
 import com.clickhouse.client.api.internal.ServerSettings;
@@ -17,6 +18,7 @@
 import java.net.SocketTimeoutException;
 import java.sql.ResultSet;
 import java.sql.SQLException;
+import java.sql.SQLTimeoutException;
 import java.sql.SQLWarning;
 import java.sql.Statement;
 import java.util.ArrayDeque;
@@ -26,6 +28,7 @@
 import java.util.UUID;
 import java.util.concurrent.ConcurrentLinkedQueue;
 import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
 import java.util.function.Supplier;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
@@ -360,6 +363,7 @@ protected ResultSetImpl executeQueryImpl(String sql, QuerySettings settings) thr
             }
             handleSocketTimeoutException(e);
             onResultSetClosed(null);
+            throwOnExecutionTimeout(e, mergedSettings.getQueryId());
             throw ExceptionUtils.toSqlState(e);
         }
     }
@@ -370,6 +374,29 @@ protected void handleSocketTimeoutException(Exception e) {
         }
     }
 
+    /**
+     * Translates an execution timeout into {@link SQLTimeoutException}, which the JDBC spec requires when a
+     * statement exceeds the limit set by {@link #setQueryTimeout(int)}. A timeout is reported either by the
+     * client, when the query is awaited with a timeout, or by the server as error code
+     * {@link ServerException#EXECUTION_TIMEOUT}. The server error code is carried over as the vendor code so
+     * callers can classify the failure without unwrapping the cause chain.
+     *
+     * @param e exception thrown by the query execution
+     * @param queryId id of the query that failed
+     */
+    protected void throwOnExecutionTimeout(Exception e, String queryId) throws SQLTimeoutException {
+        ServerException serverException = e instanceof ServerException ? (ServerException) e
+                : e.getCause() instanceof ServerException ? (ServerException) e.getCause() : null;
+        boolean isTimeout = e instanceof TimeoutException || e.getCause() instanceof TimeoutException
+                || (serverException != null && serverException.getCode() == ServerException.EXECUTION_TIMEOUT);
+
+        if (isTimeout) {
+            throw new SQLTimeoutException("Query execution time exceeded limit (queryId=" + queryId + ")",
+                    ExceptionUtils.SQL_STATE_OPERATION_CANCELLED,
+                    serverException == null ? ServerException.CODE_UNKNOWN : serverException.getCode(), e);
+        }
+    }
+
     @Override
     public int executeUpdate(String sql) throws SQLException {
         ensureOpen();
@@ -396,6 +423,7 @@ protected long executeUpdateImpl(String sql, QuerySettings settings) throws SQLE
             lastQueryId = response.getQueryId();
         } catch (Exception e) {
             handleSocketTimeoutException(e);
+            throwOnExecutionTimeout(e, mergedSettings.getQueryId());
             throw ExceptionUtils.toSqlState(e);
         }
 
@@ -469,9 +497,33 @@ public int getQueryTimeout() throws SQLException {
     @Override
     public void setQueryTimeout(int seconds) throws SQLException {
         ensureOpen();
+        if (seconds < 0) {
+            throw new SQLException("Timeout should be >= 0 but " + seconds + " was passed");
+        }
+
+        if (seconds > 0) {
+            // With asynchronous operations the query is awaited with a timeout, which bounds the call on its own.
+            // Otherwise it runs in the calling thread and `max_execution_time` is the only way to bound it.
+            if (!isAsyncOperationsEnabled()) {
+                getLocalSettings().setMaxExecutionTime(seconds);
+            }
+        } else {
+            getLocalSettings().resetOption(ClientConfigProperties.serverSetting(ServerSettings.MAX_EXECUTION_TIME));
+        }
         queryTimeout = seconds;
     }
 
+    private boolean isAsyncOperationsEnabled() {
+        try {
+            return Boolean.parseBoolean(getConnection().getClient().getConfiguration()
+                    .getOrDefault(ClientConfigProperties.ASYNC_OPERATIONS.getKey(),
+                            ClientConfigProperties.ASYNC_OPERATIONS.getDefaultValue()));
+        } catch (Exception e) {
+            LOG.error("Failed to read client configuration " + ClientConfigProperties.ASYNC_OPERATIONS.getKey(), e);
+            return false;
+        }
+    }
+
     @Override
     public void cancel() throws SQLException {
         if (closed) {

jdbc-v2/src/test/java/com/clickhouse/jdbc/StatementTest.java

@@ -1,6 +1,7 @@
 package com.clickhouse.jdbc;
 
 import com.clickhouse.client.api.ClientConfigProperties;
+import com.clickhouse.client.api.ServerException;
 import com.clickhouse.client.api.Session;
 import com.clickhouse.client.api.data_formats.GsonJsonParserFactory;
 import com.clickhouse.client.api.data_formats.JacksonJsonParserFactory;
@@ -24,6 +25,7 @@
 import java.sql.ResultSet;
 import java.sql.ResultSetMetaData;
 import java.sql.SQLException;
+import java.sql.SQLTimeoutException;
 import java.sql.Statement;
 import java.time.LocalDate;
 import java.util.Arrays;
@@ -44,13 +46,18 @@
 import static org.testng.Assert.assertSame;
 import static org.testng.Assert.assertThrows;
 import static org.testng.Assert.assertTrue;
+import static org.testng.Assert.expectThrows;
 import static org.testng.Assert.fail;
 
 
 @Test(groups = {"integration"})
 public class StatementTest extends JdbcIntegrationTest {
     private static final Logger log = LoggerFactory.getLogger(StatementTest.class);
 
+    /** Runs long enough on a single thread for a low `max_execution_time` to interrupt it. */
+    private static final String SLOW_QUERY =
+            "SELECT count(), sum(sipHash64(number)) FROM numbers(1000000000) SETTINGS max_threads = 1";
+
     @Test(groups = {"integration"})
     public void testExecuteQuerySimpleNumbers() throws Exception {
         try (Connection conn = getJdbcConnection()) {
@@ -1763,6 +1770,66 @@ public void testEscapedSQLToNative(String sql, String expected) {
         assertEquals(StatementImpl.escapedSQLToNative(sql), expected);
     }
 
+    @Test(groups = {"integration"})
+    public void testSetQueryTimeoutRejectsNegativeValue() throws Exception {
+        try (Connection conn = getJdbcConnection();
+             Statement stmt = conn.createStatement()) {
+            assertThrows(SQLException.class, () -> stmt.setQueryTimeout(-1));
+            assertEquals(stmt.getQueryTimeout(), 0);
+        }
+    }
+
+    @Test(groups = {"integration"})
+    public void testSetQueryTimeoutSetsAndResetsMaxExecutionTime() throws Exception {
+        try (Connection conn = getJdbcConnection();
+             StatementImpl stmt = (StatementImpl) conn.createStatement()) {
+            assertNull(stmt.getLocalSettings().getMaxExecutionTime());
+
+            stmt.setQueryTimeout(5);
+            assertEquals(stmt.getQueryTimeout(), 5);
+            assertEquals(stmt.getLocalSettings().getMaxExecutionTime(), Integer.valueOf(5));
+
+            stmt.setQueryTimeout(0);
+            assertEquals(stmt.getQueryTimeout(), 0);
+            assertNull(stmt.getLocalSettings().getMaxExecutionTime());
+        }
+    }
+
+    @Test(groups = {"integration"})
+    public void testSetQueryTimeoutLeavesMaxExecutionTimeUnsetForAsyncOperations() throws Exception {
+        Properties config = new Properties();
+        config.setProperty(ClientConfigProperties.ASYNC_OPERATIONS.getKey(), "true");
+        try (Connection conn = getJdbcConnection(config);
+             StatementImpl stmt = (StatementImpl) conn.createStatement()) {
+            stmt.setQueryTimeout(5);
+
+            assertEquals(stmt.getQueryTimeout(), 5);
+            assertNull(stmt.getLocalSettings().getMaxExecutionTime());
+        }
+    }
+
+    @Test(groups = {"integration"})
+    public void testServerExecutionTimeoutIsReportedAsSqlTimeoutException() throws Exception {
+        try (Connection conn = getJdbcConnection();
+             Statement stmt = conn.createStatement()) {
+            stmt.setQueryTimeout(1);
+
+            SQLTimeoutException e = expectThrows(SQLTimeoutException.class, () -> stmt.executeQuery(SLOW_QUERY));
+            assertEquals(e.getErrorCode(), ServerException.EXECUTION_TIMEOUT);
+        }
+    }
+
+    @Test(groups = {"integration"})
+    public void testServerExecutionTimeoutIsReportedAsSqlTimeoutExceptionOnUpdate() throws Exception {
+        try (Connection conn = getJdbcConnection();
+             Statement stmt = conn.createStatement()) {
+            stmt.setQueryTimeout(1);
+
+            SQLTimeoutException e = expectThrows(SQLTimeoutException.class, () -> stmt.executeUpdate(SLOW_QUERY));
+            assertEquals(e.getErrorCode(), ServerException.EXECUTION_TIMEOUT);
+        }
+    }
+
     private static String getDBName(Statement stmt) throws SQLException {
         try (ResultSet rs = stmt.executeQuery("SELECT database()")) {
             rs.next();