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 @@ -85,6 +85,25 @@ public final class ClickHouseUtils {
public static final String VARIABLE_PREFIX = "{{";
public static final String VARIABLE_SUFFIX = "}}";

/**
* Parses a boolean value from string. Accepts "true"/"false" and "1"/"0".
*
* @param value string value
* @return parsed boolean, or {@code false} if value is null or unrecognized
*/
public static boolean parseBoolean(String value) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please move this method to client-v2
I suggest utilizing ValueConverters.

Copy link
Contributor

Choose a reason for hiding this comment

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

PS: clickhouse-data is an old package that we may deprecate so V2 should minimize dependencies on it. Besides clickhouse-data relates to client-v1 and this method is not used there.

if (value == null) {
return false;
}
if ("1".equals(value)) {
return true;
}
if ("0".equals(value)) {
return false;
}
return Boolean.parseBoolean(value);
}

private static <T> T findFirstService(Class<? extends T> serviceInterface) {
ClickHouseChecker.nonNull(serviceInterface, "serviceInterface");

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,20 @@
import org.testng.annotations.Test;

public class ClickHouseUtilsTest {
@Test(groups = { "unit" })
public void testParseBoolean() {
Assert.assertFalse(ClickHouseUtils.parseBoolean(null));

Assert.assertTrue(ClickHouseUtils.parseBoolean("1"));
Assert.assertFalse(ClickHouseUtils.parseBoolean("0"));

Assert.assertTrue(ClickHouseUtils.parseBoolean("true"));
Assert.assertFalse(ClickHouseUtils.parseBoolean("false"));

// unrecognized values should be parsed as false
Copy link
Member

Choose a reason for hiding this comment

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

@chernser shouldn't we throw an exception for unrecognized values?

Copy link
Author

Choose a reason for hiding this comment

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

If there is an unrecognized value, then we should also follow the behavior pattern of other configs in this case.

For example, the default_query_settings config accepts a set of settings, else null by default. Now, if someone sets a random setting that is not used by Clickhouse then does it throw an error, or simply ignores that?

Similarly, jdbc_sql_parser accepts 1 of 3 enum constants. Does it throw an error if any other value is passed?

If we throw an exception only for boolean types and simply ignore for other types, then it will create inconsistent behaviour. Please let me know your thoughts on this.

Copy link
Contributor

Choose a reason for hiding this comment

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

@kailash360 both mentioned settings are not validated but should.
jdbc_sql_parser will throw exception later while creating connection. That is not correct and we will fix it.
Most Client configurations validated in the Builder.

Please add throwing exception if boolean value is not what we expect.

Thanks!

Assert.assertFalse(ClickHouseUtils.parseBoolean("yes"));
}

@Test(groups = { "unit" })
public void testCreateTempFile() throws IOException {
File f = ClickHouseUtils.createTempFile(null, null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

import java.util.ArrayList;
import java.util.Arrays;
import com.clickhouse.data.ClickHouseUtils;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
Expand Down Expand Up @@ -270,6 +271,10 @@ public static List<String> valuesFromCommaSeparated(String value) {
.collect(Collectors.toList());
}

/**
* Parses a string value into the configured property type.
* Boolean values accept "true"/"false" and "1"/"0".
*/
public Object parseValue(String value) {
if (value == null) {
return null;
Expand All @@ -280,9 +285,7 @@ public Object parseValue(String value) {
}

if (valueType.equals(Boolean.class)) {
if (value.equals("1")) return true;
if (value.equals("0")) return false;
return Boolean.parseBoolean(value);
return ClickHouseUtils.parseBoolean(value);
}

if (valueType.equals(Integer.class)) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.clickhouse.client.api.data_formats.internal;

import com.clickhouse.client.api.Client;
import com.clickhouse.data.ClickHouseUtils;
import com.clickhouse.client.api.ClientException;
import com.clickhouse.client.api.serde.POJOFieldDeserializer;
import com.clickhouse.data.ClickHouseAggregateFunction;
Expand Down Expand Up @@ -817,7 +818,7 @@ public static boolean convertToBoolean(Object value) {
} else if (value instanceof Number) {
return ((Number) value).longValue() != 0;
} else if (value instanceof String) {
return Boolean.parseBoolean((String) value);
return ClickHouseUtils.parseBoolean((String) value);
} else {
throw new IllegalArgumentException("Cannot convert " + value + " to Boolean");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import java.math.BigDecimal;
import java.math.BigInteger;
import java.net.MalformedURLException;
import com.clickhouse.data.ClickHouseUtils;
import java.net.URL;
import java.sql.Date;
import java.sql.Time;
Expand Down Expand Up @@ -137,7 +138,7 @@ public byte[] convertStringToBytes(Object value) {
}

public boolean convertStringToBoolean(Object value) {
return Boolean.parseBoolean((String) value);
return ClickHouseUtils.parseBoolean((String) value);
}

public byte convertStringToByte(Object value) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,28 @@
package com.clickhouse.client.api;

import org.testng.Assert;
import org.testng.annotations.Test;

public class ClientConfigPropertiesTest {

@Test
public void testBooleanParseValue() {
// HTTP_USE_BASIC_AUTH is defined as Boolean.class
Object v1 = ClientConfigProperties.HTTP_USE_BASIC_AUTH.parseValue("1");
Assert.assertTrue(v1 instanceof Boolean && (Boolean) v1);

Object v0 = ClientConfigProperties.HTTP_USE_BASIC_AUTH.parseValue("0");
Assert.assertTrue(v0 instanceof Boolean && !((Boolean) v0));

Object vt = ClientConfigProperties.HTTP_USE_BASIC_AUTH.parseValue("true");
Assert.assertTrue(vt instanceof Boolean && (Boolean) vt);

Object vf = ClientConfigProperties.HTTP_USE_BASIC_AUTH.parseValue("false");
Assert.assertTrue(vf instanceof Boolean && !((Boolean) vf));
}
}
package com.clickhouse.client.api;


import org.testng.Assert;
import org.testng.annotations.Test;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package com.clickhouse.client.api.data_formats.internal;

import org.testng.Assert;
import org.testng.annotations.Test;

public class ValueConvertersTest {

@Test
public void testConvertStringToBoolean() {
ValueConverters vc = new ValueConverters();
Assert.assertTrue(vc.convertStringToBoolean("1"));
Assert.assertFalse(vc.convertStringToBoolean("0"));
Assert.assertTrue(vc.convertStringToBoolean("true"));
Assert.assertFalse(vc.convertStringToBoolean("false"));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import com.clickhouse.client.api.ClientConfigProperties;
import com.clickhouse.client.api.http.ClickHouseHttpProto;
import com.clickhouse.data.ClickHouseDataType;
import com.clickhouse.data.ClickHouseUtils;
import com.clickhouse.jdbc.Driver;
import com.clickhouse.jdbc.DriverProperties;
import com.google.common.collect.ImmutableMap;
Expand Down Expand Up @@ -80,7 +81,8 @@ public boolean isIgnoreUnsupportedRequests() {
*/
public JdbcConfiguration(String url, Properties info) throws SQLException {
final Properties props = info == null ? new Properties() : info;
this.disableFrameworkDetection = Boolean.parseBoolean(props.getProperty("disable_frameworks_detection", "false"));
this.disableFrameworkDetection = ClickHouseUtils.parseBoolean(
props.getProperty("disable_frameworks_detection", "false"));
this.clientProperties = new HashMap<>();
this.driverProperties = new HashMap<>();

Expand All @@ -92,16 +94,19 @@ public JdbcConfiguration(String url, Properties info) throws SQLException {
initProperties(urlProperties, props);

// after initializing all properties - set final connection URL
boolean useSSLInfo = Boolean.parseBoolean(props.getProperty(DriverProperties.SECURE_CONNECTION.getKey(), "false"));
boolean useSSLUrlProperties = Boolean.parseBoolean(urlProperties.getOrDefault(DriverProperties.SECURE_CONNECTION.getKey(), "false"));
boolean useSSLInfo = ClickHouseUtils.parseBoolean(
props.getProperty(DriverProperties.SECURE_CONNECTION.getKey(), "false"));
boolean useSSLUrlProperties = ClickHouseUtils.parseBoolean(
urlProperties.getOrDefault(DriverProperties.SECURE_CONNECTION.getKey(), "false"));
boolean useSSL = useSSLInfo || useSSLUrlProperties;
String bearerToken = props.getProperty(ClientConfigProperties.BEARERTOKEN_AUTH.getKey(), null);
if (bearerToken != null) {
clientProperties.put(ClientConfigProperties.BEARERTOKEN_AUTH.getKey(), bearerToken);
}

this.connectionUrl = createConnectionURL(tmpConnectionUrl, useSSL);
this.isIgnoreUnsupportedRequests = Boolean.parseBoolean(getDriverProperty(DriverProperties.IGNORE_UNSUPPORTED_VALUES.getKey(), "false"));
this.isIgnoreUnsupportedRequests = ClickHouseUtils.parseBoolean(
getDriverProperty(DriverProperties.IGNORE_UNSUPPORTED_VALUES.getKey(), "false"));
}

/**
Expand Down Expand Up @@ -346,7 +351,7 @@ public Supplier<String> getQueryIdGenerator() {

public Boolean isSet(DriverProperties driverProp) {
String v = driverProperties.getOrDefault(driverProp.getKey(), driverProp.getDefaultValue());
return Boolean.parseBoolean(v);
return ClickHouseUtils.parseBoolean(v);
}

public Client.Builder applyClientProperties(Client.Builder builder) {
Expand Down Expand Up @@ -374,7 +379,7 @@ public boolean isBetaFeatureEnabled(DriverProperties prop) {

public boolean isFlagSet(DriverProperties prop) {
String value = driverProperties.getOrDefault(prop.getKey(), prop.getDefaultValue());
return Boolean.parseBoolean(value);
return ClickHouseUtils.parseBoolean(value);
}

private Map<ClickHouseDataType, Class<?>> defaultTypeHintMapping() {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,24 @@
package com.clickhouse.jdbc.internal;

import com.clickhouse.jdbc.DriverProperties;
import org.testng.Assert;
import org.testng.annotations.Test;

import java.sql.SQLException;
import java.util.Properties;

public class JdbcConfigurationTest {

@Test
public void testIgnoreUnsupportedRequestsParsing() throws SQLException {
Properties props = new Properties();
props.setProperty(DriverProperties.IGNORE_UNSUPPORTED_VALUES.getKey(), "1");
JdbcConfiguration cfg = new JdbcConfiguration("jdbc:clickhouse://localhost", props);
Assert.assertTrue(cfg.isIgnoreUnsupportedRequests());
}
}
package com.clickhouse.jdbc.internal;

import com.clickhouse.client.api.Client;
import com.clickhouse.client.api.ClientConfigProperties;

Expand Down
Loading