From 797520d58659b5d2d842b0a981fe9eb5f2c80571 Mon Sep 17 00:00:00 2001 From: Sergey Chernov Date: Thu, 15 Jan 2026 06:51:41 -0800 Subject: [PATCH 1/3] added check for unknown parameters to warn user about misconfiguration --- .../r2dbc/spi/test/R2DBCTestKitImplTest.java | 2 +- .../client/api/ClientConfigProperties.java | 23 ++++- .../client/api/ServerException.java | 2 + .../com/clickhouse/client/ClientTests.java | 92 ++++++++++++++----- .../com/clickhouse/jdbc/DriverProperties.java | 10 ++ .../jdbc/internal/JdbcConfiguration.java | 31 +++++-- .../java/com/clickhouse/jdbc/DriverTest.java | 64 ++++++++++++- .../jdbc/internal/JdbcConfigurationTest.java | 43 ++++----- 8 files changed, 210 insertions(+), 57 deletions(-) diff --git a/clickhouse-r2dbc/src/test/java/com/clickhouse/r2dbc/spi/test/R2DBCTestKitImplTest.java b/clickhouse-r2dbc/src/test/java/com/clickhouse/r2dbc/spi/test/R2DBCTestKitImplTest.java index 38f8b6ace..510bd614f 100644 --- a/clickhouse-r2dbc/src/test/java/com/clickhouse/r2dbc/spi/test/R2DBCTestKitImplTest.java +++ b/clickhouse-r2dbc/src/test/java/com/clickhouse/r2dbc/spi/test/R2DBCTestKitImplTest.java @@ -100,7 +100,7 @@ private static JdbcTemplate jdbcTemplate(String database) throws SQLException { .map(Duration::toMillis).orElse(0L)); ZoneId zoneId = ZoneId.systemDefault(); - source.addDataSourceProperty("serverTimezone", TimeZone.getTimeZone(zoneId).getID()); + source.addDataSourceProperty("server_time_zone", TimeZone.getTimeZone(zoneId).getID()); return new JdbcTemplate(source); } diff --git a/client-v2/src/main/java/com/clickhouse/client/api/ClientConfigProperties.java b/client-v2/src/main/java/com/clickhouse/client/api/ClientConfigProperties.java index 131fc0f20..cd7e9ac2c 100644 --- a/client-v2/src/main/java/com/clickhouse/client/api/ClientConfigProperties.java +++ b/client-v2/src/main/java/com/clickhouse/client/api/ClientConfigProperties.java @@ -182,6 +182,12 @@ public Object parseValue(String value) { * SNI SSL parameter that will be set for each outbound SSL socket. */ SSL_SOCKET_SNI("ssl_socket_sni", String.class,""), + + /** + * Prefix for custom settings. Should be aligned with server configuration. + * See ClickHouse Docs + */ + CUSTOM_SETTINGS_PREFIX("clickhouse_setting_", String.class, "custom_"), ; private static final Logger LOG = LoggerFactory.getLogger(ClientConfigProperties.class); @@ -225,6 +231,8 @@ public T getDefObjVal() { // Key used to identify default value in configuration map public static final String DEFAULT_KEY = "_default_"; + public static final String NO_THROW_ON_UNKNOWN_CONFIG = "no_throw_on_unknown_config"; + public static String serverSetting(String key) { return SERVER_SETTING_PREFIX + key; } @@ -334,14 +342,27 @@ public static Map parseConfigMap(Map configMap) } } + final String customSettingsPrefix = configMap.getOrDefault(ClientConfigProperties.CUSTOM_SETTINGS_PREFIX.getKey(), + CUSTOM_SETTINGS_PREFIX.getDefaultValue()); + if (customSettingsPrefix == null || customSettingsPrefix.isEmpty()) { + throw new ClientException(ClientConfigProperties.CUSTOM_SETTINGS_PREFIX.getKey() + " must be not-blank"); + } for (String key : new HashSet<>(tmpMap.keySet())) { if (key.startsWith(HTTP_HEADER_PREFIX) || key.startsWith(SERVER_SETTING_PREFIX)) { parsedConfig.put(key, tmpMap.remove(key)); + } else if (key.startsWith(customSettingsPrefix)) { + parsedConfig.put(serverSetting(key), tmpMap.remove(key)); } } + tmpMap.remove(ClientConfigProperties.NO_THROW_ON_UNKNOWN_CONFIG); if (!tmpMap.isEmpty()) { - LOG.warn("Unknown and unmapped config properties: {}", tmpMap); + String msg = "Unknown and unmapped config properties: " + tmpMap.keySet(); + if (configMap.containsKey(NO_THROW_ON_UNKNOWN_CONFIG)) { + LOG.warn(msg); + } else { + throw new ClientMisconfigurationException(msg); + } } return parsedConfig; diff --git a/client-v2/src/main/java/com/clickhouse/client/api/ServerException.java b/client-v2/src/main/java/com/clickhouse/client/api/ServerException.java index 0c6ed7574..53dd43429 100644 --- a/client-v2/src/main/java/com/clickhouse/client/api/ServerException.java +++ b/client-v2/src/main/java/com/clickhouse/client/api/ServerException.java @@ -6,6 +6,8 @@ public class ServerException extends ClickHouseException { public static final int TABLE_NOT_FOUND = 60; + public static final int UNKNOWN_SETTING = 115; + private final int code; private final int transportProtocolCode; diff --git a/client-v2/src/test/java/com/clickhouse/client/ClientTests.java b/client-v2/src/test/java/com/clickhouse/client/ClientTests.java index 60f2bc59b..e95da4705 100644 --- a/client-v2/src/test/java/com/clickhouse/client/ClientTests.java +++ b/client-v2/src/test/java/com/clickhouse/client/ClientTests.java @@ -4,8 +4,9 @@ import com.clickhouse.client.api.ClientConfigProperties; import com.clickhouse.client.api.ClientException; import com.clickhouse.client.api.ClientFaultCause; +import com.clickhouse.client.api.ClientMisconfigurationException; import com.clickhouse.client.api.ConnectionReuseStrategy; -import com.clickhouse.client.api.command.CommandResponse; +import com.clickhouse.client.api.ServerException; import com.clickhouse.client.api.enums.Protocol; import com.clickhouse.client.api.insert.InsertSettings; import com.clickhouse.client.api.internal.ClickHouseLZ4OutputStream; @@ -17,8 +18,6 @@ import com.clickhouse.client.api.query.QuerySettings; import com.clickhouse.client.api.query.Records; import com.clickhouse.client.config.ClickHouseClientOption; -import com.clickhouse.client.query.QueryTests; -import com.clickhouse.data.ClickHouseColumn; import com.clickhouse.data.ClickHouseFormat; import com.clickhouse.data.ClickHouseVersion; import org.apache.commons.lang3.RandomStringUtils; @@ -76,30 +75,44 @@ public void testAddSecureEndpoint(Client client) { } } - @DataProvider + @DataProvider(name = "secureClientProvider") public static Object[][] secureClientProvider() throws Exception { ClickHouseNode node = ClickHouseServerForTest.getClickHouseNode(ClickHouseProtocol.HTTP, true, ClickHouseNode.builder() .addOption(ClickHouseClientOption.SSL_MODE.getKey(), "none") .addOption(ClickHouseClientOption.SSL.getKey(), "true").build()); + + Client client1; + Client client2; + try { + client1 = new Client.Builder() + .addEndpoint("https://" + node.getHost() + ":" + node.getPort()) + .setUsername("default") + .setPassword(ClickHouseServerForTest.getPassword()) + .setRootCertificate("containers/clickhouse-server/certs/localhost.crt") + .build(); + + client2 = new Client.Builder() + .addEndpoint(Protocol.HTTP, node.getHost(), node.getPort(), true) + .setUsername("default") + .setPassword(ClickHouseServerForTest.getPassword()) + .setRootCertificate("containers/clickhouse-server/certs/localhost.crt") + .setClientKey("some_user.key") + .setClientCertificate("some_user.crt") + .build(); + + } catch (Exception e) { + e.printStackTrace(); + throw new RuntimeException(e); + } + + return new Client[][]{ { - new Client.Builder() - .addEndpoint("https://" + node.getHost() + ":" + node.getPort()) - .setUsername("default") - .setPassword("") - .setRootCertificate("containers/clickhouse-server/certs/localhost.crt") - .build() + client1 }, { - new Client.Builder() - .addEndpoint(Protocol.HTTP, node.getHost(), node.getPort(), true) - .setUsername("default") - .setPassword("") - .setRootCertificate("containers/clickhouse-server/certs/localhost.crt") - .setClientKey("user.key") - .setClientCertificate("user.crt") - .build() + client2 } }; } @@ -223,7 +236,7 @@ public void testDefaultSettings() { Assert.assertEquals(config.get(p.getKey()), p.getDefaultValue(), "Default value doesn't match"); } } - Assert.assertEquals(config.size(), 32); // to check everything is set. Increment when new added. + Assert.assertEquals(config.size(), 33); // to check everything is set. Increment when new added. } try (Client client = new Client.Builder() @@ -256,7 +269,7 @@ public void testDefaultSettings() { .setSocketSndbuf(100000) .build()) { Map config = client.getConfiguration(); - Assert.assertEquals(config.size(), 33); // to check everything is set. Increment when new added. + Assert.assertEquals(config.size(), 34); // to check everything is set. Increment when new added. Assert.assertEquals(config.get(ClientConfigProperties.DATABASE.getKey()), "mydb"); Assert.assertEquals(config.get(ClientConfigProperties.MAX_EXECUTION_TIME.getKey()), "10"); Assert.assertEquals(config.get(ClientConfigProperties.COMPRESSION_LZ4_UNCOMPRESSED_BUF_SIZE.getKey()), "300000"); @@ -323,7 +336,7 @@ public void testWithOldDefaults() { Assert.assertEquals(config.get(p.getKey()), p.getDefaultValue(), "Default value doesn't match"); } } - Assert.assertEquals(config.size(), 32); // to check everything is set. Increment when new added. + Assert.assertEquals(config.size(), 33); // to check everything is set. Increment when new added. } } @@ -456,6 +469,43 @@ public void testInvalidEndpoint() { } } + @Test(groups = {"integration"}) + public void testUnknownClientSettings() throws Exception { + try (Client client = newClient().setOption("unknown_setting", "value").build()) { + Assert.fail("Exception expected"); + } catch (Exception ex) { + Assert.assertTrue(ex instanceof ClientMisconfigurationException); + Assert.assertTrue(ex.getMessage().contains("unknown_setting")); + } + + try (Client client = newClient().setOption(ClientConfigProperties.NO_THROW_ON_UNKNOWN_CONFIG, "what ever").setOption("unknown_setting", "value").build()) { + Assert.assertTrue(client.ping()); + } + + try (Client client = newClient().setOption(ClientConfigProperties.SERVER_SETTING_PREFIX + "unknown_setting", "value").build()) { + try { + client.execute("SELECT 1"); + Assert.fail("Exception expected"); + } catch (ServerException e) { + Assert.assertEquals(e.getCode(), ServerException.UNKNOWN_SETTING); + } + } + + try (Client client = newClient().setOption(ClientConfigProperties.HTTP_HEADER_PREFIX + "unknown_setting", "value").build()) { + Assert.assertTrue(client.ping()); + } + } + + @Test(groups = {"integration"}) + public void testInvalidConfig() { + try { + newClient().setOption(ClientConfigProperties.CUSTOM_SETTINGS_PREFIX.getKey(), "").build(); + Assert.fail("exception expected"); + } catch (ClientException e) { + Assert.assertTrue(e.getMessage().contains(ClientConfigProperties.CUSTOM_SETTINGS_PREFIX.getKey())); + } + } + public boolean isVersionMatch(String versionExpression, Client client) { List serverVersion = client.queryAll("SELECT version()"); return ClickHouseVersion.of(serverVersion.get(0).getString(1)).check(versionExpression); diff --git a/jdbc-v2/src/main/java/com/clickhouse/jdbc/DriverProperties.java b/jdbc-v2/src/main/java/com/clickhouse/jdbc/DriverProperties.java index 7c8732187..0e3d0b01b 100644 --- a/jdbc-v2/src/main/java/com/clickhouse/jdbc/DriverProperties.java +++ b/jdbc-v2/src/main/java/com/clickhouse/jdbc/DriverProperties.java @@ -1,5 +1,6 @@ package com.clickhouse.jdbc; +import com.clickhouse.client.api.ClientConfigProperties; import com.clickhouse.client.api.internal.ServerSettings; import java.util.Arrays; @@ -98,4 +99,13 @@ public String getDefaultValue() { public List getChoices() { return choices; } + + + public static String serverSetting(String key) { + return ClientConfigProperties.serverSetting(key); + } + + public static String httpHeader(String key) { + return ClientConfigProperties.httpHeader(key); + } } diff --git a/jdbc-v2/src/main/java/com/clickhouse/jdbc/internal/JdbcConfiguration.java b/jdbc-v2/src/main/java/com/clickhouse/jdbc/internal/JdbcConfiguration.java index 2581cd545..d1b1889d6 100644 --- a/jdbc-v2/src/main/java/com/clickhouse/jdbc/internal/JdbcConfiguration.java +++ b/jdbc-v2/src/main/java/com/clickhouse/jdbc/internal/JdbcConfiguration.java @@ -7,6 +7,7 @@ import com.clickhouse.jdbc.Driver; import com.clickhouse.jdbc.DriverProperties; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -22,6 +23,7 @@ import java.util.List; import java.util.Map; import java.util.Properties; +import java.util.Set; import java.util.regex.Pattern; import java.util.stream.Collectors; @@ -57,6 +59,17 @@ public boolean isIgnoreUnsupportedRequests() { return isIgnoreUnsupportedRequests; } + private static final Set DRIVER_PROP_KEYS; + static { + ImmutableSet.Builder driverPropertiesMapBuilder = ImmutableSet.builder(); + for (DriverProperties prop : DriverProperties.values()) { + driverPropertiesMapBuilder.add(prop.getKey()); + } + + DRIVER_PROP_KEYS = driverPropertiesMapBuilder.build(); + } + + /** * Parses URL to get property and target host. * Properties that are passed in the {@code info} parameter will override that are set in the {@code url}. @@ -249,6 +262,12 @@ private void initProperties(Map urlProperties, Properties provid // Copy provided properties Map props = new HashMap<>(); + // Set driver properties defaults (client will do the same) + for (DriverProperties prop : DriverProperties.values()) { + if (prop.getDefaultValue() != null) { + props.put(prop.getKey(), prop.getDefaultValue()); + } + } for (Map.Entry entry : providedProperties.entrySet()) { if (entry.getKey() instanceof String && entry.getValue() instanceof String) { props.put((String) entry.getKey(), (String) entry.getValue()); @@ -273,7 +292,12 @@ private void initProperties(Map urlProperties, Properties provid DriverPropertyInfo propertyInfo = new DriverPropertyInfo(prop.getKey(), prop.getValue()); propertyInfo.description = "(User Defined)"; propertyInfos.put(prop.getKey(), propertyInfo); - clientProperties.put(prop.getKey(), prop.getValue()); + + if (DRIVER_PROP_KEYS.contains(prop.getKey())) { + driverProperties.put(prop.getKey(), prop.getValue()); + } else { + clientProperties.put(prop.getKey(), prop.getValue()); + } } // Fill list of client properties information, add not specified properties (doesn't affect client properties) @@ -294,11 +318,6 @@ private void initProperties(Map urlProperties, Properties provid propertyInfo = new DriverPropertyInfo(driverProp.getKey(), driverProp.getDefaultValue()); propertyInfos.put(driverProp.getKey(), propertyInfo); } - - String value = clientProperties.get(driverProp.getKey()); - if (value != null) { - driverProperties.put(driverProp.getKey(), value); - } } listOfProperties = propertyInfos.values().stream().sorted(Comparator.comparing(o -> o.name)).collect(Collectors.toList()); diff --git a/jdbc-v2/src/test/java/com/clickhouse/jdbc/DriverTest.java b/jdbc-v2/src/test/java/com/clickhouse/jdbc/DriverTest.java index 7e1fb77df..217509344 100644 --- a/jdbc-v2/src/test/java/com/clickhouse/jdbc/DriverTest.java +++ b/jdbc-v2/src/test/java/com/clickhouse/jdbc/DriverTest.java @@ -1,17 +1,22 @@ package com.clickhouse.jdbc; import com.clickhouse.client.api.ClientConfigProperties; +import com.clickhouse.client.api.ClientMisconfigurationException; +import com.clickhouse.client.api.ServerException; import org.testng.Assert; import org.testng.annotations.DataProvider; import org.testng.annotations.Test; +import java.sql.Connection; import java.sql.DriverManager; import java.sql.DriverPropertyInfo; import java.sql.SQLException; +import java.sql.Statement; import java.util.HashMap; import java.util.Map; import java.util.Properties; +import static org.testng.Assert.assertEquals; import static org.testng.AssertJUnit.assertTrue; @@ -62,16 +67,16 @@ public void testGetPropertyInfo(String url, Properties props, Map Date: Fri, 16 Jan 2026 10:27:25 -0800 Subject: [PATCH 2/3] Added more clear tests for custom properties. More as reference code --- .../client/api/ClientConfigProperties.java | 2 +- .../com/clickhouse/client/ClientTests.java | 56 +++++++++++++------ .../com/clickhouse/jdbc/ConnectionTest.java | 17 ++++++ 3 files changed, 58 insertions(+), 17 deletions(-) diff --git a/client-v2/src/main/java/com/clickhouse/client/api/ClientConfigProperties.java b/client-v2/src/main/java/com/clickhouse/client/api/ClientConfigProperties.java index cd7e9ac2c..910aab862 100644 --- a/client-v2/src/main/java/com/clickhouse/client/api/ClientConfigProperties.java +++ b/client-v2/src/main/java/com/clickhouse/client/api/ClientConfigProperties.java @@ -187,7 +187,7 @@ public Object parseValue(String value) { * Prefix for custom settings. Should be aligned with server configuration. * See ClickHouse Docs */ - CUSTOM_SETTINGS_PREFIX("clickhouse_setting_", String.class, "custom_"), + CUSTOM_SETTINGS_PREFIX("custom_settings_prefix", String.class, "custom_"), ; private static final Logger LOG = LoggerFactory.getLogger(ClientConfigProperties.class); diff --git a/client-v2/src/test/java/com/clickhouse/client/ClientTests.java b/client-v2/src/test/java/com/clickhouse/client/ClientTests.java index e95da4705..e9b450e88 100644 --- a/client-v2/src/test/java/com/clickhouse/client/ClientTests.java +++ b/client-v2/src/test/java/com/clickhouse/client/ClientTests.java @@ -31,6 +31,8 @@ import java.io.ByteArrayInputStream; import java.net.ConnectException; import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.List; @@ -119,27 +121,49 @@ public static Object[][] secureClientProvider() throws Exception { @Test(groups = {"integration"}) public void testRawSettings() { - Client client = newClient() + try (Client client = newClient() .setOption("custom_setting_1", "value_1") - .build(); + .build()) { - client.execute("SELECT 1"); + client.execute("SELECT 1"); - QuerySettings querySettings = new QuerySettings(); - querySettings.serverSetting("session_timezone", "Europe/Zurich"); + QuerySettings querySettings = new QuerySettings(); + querySettings.serverSetting("session_timezone", "Europe/Zurich"); - try (Records response = - client.queryRecords("SELECT timeZone(), serverTimeZone()", querySettings).get(10, TimeUnit.SECONDS)) { + try (Records response = + client.queryRecords("SELECT timeZone(), serverTimeZone()", querySettings).get(10, TimeUnit.SECONDS)) { - response.forEach(record -> { - System.out.println(record.getString(1) + " " + record.getString(2)); - Assert.assertEquals("Europe/Zurich", record.getString(1)); - Assert.assertEquals("UTC", record.getString(2)); - }); - } catch (Exception e) { - Assert.fail(e.getMessage()); - } finally { - client.close(); + response.forEach(record -> { + System.out.println(record.getString(1) + " " + record.getString(2)); + Assert.assertEquals("Europe/Zurich", record.getString(1)); + Assert.assertEquals("UTC", record.getString(2)); + }); + } catch (Exception e) { + Assert.fail(e.getMessage()); + } + } + } + + @Test + public void testCustomSettings() { + if (!isCloud()) { + return; // no custom parameters on cloud instance + } + final String CLIENT_OPTION = "custom_client_option"; // prefix should be known from server config + try (Client client = newClient().serverSetting(CLIENT_OPTION, "opt1").build()) { + + final List clientOption = client.queryAll("SELECT getSetting({option_name:String})", + Collections.singletonMap("option_name", CLIENT_OPTION)); + + Assert.assertEquals(clientOption.get(0).getString(1), "opt1"); + + QuerySettings querySettings = new QuerySettings(); + querySettings.serverSetting(CLIENT_OPTION, "opt2"); + + final List requestOption = client.queryAll("SELECT getSetting({option_name:String})", + Collections.singletonMap("option_name", CLIENT_OPTION), querySettings); + + Assert.assertEquals(requestOption.get(0).getString(1), "opt2"); } } diff --git a/jdbc-v2/src/test/java/com/clickhouse/jdbc/ConnectionTest.java b/jdbc-v2/src/test/java/com/clickhouse/jdbc/ConnectionTest.java index d149357e9..aaf4be005 100644 --- a/jdbc-v2/src/test/java/com/clickhouse/jdbc/ConnectionTest.java +++ b/jdbc-v2/src/test/java/com/clickhouse/jdbc/ConnectionTest.java @@ -1092,4 +1092,21 @@ public void testEndpointUrlPathIsPreserved() throws Exception { mockServer.stop(); } } + + @Test(groups = {"integration"}) + public void testCustomParameters() throws Exception { + if (isCloud()) { + return; // no custom settings on cloud instance + } + Properties properties = new Properties(); + properties.put(DriverProperties.serverSetting("custom_option"), "opt1"); + + try (Connection connection = getJdbcConnection(properties)) { + try (Statement stmt = connection.createStatement(); ResultSet rs = stmt.executeQuery("SELECT getSetting('custom_option')")){ + + assertTrue(rs.next()); + assertEquals(rs.getString(1), "opt1"); + } + } + } } From 1bb82e599dfab215c6d9d229b663a496ce05bf5e Mon Sep 17 00:00:00 2001 From: Sergey Chernov Date: Fri, 16 Jan 2026 10:47:29 -0800 Subject: [PATCH 3/3] Fixed test condition for cloud --- client-v2/src/test/java/com/clickhouse/client/ClientTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client-v2/src/test/java/com/clickhouse/client/ClientTests.java b/client-v2/src/test/java/com/clickhouse/client/ClientTests.java index e9b450e88..4da27a216 100644 --- a/client-v2/src/test/java/com/clickhouse/client/ClientTests.java +++ b/client-v2/src/test/java/com/clickhouse/client/ClientTests.java @@ -146,7 +146,7 @@ public void testRawSettings() { @Test public void testCustomSettings() { - if (!isCloud()) { + if (isCloud()) { return; // no custom parameters on cloud instance } final String CLIENT_OPTION = "custom_client_option"; // prefix should be known from server config