Skip to content

Commit 8b39570

Browse files
committed
Fixed clearing session when do cancel on Statement in jdbc
1 parent 51c7955 commit 8b39570

8 files changed

Lines changed: 99 additions & 12 deletions

File tree

CHANGELOG.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@
2929

3030
### New Features
3131

32+
- **[client-v2]** Added `Session` API to encapsulate and manage ClickHouse session settings (`session_id`, `session_check`, `session_timeout`, `session_timezone`) as a reusable object. The `Session` instance can be applied to any request settings using `applyTo()`, and session state can be cleared via `clearSession()`. Additionally, added `resetOption(String)` and `suppressOption(String)` to `InsertSettings`, `QuerySettings`, and `CommonSettings` to allow removing or suppressing specific settings. Suppressed settings (set to `null`) will not be sent to the server, which is useful for overriding global settings.
33+
3234
- **[client-v2]** Added runtime credential update APIs on `Client`: `updateUserAndPassword(String, String)`, `updateAccessToken(String)`, and `updateBearerToken(String)`. Subsequent requests on the same `Client` instance use the new credentials without rebuilding the client. The authentication method is fixed at construction time; calling a runtime updater that does not match the configured method throws `ClientMisconfigurationException`. See `docs/authentication.md` for details and migration guidance.
3335

3436
- **[jdbc-v2]** Added `cluster_name` configuration property to specify a target cluster for statements like `KILL QUERY` that require an `ON CLUSTER` clause to execute across all nodes. (https://github.com/ClickHouse/clickhouse-java/issues/2837)
@@ -39,7 +41,7 @@
3941

4042
### Bug Fixes
4143

42-
- **[jdbc-v2]** Fixed `Statement.cancel()` throwing `SESSION_IS_LOCKED` when the statement was running inside a ClickHouse session (e.g. via `clickhouse_setting_session_id`). The `KILL QUERY` request issued by `cancel()` now runs outside the session, so it no longer contends with the running query for the session lock. (https://github.com/ClickHouse/clickhouse-java/issues/2690)
44+
- **[jdbc-v2]** Fixed `Statement.cancel()` throwing `SESSION_IS_LOCKED` when the statement was running inside a ClickHouse session. The driver now accepts `session_id`, `session_check`, and `session_timeout` as first-class connection properties and correctly suppresses them when issuing a `KILL QUERY` during cancellation. This ensures the cancellation request runs outside the session and no longer contends with the running query for the session lock. (https://github.com/ClickHouse/clickhouse-java/issues/2690, https://github.com/ClickHouse/clickhouse-java/issues/2881)
4345

4446
- **[client-v2]** Fixed inconsistent use of `executionTimeout` parameter in `Client` component. The timeout was previously set in milliseconds but mistakenly retrieved and used in seconds in some places. Now it correctly uses milliseconds consistently. (https://github.com/ClickHouse/clickhouse-java/issues/2358)
4547

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

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import com.clickhouse.client.api.http.ClickHouseHttpProto;
44
import com.clickhouse.client.api.internal.ValidationUtils;
55

6+
import java.util.HashMap;
67
import java.util.Map;
78

89
/**
@@ -83,7 +84,7 @@ public synchronized void updateSessionId(String sessionId) {
8384
setSessionId(sessionId);
8485
}
8586

86-
public synchronized void applyTo(Map<String, Object> requestSettings) {
87+
public synchronized void applyTo(Map<? super String, Object> requestSettings) {
8788
putIfSet(requestSettings, ClickHouseHttpProto.QPARAM_SESSION_ID, sessionId);
8889
putIfSet(requestSettings, ClickHouseHttpProto.QPARAM_SESSION_CHECK,
8990
sessionCheck == null ? null : (sessionCheck ? "1" : "0"));
@@ -92,7 +93,15 @@ public synchronized void applyTo(Map<String, Object> requestSettings) {
9293
putIfSet(requestSettings, ClickHouseHttpProto.QPARAM_SESSION_TIMEZONE, sessionTimezone);
9394
}
9495

95-
private static void putIfSet(Map<String, Object> settings, String key, String value) {
96+
public static void clearSession(HashMap<String, Object> settings) {
97+
settings.put(ClientConfigProperties.serverSetting(ClickHouseHttpProto.QPARAM_SESSION_ID), null);
98+
settings.put(ClientConfigProperties.serverSetting(ClickHouseHttpProto.QPARAM_SESSION_TIMEOUT), null);
99+
settings.put(ClientConfigProperties.serverSetting(ClickHouseHttpProto.QPARAM_SESSION_CHECK), null);
100+
// Do not clean `session_timezone` setting because it is not related to session management and used to
101+
// set timezone for consequent queries in some multi-user applications.
102+
}
103+
104+
private static void putIfSet(Map<? super String, Object> settings, String key, String value) {
96105
if (value != null) {
97106
settings.put(ClientConfigProperties.serverSetting(key), value);
98107
}

client-v2/src/main/java/com/clickhouse/client/api/insert/InsertSettings.java

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,26 @@ public InsertSettings setOption(String option, Object value) {
6060
return this;
6161
}
6262

63+
/**
64+
* Removes options from the settings.
65+
* @param option - configuration option name
66+
*/
67+
public InsertSettings resetOption(String option) {
68+
settings.resetOption(option);
69+
return this;
70+
}
71+
72+
/**
73+
* Makes option value to null that makes agent to remove it from final collection.
74+
* This is useful to override even global settings when they need to be removed.
75+
* @param option - option key
76+
* @return current settings instance
77+
*/
78+
public InsertSettings suppressOption(String option) {
79+
settings.suppressOption(option);
80+
return this;
81+
}
82+
6383
/**
6484
* Get all settings as an unmodifiable map.
6585
*

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

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ public class CommonSettings {
2020

2121
private String operationId;
2222
private String logComment;
23-
protected Map<String, Object> settings;
23+
protected HashMap<String, Object> settings; // using hashmap to store null values
2424

2525
public CommonSettings() {
2626
settings = new HashMap<>();
@@ -66,11 +66,27 @@ public CommonSettings setOption(String option, Object value) {
6666
return this;
6767
}
6868

69+
/**
70+
* Removes option from the setting
71+
* @param option - option key
72+
* @return current settings instance
73+
*/
6974
public CommonSettings resetOption(String option) {
7075
settings.remove(option);
7176
return this;
7277
}
7378

79+
/**
80+
* Makes option value to null that makes agent to remove it from final collection.
81+
* This is useful to override even global settings when they need to be removed.
82+
* @param option - option key
83+
* @return current settings instance
84+
*/
85+
public CommonSettings suppressOption(String option) {
86+
settings.put(option, null);
87+
return this;
88+
}
89+
7490
/**
7591
* Get all settings as an unmodifiable map.
7692
*
@@ -140,11 +156,7 @@ public CommonSettings use(Session session) {
140156
}
141157

142158
public void clearSession() {
143-
resetOption(ClientConfigProperties.serverSetting(ClickHouseHttpProto.QPARAM_SESSION_ID));
144-
resetOption(ClientConfigProperties.serverSetting(ClickHouseHttpProto.QPARAM_SESSION_CHECK));
145-
resetOption(ClientConfigProperties.serverSetting(ClickHouseHttpProto.QPARAM_SESSION_TIMEOUT));
146-
// Do not clean `session_timezone` setting because it is not related to session management and used to
147-
// set timezone for consequent queries in some multi-user applications.
159+
Session.clearSession(settings);
148160
}
149161

150162
/**

client-v2/src/main/java/com/clickhouse/client/api/query/QuerySettings.java

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,11 +50,26 @@ public QuerySettings setOption(String option, Object value) {
5050
return this;
5151
}
5252

53+
/**
54+
* Removes options from the settings.
55+
* @param option - configuration option name
56+
*/
5357
public QuerySettings resetOption(String option) {
5458
settings.resetOption(option);
5559
return this;
5660
}
5761

62+
/**
63+
* Makes option value to null that makes agent to remove it from final collection.
64+
* This is useful to override even global settings when they need to be removed.
65+
* @param option - option key
66+
* @return current settings instance
67+
*/
68+
public QuerySettings suppressOption(String option) {
69+
settings.suppressOption(option);
70+
return this;
71+
}
72+
5873
/**
5974
* Gets a configuration option.
6075
*

client-v2/src/test/java/com/clickhouse/client/ClientTests.java

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@
5050
import java.util.concurrent.Executors;
5151
import java.util.concurrent.TimeUnit;
5252
import java.util.concurrent.atomic.AtomicBoolean;
53+
import java.util.function.BiConsumer;
5354
import java.util.function.BiFunction;
5455
import java.util.function.Consumer;
5556
import java.util.function.Supplier;
@@ -729,6 +730,24 @@ public void testInvalidAuthConfiguration() throws Exception {
729730
Assert.assertTrue(e.getMessage().contains("Trust store and certificates cannot be used together"), e.getMessage()));
730731
}
731732

733+
@Test(groups = {"integration"})
734+
public void testOverrideSettings() throws Exception {
735+
final String clientTimezone = "America/Los_Angeles";
736+
try (Client client = newClient().setSessionTimezone(clientTimezone).build()) {
737+
738+
final BiConsumer<QuerySettings, String> checkTzStmt = (settings, timezone) -> {
739+
GenericRecord firstRecord = client.queryAll("SELECT timezone()", settings).stream().findFirst().get();
740+
Assert.assertEquals(firstRecord.getString(1), timezone);
741+
};
742+
743+
checkTzStmt.accept(new QuerySettings(), clientTimezone);
744+
745+
final String altTimezone = "America/New_York";
746+
checkTzStmt.accept(new QuerySettings().setSessionTimezone(altTimezone), altTimezone);
747+
checkTzStmt.accept(new QuerySettings().setOption(ClientConfigProperties.serverSetting("session_timezone"), null), "UTC");
748+
}
749+
}
750+
732751
public boolean isVersionMatch(String versionExpression, Client client) {
733752
List<GenericRecord> serverVersion = client.queryAll("SELECT version()");
734753
return ClickHouseVersion.of(serverVersion.get(0).getString(1)).check(versionExpression);

docs/features.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ This document lists stable, user-visible behavior in `client-v2` and `jdbc-v2` t
1111
- Proxy support: Can send requests through configured HTTP proxies, including proxy credentials.
1212
- Connection and socket tuning: Exposes pool sizing, keep-alive, reuse strategy, connect/request/socket timeouts, and low-level socket options.
1313
- Query execution: Executes SQL asynchronously and returns streaming query responses with response metadata and metrics.
14-
- Query settings: Supports per-query database selection, output format, execution limits, roles, log comments, headers, reusable `Session` objects, session settings, server settings, and network timeout overrides.
14+
- Query settings: Supports per-query database selection, output format, execution limits, roles, log comments, headers, reusable `Session` objects, session settings, server settings, and network timeout overrides. Settings explicitly set to `null` are suppressed and will not be sent to the server.
1515
- Parameterized SQL: Accepts named query parameters and can send them through supported HTTP request encodings.
1616
- Result materialization helpers: Provides streaming `Records`, generic row access, and convenience APIs that materialize all rows into generic records or typed POJOs.
1717
- Binary format readers: Reads ClickHouse binary result formats including `Native`, `RowBinary`, `RowBinaryWithNames`, and `RowBinaryWithNamesAndTypes`.

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

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package com.clickhouse.jdbc;
22

33
import com.clickhouse.client.api.ClientConfigProperties;
4+
import com.clickhouse.client.api.Session;
45
import com.clickhouse.client.api.internal.ServerSettings;
56
import com.clickhouse.client.api.query.GenericRecord;
67
import com.clickhouse.data.ClickHouseVersion;
@@ -758,7 +759,11 @@ public void testCancelQueryWithSession() throws Exception {
758759
// "Session is locked by a concurrent client" (SESSION_IS_LOCKED). The KILL QUERY request issued by
759760
// cancel() must not carry the session id of the query being cancelled.
760761
String sessionId = "test-session-" + UUID.randomUUID();
761-
try (Connection conn = getJdbcConnection()) {
762+
Properties properties = new Properties();
763+
Session session = new Session();
764+
session.setSessionId(sessionId);
765+
session.applyTo(properties);
766+
try (Connection conn = getJdbcConnection(properties)) {
762767
try (StatementImpl stmt = (StatementImpl) conn.createStatement()) {
763768
stmt.getLocalSettings().setSessionId(sessionId);
764769
stmt.setQueryTimeout(30); // safety net so a failed cancel cannot hang the test
@@ -799,7 +804,12 @@ public void testCancelInsertWithSession() throws Exception {
799804
// Regression test for #2690 covering a long-running INSERT executed inside a session.
800805
String tableName = getDatabase() + ".cancel_insert_with_session";
801806
String sessionId = "test-session-" + UUID.randomUUID();
802-
try (Connection conn = getJdbcConnection(Map.of(ASYNC_INSERT_SETTING_KEY, ServerSettings.OFF))) {
807+
Properties properties = new Properties();
808+
properties.put(ASYNC_INSERT_SETTING_KEY, ServerSettings.OFF);
809+
Session session = new Session();
810+
session.setSessionId(sessionId);
811+
session.applyTo(properties);
812+
try (Connection conn = getJdbcConnection(properties)) {
803813
try (Statement setup = conn.createStatement()) {
804814
setup.execute("DROP TABLE IF EXISTS " + tableName);
805815
setup.execute("CREATE TABLE " + tableName + " (num UInt64) ENGINE = MergeTree ORDER BY ()");

0 commit comments

Comments
 (0)