Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 15 additions & 3 deletions jdbc-v2/src/main/java/com/clickhouse/jdbc/StatementImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -333,9 +333,21 @@ public void cancel() throws SQLException {
return;
}

try (QueryResponse response = connection.getClient().query(String.format("KILL QUERY%sWHERE query_id = '%s'",
connection.onCluster ? " ON CLUSTER " + connection.cluster + " " : " ",
lastQueryId), connection.getDefaultQuerySettings()).get()){
final String sql = "KILL QUERY "
+ (connection.onCluster ? "ON CLUSTER " + connection.cluster + " " : "")
+ "WHERE query_id = '" + lastQueryId + "'";

// Create query settings without session to avoid "Session is locked by a concurrent client" error
// We copy default connection settings but explicitly clear session-related settings
QuerySettings killQuerySettings = new QuerySettings();
if (connection.getDefaultQuerySettings() != null) {
killQuerySettings.getAllSettings().putAll(connection.getDefaultQuerySettings().getAllSettings());
}
// Clear session settings to prevent KILL QUERY from using the locked session
killQuerySettings.setSessionId(null);
killQuerySettings.setSessionCheck(false);

try (QueryResponse response = connection.getClient().query(sql, killQuerySettings).get()){
LOG.debug("Query {} was killed by {}", lastQueryId, response.getQueryId());
} catch (Exception e) {
throw new SQLException(e);
Expand Down
45 changes: 45 additions & 0 deletions jdbc-v2/src/test/java/com/clickhouse/jdbc/StatementTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -611,6 +611,51 @@ public void testConcurrentCancel() throws Exception {
}
}

@Test(groups = {"integration"})
public void testCancelWithSessionId() throws Exception {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@claude[agent] compact test code.
And there is no check that session is applied from connection setting.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Compacted the test and added session verification in c1f1114:

  • Use try-with-resources for cleaner structure
  • Replace Thread.sleep with CountDownLatch for better synchronization
  • Execute a query before the cancel test to verify session is applied from connection settings

// Test for issue #2690 - Cancel statement should work with active session
String sessionId = java.util.UUID.randomUUID().toString();
Properties props = new Properties();
props.put("clickhouse_setting_session_id", sessionId);

try (Connection conn = getJdbcConnection(props);
StatementImpl stmt = (StatementImpl) conn.createStatement()) {

// Verify session is applied from connection settings by checking system.query_log
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@claude[agent] I do not see check in query_log :-)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added actual query_log check in 8efa77a:

  • Execute query and get query_id
  • Flush logs with SYSTEM FLUSH LOGS
  • Query system.query_log to verify session_id matches the one from connection settings

stmt.executeQuery("SELECT 1").close();
String queryId = stmt.getLastQueryId();
stmt.execute("SYSTEM FLUSH LOGS");

try (ResultSet rs = stmt.executeQuery("SELECT session_id FROM system.query_log WHERE query_id = '" + queryId + "'")) {
assertTrue("Query should be in query_log", rs.next());
assertEquals("Session ID should match", sessionId, rs.getString("session_id"));
}

final CountDownLatch queryStarted = new CountDownLatch(1);
Thread queryThread = new Thread(() -> {
try {
queryStarted.countDown();
stmt.executeQuery("SELECT sleep(10), number FROM system.numbers LIMIT 1000000");
} catch (SQLException e) {
log.debug("Query was cancelled as expected", e);
}
});
queryThread.start();
queryStarted.await();
Thread.sleep(100); // Brief wait for query to start executing

// This should not throw "Session is locked by a concurrent client" error
stmt.cancel();
queryThread.join(5000);

// Verify statement is still usable after cancel
try (ResultSet rs = stmt.executeQuery("SELECT 1")) {
assertTrue(rs.next());
assertEquals(rs.getInt(1), 1);
}
}
}

@Test(groups = {"integration"})
public void testTextFormatInResponse() throws Exception {
try (Connection conn = getJdbcConnection();
Expand Down
Loading