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
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 <a href="https://clickhouse.com/docs/operations/settings/query-level#custom_settings">ClickHouse Docs</a>
*/
CUSTOM_SETTINGS_PREFIX("custom_settings_prefix", String.class, "custom_"),
;

private static final Logger LOG = LoggerFactory.getLogger(ClientConfigProperties.class);
Expand Down Expand Up @@ -225,6 +231,8 @@ public <T> 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;
}
Expand Down Expand Up @@ -334,14 +342,27 @@ public static Map<String, Object> parseConfigMap(Map<String, String> 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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
148 changes: 111 additions & 37 deletions client-v2/src/test/java/com/clickhouse/client/ClientTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -32,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;
Expand Down Expand Up @@ -76,57 +77,93 @@ 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
}
};
}

@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
Copy link

Choose a reason for hiding this comment

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

Missing integration test group annotation

Low Severity

The new testCustomSettings() test method uses @Test without groups = {"integration"}, while all other tests in this file that interact with the server have this annotation. The test makes actual server calls via client.queryAll(), so it requires a running ClickHouse instance. Without the integration group annotation, this test may run during unit test phases without a server available, causing unexpected test failures.

Fix in Cursor Fix in Web

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<GenericRecord> 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<GenericRecord> requestOption = client.queryAll("SELECT getSetting({option_name:String})",
Collections.singletonMap("option_name", CLIENT_OPTION), querySettings);

Assert.assertEquals(requestOption.get(0).getString(1), "opt2");
}
}

Expand Down Expand Up @@ -223,7 +260,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()
Expand Down Expand Up @@ -256,7 +293,7 @@ public void testDefaultSettings() {
.setSocketSndbuf(100000)
.build()) {
Map<String, String> 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");
Expand Down Expand Up @@ -323,7 +360,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.
}
}

Expand Down Expand Up @@ -456,6 +493,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<GenericRecord> serverVersion = client.queryAll("SELECT version()");
return ClickHouseVersion.of(serverVersion.get(0).getString(1)).check(versionExpression);
Expand Down
10 changes: 10 additions & 0 deletions jdbc-v2/src/main/java/com/clickhouse/jdbc/DriverProperties.java
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -98,4 +99,13 @@ public String getDefaultValue() {
public List<String> getChoices() {
return choices;
}


public static String serverSetting(String key) {
return ClientConfigProperties.serverSetting(key);
}

public static String httpHeader(String key) {
return ClientConfigProperties.httpHeader(key);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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;

Expand Down Expand Up @@ -57,6 +59,17 @@ public boolean isIgnoreUnsupportedRequests() {
return isIgnoreUnsupportedRequests;
}

private static final Set<String> DRIVER_PROP_KEYS;
static {
ImmutableSet.Builder<String> 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}.
Expand Down Expand Up @@ -249,6 +262,12 @@ private void initProperties(Map<String, String> urlProperties, Properties provid

// Copy provided properties
Map<String, String> 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<Object, Object> entry : providedProperties.entrySet()) {
if (entry.getKey() instanceof String && entry.getValue() instanceof String) {
props.put((String) entry.getKey(), (String) entry.getValue());
Expand All @@ -273,7 +292,12 @@ private void initProperties(Map<String, String> 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)
Expand All @@ -294,11 +318,6 @@ private void initProperties(Map<String, String> 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());
Expand Down
Loading
Loading