Skip to content

Fixes and improvements for multiple problems of jdbc_v2 and client_v2 #2302

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
chernser merged 2 commits into from
Apr 15, 2025
Merged

Fixes and improvements for multiple problems of jdbc_v2 and client_v2 #2302

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0a06285
improvements:
oleg-zinovev Apr 12, 2025
2e57790
removed:
oleg-zinovev Apr 15, 2025
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
26 changes: 25 additions & 1 deletion client-v2/src/main/java/com/clickhouse/client/api/internal/HttpAPIClientHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,11 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import javax.net.ssl.KeyManager;
import javax.net.ssl.SSLContext;
import javax.net.ssl.SSLException;
import javax.net.ssl.TrustManager;
import javax.net.ssl.X509TrustManager;
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
Expand All @@ -74,7 +77,10 @@
import java.net.URL;
import java.net.UnknownHostException;
import java.nio.charset.StandardCharsets;
import java.security.KeyManagementException;
import java.security.NoSuchAlgorithmException;
import java.security.SecureRandom;
import java.security.cert.X509Certificate;
import java.util.Base64;
import java.util.Collection;
import java.util.Collections;
Expand Down Expand Up @@ -380,7 +386,7 @@ public Exception readError(ClassicHttpResponse httpResponse) {

public ClassicHttpResponse executeRequest(ClickHouseNode server, Map<String, Object> requestConfig, LZ4Factory lz4Factory,
IOCallback<OutputStream> writeCallback) throws IOException {
if (timeToPoolVent.get() < System.currentTimeMillis()) {
if (poolControl != null && timeToPoolVent.get() < System.currentTimeMillis()) {
timeToPoolVent.set(System.currentTimeMillis() + POOL_VENT_TIMEOUT);
poolControl.closeExpired();
}
Expand Down Expand Up @@ -828,4 +834,22 @@ public long getTime() {
return count > 0 ? runningAverage / count : 0;
}
}

private static final class TrustAllManager implements X509TrustManager {

@Override
public void checkClientTrusted(X509Certificate[] chain, String authType) {
// ignore
}

@Override
public void checkServerTrusted(X509Certificate[] chain, String authType) {
// ignore
}

@Override
public X509Certificate[] getAcceptedIssuers() {
return new X509Certificate[0];
}
}
}
8 changes: 8 additions & 0 deletions client-v2/src/test/java/com/clickhouse/client/ClientTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.clickhouse.client;

import com.clickhouse.client.api.Client;
import com.clickhouse.client.api.ClientConfigProperties;
import com.clickhouse.client.api.ClientException;
import com.clickhouse.client.api.enums.Protocol;
import com.clickhouse.client.api.query.GenericRecord;
Expand Down Expand Up @@ -108,6 +109,13 @@ public void testPing() {
}
}

@Test
public void testPingUnpooled() {
try (Client client = newClient().enableConnectionPool(false).build()) {
Assert.assertTrue(client.ping());
}
}

@Test
public void testPingFailure() {
try (Client client = new Client.Builder()
Expand Down
15 changes: 9 additions & 6 deletions jdbc-v2/src/main/java/com/clickhouse/jdbc/Driver.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,11 @@

import com.clickhouse.client.api.ClientConfigProperties;
import com.clickhouse.client.config.ClickHouseClientOption;
import com.clickhouse.jdbc.internal.JdbcConfiguration;
import com.clickhouse.jdbc.internal.ExceptionUtils;
import com.clickhouse.jdbc.internal.JdbcConfiguration;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.io.InputStream;
import java.sql.Connection;
import java.sql.DriverManager;
import java.sql.DriverPropertyInfo;
Expand All @@ -33,11 +32,14 @@ public class Driver implements java.sql.Driver {
private final DataSourceImpl dataSource;

public static String frameworksDetected = null;

public static class FrameworksDetection {
private static final List<String> FRAMEWORKS_TO_DETECT = Arrays.asList("apache.spark");
static volatile String frameworksDetected = null;

private FrameworksDetection() {}
private FrameworksDetection() {
}

public static String getFrameworksDetected() {
if (frameworksDetected == null) {//Only detect frameworks once
Set<String> inferredFrameworks = new LinkedHashSet<>();
Expand Down Expand Up @@ -98,22 +100,21 @@ public Driver(DataSourceImpl dataSourceImpl) {

public static void load() {
try {
DriverManager.registerDriver(new Driver());
DriverManager.registerDriver(INSTANCE);
} catch (SQLException e) {
log.error("Failed to register ClickHouse JDBC driver", e);
}
}

public static void unload() {
try {
DriverManager.deregisterDriver(new Driver());
DriverManager.deregisterDriver(INSTANCE);
} catch (SQLException e) {
log.error("Failed to deregister ClickHouse JDBC driver", e);
}
}



@Override
public Connection connect(String url, Properties info) throws SQLException {
if (!acceptsURL(url)) {
Expand Down Expand Up @@ -163,4 +164,6 @@ public static String chSettingKey(String key) {
public java.util.logging.Logger getParentLogger() throws SQLFeatureNotSupportedException {
throw new SQLFeatureNotSupportedException("Method not supported", ExceptionUtils.SQL_STATE_FEATURE_NOT_SUPPORTED);
}

private static final Driver INSTANCE = new Driver();
}
88 changes: 76 additions & 12 deletions jdbc-v2/src/main/java/com/clickhouse/jdbc/PreparedStatementImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package com.clickhouse.jdbc;

import com.clickhouse.client.api.metadata.TableSchema;
import com.clickhouse.data.ClickHouseColumn;
import com.clickhouse.data.Tuple;
import com.clickhouse.jdbc.internal.ExceptionUtils;
import org.slf4j.Logger;
Expand All @@ -26,6 +28,7 @@
import java.sql.SQLFeatureNotSupportedException;
import java.sql.SQLType;
import java.sql.SQLXML;
import java.sql.Statement;
import java.sql.Time;
import java.sql.Timestamp;
import java.sql.Types;
Expand All @@ -42,7 +45,6 @@
import java.util.ArrayList;
import java.util.Calendar;
import java.util.Collection;
import java.util.GregorianCalendar;
import java.util.List;
import java.util.Map;

Expand All @@ -64,11 +66,12 @@ public class PreparedStatementImpl extends StatementImpl implements PreparedStat
String insertIntoSQL;

StatementType statementType;

public PreparedStatementImpl(ConnectionImpl connection, String sql) throws SQLException {
super(connection);
this.originalSql = sql.trim();
//Split the sql string into an array of strings around question mark tokens
this.sqlSegments = originalSql.split("\\?");
this.sqlSegments = splitStatement(originalSql);
this.statementType = parseStatementType(originalSql);

if (statementType == StatementType.INSERT) {
Expand All @@ -77,17 +80,11 @@ public PreparedStatementImpl(ConnectionImpl connection, String sql) throws SQLEx
}

//Create an array of objects to store the parameters
if (originalSql.contains("?")) {
int count = originalSql.length() - originalSql.replace("?", "").length();
this.parameters = new Object[count];
} else {
this.parameters = new Object[0];
}

this.parameters = new Object[sqlSegments.length - 1];
this.defaultCalendar = connection.defaultCalendar;
}

private String compileSql(String []segments) {
private String compileSql(String [] segments) {
StringBuilder sb = new StringBuilder();
for (int i = 0; i < segments.length; i++) {
sb.append(segments[i]);
Expand All @@ -98,6 +95,7 @@ private String compileSql(String []segments) {
LOG.trace("Compiled SQL: {}", sb);
return sb.toString();
}

@Override
public ResultSet executeQuery() throws SQLException {
checkClosed();
Expand Down Expand Up @@ -517,14 +515,14 @@ private static String encodeObject(Object x) throws SQLException {
} else if (x instanceof ZonedDateTime) {
return encodeObject(((ZonedDateTime) x).toInstant());
} else if (x instanceof Instant) {
return "fromUnixTimestamp64Nano(" + (((Instant) x).getEpochSecond() * 1_000_000_000L + ((Instant) x).getNano())+ ")";
return "fromUnixTimestamp64Nano(" + (((Instant) x).getEpochSecond() * 1_000_000_000L + ((Instant) x).getNano()) + ")";
} else if (x instanceof InetAddress) {
return "'" + ((InetAddress) x).getHostAddress() + "'";
} else if (x instanceof Array) {
StringBuilder listString = new StringBuilder();
listString.append("[");
int i = 0;
for (Object item : (Object[])((Array) x).getArray()) {
for (Object item : (Object[]) ((Array) x).getArray()) {
if (i > 0) {
listString.append(", ");
}
Expand Down Expand Up @@ -616,4 +614,70 @@ private static String encodeObject(Object x) throws SQLException {
private static String escapeString(String x) {
return x.replace("\\", "\\\\").replace("'", "\\'");//Escape single quotes
}

private static String [] splitStatement(String sql) {
List<String> segments = new ArrayList<>();
char[] chars = sql.toCharArray();
int segmentStart = 0;
for (int i = 0; i < chars.length; i++) {
char c = chars[i];
if (c == '\'' || c == '"' || c == '`') {
// string literal or identifier
i = skip(chars, i + 1, c, true);
} else if (c == '/' && lookahead(chars, i) == '*') {
// block comment
int end = sql.indexOf("*/", i);
if (end == -1) {
// missing comment end
break;
}
i = end + 1;
} else if (c == '#' || (c == '-' && lookahead(chars, i) == '-')) {
// line comment
i = skip(chars, i + 1, '\n', false);
} else if (c == '?') {
// question mark
segments.add(sql.substring(segmentStart, i));
segmentStart = i + 1;
}
}
if (segmentStart < chars.length) {
segments.add(sql.substring(segmentStart));
} else {
// add empty segment in case question mark was last char of sql
segments.add("");
}
return segments.toArray(new String[0]);
}

private static int skip(char[] chars, int from, char until, boolean escape) {
for (int i = from; i < chars.length; i++) {
char curr = chars[i];
if (escape) {
char next = lookahead(chars, i);
if ((curr == '\\' && (next == '\\' || next == until)) || (curr == until && next == until)) {
// should skip:
// 1) double \\ (backslash escaped with backslash)
// 2) \[until] ([until] char, escaped with backslash)
// 3) [until][until] ([until] char, escaped with [until])
i++;
continue;
}
}

if (curr == until) {
return i;
}
}
return chars.length;
}

private static char lookahead(char[] chars, int pos) {
pos = pos + 1;
if (pos >= chars.length) {
return '\0';
}
return chars[pos];
}

}
12 changes: 9 additions & 3 deletions jdbc-v2/src/main/java/com/clickhouse/jdbc/StatementImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,15 @@
import java.util.List;
import java.util.UUID;
import java.util.concurrent.TimeUnit;
import java.util.regex.Pattern;

public class StatementImpl implements Statement, JdbcV2Wrapper {
private static final Logger LOG = LoggerFactory.getLogger(StatementImpl.class);

ConnectionImpl connection;
private int queryTimeout;
protected boolean closed;
private ResultSetImpl currentResultSet;
protected ResultSetImpl currentResultSet;
private OperationMetrics metrics;
protected List<String> batch;
private String lastSql;
Expand Down Expand Up @@ -70,7 +71,7 @@ protected static StatementType parseStatementType(String sql) {
return StatementType.OTHER;
}

trimmedSql = trimmedSql.replaceAll("/\\*.*?\\*/", "").trim(); // remove comments
trimmedSql = BLOCK_COMMENT.matcher(trimmedSql).replaceAll("").trim(); // remove comments
String[] lines = trimmedSql.split("\n");
for (String line : lines) {
String trimmedLine = line.trim();
Expand Down Expand Up @@ -172,7 +173,10 @@ public ResultSetImpl executeQuery(String sql, QuerySettings settings) throws SQL
closePreviousResultSet();

QuerySettings mergedSettings = QuerySettings.merge(connection.getDefaultQuerySettings(), settings);

if (maxRows > 0) {
mergedSettings.setOption(ClientConfigProperties.serverSetting(ServerSettings.MAX_RESULT_ROWS), maxRows);
mergedSettings.setOption(ClientConfigProperties.serverSetting(ServerSettings.RESULT_OVERFLOW_MODE), "break");
}

if (mergedSettings.getQueryId() != null) {
lastQueryId = mergedSettings.getQueryId();
Expand Down Expand Up @@ -627,4 +631,6 @@ public String enquoteNCharLiteral(String val) throws SQLException {
public String getLastQueryId() {
return lastQueryId;
}

private static final Pattern BLOCK_COMMENT = Pattern.compile("/\\*.*?\\*/", Pattern.DOTALL);
}
42 changes: 42 additions & 0 deletions jdbc-v2/src/test/java/com/clickhouse/jdbc/PreparedStatementTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@

import java.sql.Array;
import java.sql.Connection;
import java.sql.JDBCType;
import java.sql.PreparedStatement;
import java.sql.ResultSet;
import java.sql.ResultSetMetaData;
import java.sql.Statement;
import java.sql.Types;
import java.util.Arrays;
Expand Down Expand Up @@ -449,4 +451,44 @@ void testMetabaseBug01() throws Exception {
}
}
}

@Test(groups = { "integration" })
void testStatementSplit() throws Exception {
try (Connection conn = getJdbcConnection()) {
try (Statement stmt = conn.createStatement()) {
stmt.execute("CREATE TABLE `with_complex_id` (`v?``1` Int32, \"v?\"\"2\" Int32,`v?\\`3` Int32, \"v?\\\"4\" Int32) ENGINE Memory;");
}
String insertQuery = "-- line comment1 ?\n"
+ "# line comment2 ?\n"
+ "#! line comment3 ?\n"
+ "/* block comment ? \n */"
+ "INSERT INTO `with_complex_id`(`v?``1`, \"v?\"\"2\",`v?\\`3`, \"v?\\\"4\") VALUES (?, ?, ?, ?);";
try (PreparedStatement stmt = conn.prepareStatement(insertQuery)) {
stmt.setInt(1, 1);
stmt.setInt(2, 2);
stmt.setInt(3, 3);
stmt.setInt(4, 4);
stmt.execute();
}
String selectQuery = "-- line comment ?\n"
+ "/* block comment ? \n */"
+ "SELECT `v?``1`, \"v?\"\"2\",`v?\\`3`, \"v?\\\"4\", 'test '' string1 ?', 'test \\' string2 ?', 'test string3 ?\\\\' FROM `with_complex_id` WHERE `v?``1` = ? AND \"v?\"\"2\" = ? AND `v?\\`3` = ? AND \"v?\\\"4\" = ?";
try (PreparedStatement stmt = conn.prepareStatement(selectQuery)) {
stmt.setInt(1, 1);
stmt.setInt(2, 2);
stmt.setInt(3, 3);
stmt.setInt(4, 4);
try (ResultSet rs = stmt.executeQuery()) {
assertTrue(rs.next());
assertEquals(rs.getInt(1), 1);
assertEquals(rs.getInt(2), 2);
assertEquals(rs.getInt(3), 3);
assertEquals(rs.getInt(4), 4);
assertEquals(rs.getString(5), "test ' string1 ?");
assertEquals(rs.getString(6), "test ' string2 ?");
assertEquals(rs.getString(7), "test string3 ?\\");
}
}
}
}
}
Loading
Loading