[jdbc-v2] Do not fail Statement.cancel() when there is no running query

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

@@ -10,6 +10,8 @@ public class ServerException extends ClickHouseException {
 
     public static final int EXECUTION_TIMEOUT = 159;
 
+    public static final int NOTHING_TO_KILL = 785;
+
     private final int code;
 
     private final int transportProtocolCode;

client-v2/src/main/java/com/clickhouse/client/api/internal/ClientUtils.java

@@ -11,6 +11,8 @@
  */
 public final class ClientUtils {
 
+    private static final int DEFAULT_CAUSE_SEARCH_DEPTH = 3;
+
     private ClientUtils() {}
 
     public static boolean isNotBlank(String str) {
@@ -50,4 +52,46 @@ public static <V> void putIfAbsentIgnoreCase(Map<String, V> map, String key, V v
         }
         map.put(key, value);
     }
+
+    /**
+     * Finds the nearest exception of the given type in the cause chain. Checks {@code throwable} first and then
+     * follows up to 3 causes.
+     *
+     * @param throwable - exception to start from, can be {@code null}
+     * @param type - type of the exception to find, not {@code null}
+     * @param <T> - type of the exception to find
+     * @return {@code throwable} or its cause that is an instance of {@code type}, or {@code null} if not found
+     */
+    public static <T extends Throwable> T findCause(Throwable throwable, Class<T> type) {
+        return findCause(throwable, type, DEFAULT_CAUSE_SEARCH_DEPTH);
+    }
+
+    /**
+     * Finds the nearest exception of the given type in the cause chain. Checks {@code throwable} first and then
+     * follows up to {@code maxDepth} causes.
+     *
+     * @param throwable - exception to start from, can be {@code null}
+     * @param type - type of the exception to find, not {@code null}
+     * @param maxDepth - maximum number of causes to follow; {@code 0} checks only {@code throwable}
+     * @param <T> - type of the exception to find
+     * @return {@code throwable} or its cause that is an instance of {@code type}, or {@code null} if not found
+     * @throws IllegalArgumentException if {@code maxDepth} is negative
+     */
+    public static <T extends Throwable> T findCause(Throwable throwable, Class<T> type, int maxDepth) {
+        Objects.requireNonNull(type, "type");
+        if (maxDepth < 0) {
+            throw new IllegalArgumentException("maxDepth must not be negative: " + maxDepth);
+        }
+        Throwable current = throwable;
+        for (int depth = 0; current != null; depth++) {
+            if (type.isInstance(current)) {
+                return type.cast(current);
+            }
+            if (depth == maxDepth) {
+                break;
+            }
+            current = current.getCause();
+        }
+        return null;
+    }
 }

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

@@ -1,5 +1,6 @@
 package com.clickhouse.client.api.internal;
 
+import com.clickhouse.client.api.ServerException;
 import org.mockito.Mockito;
 import org.slf4j.Logger;
 import org.testng.Assert;
@@ -80,4 +81,58 @@ public void testPutIfAbsentIgnoreCaseNullKey() {
         Assert.expectThrows(NullPointerException.class, () -> ClientUtils.putIfAbsentIgnoreCase(map, null, "added"));
         Assert.assertTrue(map.isEmpty());
     }
+
+    @Test(groups = {"unit"}, dataProvider = "findCause")
+    public void testFindCause(int wrappers, Integer maxDepth, boolean found) {
+        ServerException target = new ServerException(ServerException.NOTHING_TO_KILL, "No query to kill", 500, "q1");
+        Throwable throwable = target;
+        for (int i = 0; i < wrappers; i++) {
+            throwable = new RuntimeException("wrapper " + i, throwable);
+        }
+
+        ServerException actual = maxDepth == null
+                ? ClientUtils.findCause(throwable, ServerException.class)
+                : ClientUtils.findCause(throwable, ServerException.class, maxDepth);
+
+        Assert.assertSame(actual, found ? target : null);
+    }
+
+    @DataProvider(name = "findCause")
+    public static Object[][] findCauseData() {
+        return new Object[][]{
+                {0, null, true},
+                {3, null, true},
+                {4, null, false},
+                {0, 0, true},
+                {1, 0, false},
+                {5, 5, true},
+        };
+    }
+
+    @Test(groups = {"unit"})
+    public void testFindCauseReturnsNearestMatch() {
+        IllegalArgumentException outer = new IllegalArgumentException("outer", new IllegalStateException("inner"));
+
+        Assert.assertSame(ClientUtils.findCause(new Exception(outer), RuntimeException.class), outer);
+    }
+
+    @Test(groups = {"unit"}, timeOut = 10000)
+    public void testFindCauseWithoutMatch() {
+        Exception first = new Exception("first");
+        Exception second = new Exception("second", first);
+        first.initCause(second);
+
+        Assert.assertNull(ClientUtils.findCause(null, ServerException.class));
+        Assert.assertNull(ClientUtils.findCause(new RuntimeException(new IOException()), ServerException.class));
+        Assert.assertNull(ClientUtils.findCause(first, ServerException.class, 10));
+    }
+
+    @Test(groups = {"unit"})
+    public void testFindCauseInvalidArguments() {
+        Throwable throwable = new RuntimeException();
+
+        Assert.expectThrows(NullPointerException.class, () -> ClientUtils.findCause(throwable, null));
+        Assert.expectThrows(IllegalArgumentException.class,
+                () -> ClientUtils.findCause(throwable, RuntimeException.class, -1));
+    }
 }

docs/features.md

@@ -108,6 +108,7 @@ Compatibility-sensitive traits:
 - Prepared statements are client-side SQL rendering, not server-side prepared statements. Changes to literal encoding or placeholder parsing are externally visible behavior.
 - `DatabaseMetaData#getSchemas`, `#getTables` and `#getColumns` return the same result-set columns and values with and without `jdbc_metadata_use_show_statements`, with two exceptions for the `SHOW` statements: `getTables()` returns `null` in `REMARKS` and `TYPE_SCHEM`, because `SHOW TABLES` returns no table comment and no database engine. `getColumns()` with the `SHOW` statements sends one `DESCRIBE TABLE` query for each table that matches, and skips a database or table that fails with `UNKNOWN_TABLE`, `UNKNOWN_DATABASE`, `ACCESS_DENIED` or `DATALAKE_DATABASE_ERROR` (for example, a table with column-level grants only, which `system.columns` shows with the granted columns).
 - JDBC `cluster_name` is compatibility-sensitive for cancellation behavior: when configured, `Statement.cancel()` issues `KILL QUERY ON CLUSTER '<name>'`, and when omitted it falls back to a local `KILL QUERY`.
+- `Statement.cancel()` does nothing when the statement has no running query (the query is finished or not started yet). The `NOTHING_TO_KILL` error, which servers with `kill_throw_if_noop` enabled (the default since ClickHouse 26.11) return for such a `KILL QUERY`, is not thrown to the caller. Other `KILL QUERY` errors are thrown as `SQLException`.
 - String parameters are escaped with backslash-based escaping: backslashes are doubled and single quotes are backslash-escaped before values are wrapped in single quotes.
 - `?` placeholder detection is SQL-aware and should not treat question marks inside quoted strings, quoted identifiers, comments, casts, or similar syntax as bind parameters.
 - String-like ClickHouse values have stable JDBC expectations: `String`, `FixedString`, and `Enum` values are returned as strings, while `UUID` is available both as `getString()` and `getObject(..., UUID.class)`.

history/latest/3264.md

@@ -0,0 +1,6 @@
+- **[client-v2, jdbc-v2]** Fixed `Statement.cancel()` to not throw an `SQLException` when the statement has no running
+  query (the query is finished or not started yet). Since ClickHouse 26.11, `kill_throw_if_noop` is enabled by default,
+  and the server returns `NOTHING_TO_KILL` for the `KILL QUERY` that `cancel()` sends. `cancel()` now ignores this
+  error. Other `KILL QUERY` errors (for example, an unknown `cluster_name`) are thrown as before. Added the
+  `ServerException.NOTHING_TO_KILL` error code constant to `client-v2`.
+  (https://github.com/ClickHouse/clickhouse-java/issues/3264)

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

@@ -4,6 +4,7 @@
 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.ClientUtils;
 import com.clickhouse.client.api.internal.ServerSettings;
 import com.clickhouse.client.api.query.QueryResponse;
 import com.clickhouse.client.api.query.QuerySettings;
@@ -522,11 +523,19 @@ public void cancel() throws SQLException {
         // KILL QUERY must not run inside the same session as the query being canceled otherwise it will
         // cause "Session is locked by a concurrent client" (SESSION_IS_LOCKED) error.
         QuerySettings cancelSettings = QuerySettings.merge(getLocalSettings(), new QuerySettings()).clearSession();
+        final String queryId = lastQueryId;
         try (QueryResponse response = connection.getClient().query(String.format("KILL QUERY%sWHERE query_id = '%s'",
                 connection.onCluster ? " ON CLUSTER " + SQLUtils.enquoteIdentifier(connection.cluster, true) + ' ' : ' ',
-                lastQueryId), cancelSettings).get()){
-            LOG.debug("Query {} was killed by {}", lastQueryId, response.getQueryId());
+                queryId), cancelSettings).get()){
+            LOG.debug("Query {} was killed by {}", queryId, response.getQueryId());
         } catch (Exception e) {
+            ServerException se = ClientUtils.findCause(e, ServerException.class);
+            if (se != null && se.getCode() == ServerException.NOTHING_TO_KILL) {
+                // The query is not running (it is finished or not started yet). A server with `kill_throw_if_noop`
+                // enabled (the default since 26.11) reports this as an error, but there is nothing to cancel.
+                LOG.debug("Query {} was not killed because it is not running: {}", queryId, se.getMessage());
+                return;
+            }
             throw new SQLException(e);
         }
     }

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

@@ -2,6 +2,7 @@
 
 import com.clickhouse.client.ClickHouseServerForTest;
 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;
@@ -11,6 +12,10 @@
 import com.clickhouse.data.ClickHouseFormat;
 import com.clickhouse.data.ClickHouseVersion;
 import com.clickhouse.jdbc.internal.SqlParserFacade;
+import com.github.tomakehurst.wiremock.WireMockServer;
+import com.github.tomakehurst.wiremock.client.WireMock;
+import com.github.tomakehurst.wiremock.common.ConsoleNotifier;
+import com.github.tomakehurst.wiremock.core.WireMockConfiguration;
 import org.apache.commons.lang3.RandomStringUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -781,6 +786,79 @@ public void testConcurrentCancel() throws Exception {
         }
     }
 
+    @Test(groups = {"integration"}, dataProvider = "testCancelWhenQueryIsNotRunningDP")
+    public void testCancelWhenQueryIsNotRunning(boolean asyncOperations) throws Exception {
+        Properties p = new Properties();
+        p.setProperty(ClientConfigProperties.ASYNC_OPERATIONS.getKey(), String.valueOf(asyncOperations));
+        if (isVersionMatch("[26.10,)")) {
+            p.setProperty(ClientConfigProperties.serverSetting("kill_throw_if_noop"), "1");
+        }
+        try (Connection conn = getJdbcConnection(p);
+             Statement stmt = conn.createStatement()) {
+            stmt.cancel();
+
+            try (ResultSet rs = stmt.executeQuery("SELECT 1")) {
+                assertTrue(rs.next());
+                assertEquals(rs.getInt(1), 1);
+            }
+            stmt.cancel();
+
+            try (ResultSet rs = stmt.executeQuery("SELECT 2")) {
+                assertTrue(rs.next());
+                assertEquals(rs.getInt(1), 2);
+            }
+        }
+
+        String testCluster = "test_cluster_" + RandomStringUtils.randomAlphanumeric(10);
+        p.setProperty(DriverProperties.CLUSTER_NAME.getKey(), testCluster);
+        try (Connection conn = getJdbcConnection(p);
+             Statement stmt = conn.createStatement()) {
+            SQLException e = expectThrows(SQLException.class, stmt::cancel);
+            assertTrue(e.getMessage().contains(testCluster), "Exception should mention the missing cluster");
+        }
+    }
+
+    @DataProvider
+    public static Object[][] testCancelWhenQueryIsNotRunningDP() {
+        return new Object[][] {
+                {false},
+                {true},
+        };
+    }
+
+    @Test(groups = {"integration"}, dataProvider = "testCancelWhenQueryIsNotRunningDP")
+    public void testCancelIgnoresNothingToKillError(boolean asyncOperations) throws Exception {
+        if (isCloud()) {
+            return; // mocked server
+        }
+
+        WireMockServer mockServer = new WireMockServer(WireMockConfiguration
+                .options().dynamicPort().notifier(new ConsoleNotifier(false)));
+        mockServer.start();
+        try {
+            // Response of a server with `kill_throw_if_noop` enabled
+            mockServer.addStubMapping(WireMock.post(WireMock.anyUrl())
+                    .withRequestBody(WireMock.matching("KILL QUERY WHERE .*"))
+                    .willReturn(WireMock.serverError()
+                            .withHeader("X-ClickHouse-Exception-Code", String.valueOf(ServerException.NOTHING_TO_KILL))
+                            .withBody("Code: 785. DB::Exception: No query to kill. (NOTHING_TO_KILL)"))
+                    .build());
+
+            Properties p = new Properties();
+            p.setProperty(ClientConfigProperties.COMPRESS_SERVER_RESPONSE.getKey(), "false");
+            p.setProperty(ClientConfigProperties.SERVER_TIMEZONE.getKey(), "UTC"); // no server info query
+            p.setProperty(ClientConfigProperties.ASYNC_OPERATIONS.getKey(), String.valueOf(asyncOperations));
+            try (Connection conn = new ConnectionImpl("jdbc:clickhouse://localhost:" + mockServer.port() + "/default", p);
+                 Statement stmt = conn.createStatement()) {
+                stmt.cancel();
+            }
+            mockServer.verify(1, WireMock.postRequestedFor(WireMock.anyUrl())
+                    .withRequestBody(WireMock.matching("KILL QUERY WHERE .*")));
+        } finally {
+            mockServer.stop();
+        }
+    }
+
     /**
      * Waits until the given query id appears in {@code system.processes}, so we know the operation has actually
      * started on the server before attempting to cancel it. Uses a dedicated connection (no session) to observe.