Merge pull request #2885 from ClickHouse/06/18/26/session_id_in_server_settings

[jdbc-v2] Fixes cleaning session id from server setting

Author: chernser

CHANGELOG.md

@@ -29,6 +29,8 @@
 
 ### New Features
 
+- **[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)` to `InsertSettings`, `QuerySettings`, and `CommonSettings` to allow removing specific settings. Settings explicitly set to `null` will not be sent to the server, which is useful for overriding global settings.
+
 - **[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.
 
 - **[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 @@
 
 ### Bug Fixes
 
-- **[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)
+- **[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)
 
 - **[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)
 

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

@@ -83,7 +83,7 @@ public synchronized void updateSessionId(String sessionId) {
         setSessionId(sessionId);
     }
 
-    public synchronized void applyTo(Map<String, Object> requestSettings) {
+    public synchronized void applyTo(Map<? super String, Object> requestSettings) {
         putIfSet(requestSettings, ClickHouseHttpProto.QPARAM_SESSION_ID, sessionId);
         putIfSet(requestSettings, ClickHouseHttpProto.QPARAM_SESSION_CHECK,
                 sessionCheck == null ? null : (sessionCheck ? "1" : "0"));
@@ -92,7 +92,15 @@ public synchronized void applyTo(Map<String, Object> requestSettings) {
         putIfSet(requestSettings, ClickHouseHttpProto.QPARAM_SESSION_TIMEZONE, sessionTimezone);
     }
 
-    private static void putIfSet(Map<String, Object> settings, String key, String value) {
+    public static void clearSession(Map<String, Object> settings) {
+        settings.put(ClientConfigProperties.serverSetting(ClickHouseHttpProto.QPARAM_SESSION_ID), null);
+        settings.put(ClientConfigProperties.serverSetting(ClickHouseHttpProto.QPARAM_SESSION_TIMEOUT), null);
+        settings.put(ClientConfigProperties.serverSetting(ClickHouseHttpProto.QPARAM_SESSION_CHECK), null);
+        // Do not clean `session_timezone` setting because it is not related to session management and used to
+        // set timezone for consequent queries in some multi-user applications.
+    }
+
+    private static void putIfSet(Map<? super String, Object> settings, String key, String value) {
         if (value != null) {
             settings.put(ClientConfigProperties.serverSetting(key), value);
         }

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

@@ -60,6 +60,15 @@ public InsertSettings setOption(String option, Object value) {
         return this;
     }
 
+    /**
+     * Removes options from the settings.
+     * @param option - configuration option name
+     */
+    public InsertSettings resetOption(String option) {
+        settings.resetOption(option);
+        return this;
+    }
+
     /**
      * Get all settings as an unmodifiable map.
      *

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

@@ -140,11 +140,7 @@ public CommonSettings use(Session session) {
     }
 
     public void clearSession() {
-        resetOption(ClientConfigProperties.serverSetting(ClickHouseHttpProto.QPARAM_SESSION_ID));
-        resetOption(ClientConfigProperties.serverSetting(ClickHouseHttpProto.QPARAM_SESSION_CHECK));
-        resetOption(ClientConfigProperties.serverSetting(ClickHouseHttpProto.QPARAM_SESSION_TIMEOUT));
-        // Do not clean `session_timezone` setting because it is not related to session management and used to
-        // set timezone for consequent queries in some multi-user applications.
+        Session.clearSession(settings);
     }
 
     /**

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

@@ -50,6 +50,7 @@
 import java.util.concurrent.Executors;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.function.BiConsumer;
 import java.util.function.BiFunction;
 import java.util.function.Consumer;
 import java.util.function.Supplier;
@@ -729,6 +730,24 @@ public void testInvalidAuthConfiguration() throws Exception {
                 Assert.assertTrue(e.getMessage().contains("Trust store and certificates cannot be used together"), e.getMessage()));
     }
 
+    @Test(groups = {"integration"})
+    public void testOverrideSettings() throws Exception {
+        final String clientTimezone = "America/Los_Angeles";
+        try (Client client = newClient().setSessionTimezone(clientTimezone).build()) {
+
+            final BiConsumer<QuerySettings, String> checkTzStmt = (settings, timezone) -> {
+                GenericRecord firstRecord = client.queryAll("SELECT timezone()", settings).stream().findFirst().get();
+                Assert.assertEquals(firstRecord.getString(1), timezone);
+            };
+
+            checkTzStmt.accept(new QuerySettings(), clientTimezone);
+
+            final String altTimezone = "America/New_York";
+            checkTzStmt.accept(new QuerySettings().setSessionTimezone(altTimezone), altTimezone);
+            checkTzStmt.accept(new QuerySettings().setOption(ClientConfigProperties.serverSetting("session_timezone"), null), "UTC");
+        }
+    }
+
     public boolean isVersionMatch(String versionExpression, Client client) {
         List<GenericRecord> serverVersion = client.queryAll("SELECT version()");
         return ClickHouseVersion.of(serverVersion.get(0).getString(1)).check(versionExpression);

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

@@ -177,6 +177,8 @@ public void testInsertSettingsSpecific() throws Exception {
             final InsertSettings settings = new InsertSettings();
             settings.setDatabase("test_db1");
             Assert.assertEquals(settings.getDatabase(), "test_db1");
+            settings.resetOption(ClientConfigProperties.DATABASE.getKey());
+            Assert.assertNull(settings.getDatabase());
         }
 
         {

docs/features.md

@@ -11,7 +11,7 @@ This document lists stable, user-visible behavior in `client-v2` and `jdbc-v2` t
 - Proxy support: Can send requests through configured HTTP proxies, including proxy credentials.
 - Connection and socket tuning: Exposes pool sizing, keep-alive, reuse strategy, connect/request/socket timeouts, and low-level socket options.
 - Query execution: Executes SQL asynchronously and returns streaming query responses with response metadata and metrics.
-- 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.
+- 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` will not be sent to the server.
 - Parameterized SQL: Accepts named query parameters and can send them through supported HTTP request encodings.
 - Result materialization helpers: Provides streaming `Records`, generic row access, and convenience APIs that materialize all rows into generic records or typed POJOs.
 - Binary format readers: Reads ClickHouse binary result formats including `Native`, `RowBinary`, `RowBinaryWithNames`, and `RowBinaryWithNamesAndTypes`.

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.Session;
 import com.clickhouse.client.api.internal.ServerSettings;
 import com.clickhouse.client.api.query.GenericRecord;
 import com.clickhouse.data.ClickHouseVersion;
@@ -754,40 +755,52 @@ public void testCancelQueryWithSession() throws Exception {
             throw new SkipException("Cloud + HTTP doesn't work well. Enough to test locally");
         }
 
-        // Regression test for #2690: cancelling a query that runs inside a session must not fail with
-        // "Session is locked by a concurrent client" (SESSION_IS_LOCKED). The KILL QUERY request issued by
-        // cancel() must not carry the session id of the query being cancelled.
         String sessionId = "test-session-" + UUID.randomUUID();
-        try (Connection conn = getJdbcConnection()) {
-            try (StatementImpl stmt = (StatementImpl) conn.createStatement()) {
-                stmt.getLocalSettings().setSessionId(sessionId);
-                stmt.setQueryTimeout(30); // safety net so a failed cancel cannot hang the test
+        Properties properties = new Properties();
+        Session session = new Session();
+        session.setSessionId(sessionId);
+        session.applyTo(properties);
+        try (Connection conn = getJdbcConnection(properties)) {
+            testCancelQueryWithSessionValidation(conn, sessionId);
+        }
 
-                final AtomicReference<Throwable> threadError = new AtomicReference<>();
-                final CountDownLatch started = new CountDownLatch(1);
-                Thread worker = new Thread(() -> {
-                    started.countDown();
-                    // Long-running query that only completes when killed.
-                    try (ResultSet rs = stmt.executeQuery("SELECT count() FROM system.numbers_mt")) {
-                        rs.next();
-                    } catch (Throwable t) {
-                        System.out.println("Error: " + t.getMessage());
-                        threadError.set(t);
-                    }
-                });
-                worker.start();
-                started.await();
+        // Test case when session id is in custom_http_header
+        properties = new Properties();
+        properties.put(DriverProperties.CUSTOM_HTTP_PARAMS.getKey(), "session_id=" + sessionId);
+        try (Connection conn = getJdbcConnection(properties)) {
+            testCancelQueryWithSessionValidation(conn, sessionId);
+        }
+    }
 
-                String queryId = waitForQueryId(stmt, 15);
-                assertNotNull(queryId, "Query id was not assigned in time");
-                assertTrue(waitForQueryToStart(queryId, 15), "Query did not start on the server in time");
+    private void testCancelQueryWithSessionValidation(Connection conn, String sessionId) throws Exception {
+        try (StatementImpl stmt = (StatementImpl) conn.createStatement()) {
+            stmt.getLocalSettings().setSessionId(sessionId);
+            stmt.setQueryTimeout(30); // safety net so a failed cancel cannot hang the test
+
+            final AtomicReference<Throwable> threadError = new AtomicReference<>();
+            final CountDownLatch started = new CountDownLatch(1);
+            Thread worker = new Thread(() -> {
+                started.countDown();
+                // Long-running query that only completes when killed.
+                try (ResultSet rs = stmt.executeQuery("SELECT count() FROM system.numbers_mt")) {
+                    rs.next();
+                } catch (Throwable t) {
+                    System.out.println("Error: " + t.getMessage());
+                    threadError.set(t);
+                }
+            });
+            worker.start();
+            started.await();
 
-                // Cancel from the main thread - must not throw SESSION_IS_LOCKED.
-                stmt.cancel();
+            String queryId = waitForQueryId(stmt, 15);
+            assertNotNull(queryId, "Query id was not assigned in time");
+            assertTrue(waitForQueryToStart(queryId, 15), "Query did not start on the server in time");
 
-                worker.join(TimeUnit.SECONDS.toMillis(20));
-                assertFalse(worker.isAlive(), "Query was not cancelled and is still running");
-            }
+            // Cancel from the main thread - must not throw SESSION_IS_LOCKED.
+            stmt.cancel();
+
+            worker.join(TimeUnit.SECONDS.toMillis(20));
+            assertFalse(worker.isAlive(), "Query was not cancelled and is still running");
         }
     }
 
@@ -799,7 +812,12 @@ public void testCancelInsertWithSession() throws Exception {
         // Regression test for #2690 covering a long-running INSERT executed inside a session.
         String tableName = getDatabase() + ".cancel_insert_with_session";
         String sessionId = "test-session-" + UUID.randomUUID();
-        try (Connection conn = getJdbcConnection(Map.of(ASYNC_INSERT_SETTING_KEY, ServerSettings.OFF))) {
+        Properties properties = new Properties();
+        properties.put(ASYNC_INSERT_SETTING_KEY, ServerSettings.OFF);
+        Session session = new Session();
+        session.setSessionId(sessionId);
+        session.applyTo(properties);
+        try (Connection conn = getJdbcConnection(properties)) {
             try (Statement setup = conn.createStatement()) {
                 setup.execute("DROP TABLE IF EXISTS " + tableName);
                 setup.execute("CREATE TABLE " + tableName + " (num UInt64) ENGINE = MergeTree ORDER BY ()");