Skip to content

Commit 4856225

Browse files
chernserchernser
authored
Merge pull request #2302 from oleg-zinovev/fixes-1
Fixes and improvements for multiple problems of jdbc_v2 and client_v2
2 parents 3dde2a0 + 2e57790 commit 4856225

7 files changed

Lines changed: 188 additions & 22 deletions

File tree

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

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,11 @@
5858
import org.slf4j.Logger;
5959
import org.slf4j.LoggerFactory;
6060

61+
import javax.net.ssl.KeyManager;
6162
import javax.net.ssl.SSLContext;
6263
import javax.net.ssl.SSLException;
64+
import javax.net.ssl.TrustManager;
65+
import javax.net.ssl.X509TrustManager;
6366
import java.io.IOException;
6467
import java.io.InputStream;
6568
import java.io.OutputStream;
@@ -74,7 +77,10 @@
7477
import java.net.URL;
7578
import java.net.UnknownHostException;
7679
import java.nio.charset.StandardCharsets;
80+
import java.security.KeyManagementException;
7781
import java.security.NoSuchAlgorithmException;
82+
import java.security.SecureRandom;
83+
import java.security.cert.X509Certificate;
7884
import java.util.Base64;
7985
import java.util.Collection;
8086
import java.util.Collections;
@@ -380,7 +386,7 @@ public Exception readError(ClassicHttpResponse httpResponse) {
380386

381387
public ClassicHttpResponse executeRequest(ClickHouseNode server, Map<String, Object> requestConfig, LZ4Factory lz4Factory,
382388
IOCallback<OutputStream> writeCallback) throws IOException {
383-
if (timeToPoolVent.get() < System.currentTimeMillis()) {
389+
if (poolControl != null && timeToPoolVent.get() < System.currentTimeMillis()) {
384390
timeToPoolVent.set(System.currentTimeMillis() + POOL_VENT_TIMEOUT);
385391
poolControl.closeExpired();
386392
}
@@ -828,4 +834,22 @@ public long getTime() {
828834
return count > 0 ? runningAverage / count : 0;
829835
}
830836
}
837+
838+
private static final class TrustAllManager implements X509TrustManager {
839+
840+
@Override
841+
public void checkClientTrusted(X509Certificate[] chain, String authType) {
842+
// ignore
843+
}
844+
845+
@Override
846+
public void checkServerTrusted(X509Certificate[] chain, String authType) {
847+
// ignore
848+
}
849+
850+
@Override
851+
public X509Certificate[] getAcceptedIssuers() {
852+
return new X509Certificate[0];
853+
}
854+
}
831855
}

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package com.clickhouse.client;
22

33
import com.clickhouse.client.api.Client;
4+
import com.clickhouse.client.api.ClientConfigProperties;
45
import com.clickhouse.client.api.ClientException;
56
import com.clickhouse.client.api.enums.Protocol;
67
import com.clickhouse.client.api.query.GenericRecord;
@@ -108,6 +109,13 @@ public void testPing() {
108109
}
109110
}
110111

112+
@Test
113+
public void testPingUnpooled() {
114+
try (Client client = newClient().enableConnectionPool(false).build()) {
115+
Assert.assertTrue(client.ping());
116+
}
117+
}
118+
111119
@Test
112120
public void testPingFailure() {
113121
try (Client client = new Client.Builder()

‎jdbc-v2/src/main/java/com/clickhouse/jdbc/Driver.java‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,11 @@
33

44
import com.clickhouse.client.api.ClientConfigProperties;
55
import com.clickhouse.client.config.ClickHouseClientOption;
6-
import com.clickhouse.jdbc.internal.JdbcConfiguration;
76
import com.clickhouse.jdbc.internal.ExceptionUtils;
7+
import com.clickhouse.jdbc.internal.JdbcConfiguration;
88
import org.slf4j.Logger;
99
import org.slf4j.LoggerFactory;
1010

11-
import java.io.InputStream;
1211
import java.sql.Connection;
1312
import java.sql.DriverManager;
1413
import java.sql.DriverPropertyInfo;
@@ -33,11 +32,14 @@ public class Driver implements java.sql.Driver {
3332
private final DataSourceImpl dataSource;
3433

3534
public static String frameworksDetected = null;
35+
3636
public static class FrameworksDetection {
3737
private static final List<String> FRAMEWORKS_TO_DETECT = Arrays.asList("apache.spark");
3838
static volatile String frameworksDetected = null;
3939

40-
private FrameworksDetection() {}
40+
private FrameworksDetection() {
41+
}
42+
4143
public static String getFrameworksDetected() {
4244
if (frameworksDetected == null) {//Only detect frameworks once
4345
Set<String> inferredFrameworks = new LinkedHashSet<>();
@@ -98,22 +100,21 @@ public Driver(DataSourceImpl dataSourceImpl) {
98100

99101
public static void load() {
100102
try {
101-
DriverManager.registerDriver(new Driver());
103+
DriverManager.registerDriver(INSTANCE);
102104
} catch (SQLException e) {
103105
log.error("Failed to register ClickHouse JDBC driver", e);
104106
}
105107
}
106108

107109
public static void unload() {
108110
try {
109-
DriverManager.deregisterDriver(new Driver());
111+
DriverManager.deregisterDriver(INSTANCE);
110112
} catch (SQLException e) {
111113
log.error("Failed to deregister ClickHouse JDBC driver", e);
112114
}
113115
}
114116

115117

116-
117118
@Override
118119
public Connection connect(String url, Properties info) throws SQLException {
119120
if (!acceptsURL(url)) {
@@ -163,4 +164,6 @@ public static String chSettingKey(String key) {
163164
public java.util.logging.Logger getParentLogger() throws SQLFeatureNotSupportedException {
164165
throw new SQLFeatureNotSupportedException("Method not supported", ExceptionUtils.SQL_STATE_FEATURE_NOT_SUPPORTED);
165166
}
167+
168+
private static final Driver INSTANCE = new Driver();
166169
}

‎jdbc-v2/src/main/java/com/clickhouse/jdbc/PreparedStatementImpl.java‎

Lines changed: 76 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
package com.clickhouse.jdbc;
22

3+
import com.clickhouse.client.api.metadata.TableSchema;
4+
import com.clickhouse.data.ClickHouseColumn;
35
import com.clickhouse.data.Tuple;
46
import com.clickhouse.jdbc.internal.ExceptionUtils;
57
import org.slf4j.Logger;
@@ -26,6 +28,7 @@
2628
import java.sql.SQLFeatureNotSupportedException;
2729
import java.sql.SQLType;
2830
import java.sql.SQLXML;
31+
import java.sql.Statement;
2932
import java.sql.Time;
3033
import java.sql.Timestamp;
3134
import java.sql.Types;
@@ -42,7 +45,6 @@
4245
import java.util.ArrayList;
4346
import java.util.Calendar;
4447
import java.util.Collection;
45-
import java.util.GregorianCalendar;
4648
import java.util.List;
4749
import java.util.Map;
4850

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

6668
StatementType statementType;
69+
6770
public PreparedStatementImpl(ConnectionImpl connection, String sql) throws SQLException {
6871
super(connection);
6972
this.originalSql = sql.trim();
7073
//Split the sql string into an array of strings around question mark tokens
71-
this.sqlSegments = originalSql.split("\\?");
74+
this.sqlSegments = splitStatement(originalSql);
7275
this.statementType = parseStatementType(originalSql);
7376

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

7982
//Create an array of objects to store the parameters
80-
if (originalSql.contains("?")) {
81-
int count = originalSql.length() - originalSql.replace("?", "").length();
82-
this.parameters = new Object[count];
83-
} else {
84-
this.parameters = new Object[0];
85-
}
86-
83+
this.parameters = new Object[sqlSegments.length - 1];
8784
this.defaultCalendar = connection.defaultCalendar;
8885
}
8986

90-
private String compileSql(String []segments) {
87+
private String compileSql(String [] segments) {
9188
StringBuilder sb = new StringBuilder();
9289
for (int i = 0; i < segments.length; i++) {
9390
sb.append(segments[i]);
@@ -98,6 +95,7 @@ private String compileSql(String []segments) {
9895
LOG.trace("Compiled SQL: {}", sb);
9996
return sb.toString();
10097
}
98+
10199
@Override
102100
public ResultSet executeQuery() throws SQLException {
103101
checkClosed();
@@ -517,14 +515,14 @@ private static String encodeObject(Object x) throws SQLException {
517515
} else if (x instanceof ZonedDateTime) {
518516
return encodeObject(((ZonedDateTime) x).toInstant());
519517
} else if (x instanceof Instant) {
520-
return "fromUnixTimestamp64Nano(" + (((Instant) x).getEpochSecond() * 1_000_000_000L + ((Instant) x).getNano())+ ")";
518+
return "fromUnixTimestamp64Nano(" + (((Instant) x).getEpochSecond() * 1_000_000_000L + ((Instant) x).getNano()) + ")";
521519
} else if (x instanceof InetAddress) {
522520
return "'" + ((InetAddress) x).getHostAddress() + "'";
523521
} else if (x instanceof Array) {
524522
StringBuilder listString = new StringBuilder();
525523
listString.append("[");
526524
int i = 0;
527-
for (Object item : (Object[])((Array) x).getArray()) {
525+
for (Object item : (Object[]) ((Array) x).getArray()) {
528526
if (i > 0) {
529527
listString.append(", ");
530528
}
@@ -616,4 +614,70 @@ private static String encodeObject(Object x) throws SQLException {
616614
private static String escapeString(String x) {
617615
return x.replace("\\", "\\\\").replace("'", "\\'");//Escape single quotes
618616
}
617+
618+
private static String [] splitStatement(String sql) {
619+
List<String> segments = new ArrayList<>();
620+
char[] chars = sql.toCharArray();
621+
int segmentStart = 0;
622+
for (int i = 0; i < chars.length; i++) {
623+
char c = chars[i];
624+
if (c == '\'' || c == '"' || c == '`') {
625+
// string literal or identifier
626+
i = skip(chars, i + 1, c, true);
627+
} else if (c == '/' && lookahead(chars, i) == '*') {
628+
// block comment
629+
int end = sql.indexOf("*/", i);
630+
if (end == -1) {
631+
// missing comment end
632+
break;
633+
}
634+
i = end + 1;
635+
} else if (c == '#' || (c == '-' && lookahead(chars, i) == '-')) {
636+
// line comment
637+
i = skip(chars, i + 1, '\n', false);
638+
} else if (c == '?') {
639+
// question mark
640+
segments.add(sql.substring(segmentStart, i));
641+
segmentStart = i + 1;
642+
}
643+
}
644+
if (segmentStart < chars.length) {
645+
segments.add(sql.substring(segmentStart));
646+
} else {
647+
// add empty segment in case question mark was last char of sql
648+
segments.add("");
649+
}
650+
return segments.toArray(new String[0]);
651+
}
652+
653+
private static int skip(char[] chars, int from, char until, boolean escape) {
654+
for (int i = from; i < chars.length; i++) {
655+
char curr = chars[i];
656+
if (escape) {
657+
char next = lookahead(chars, i);
658+
if ((curr == '\\' && (next == '\\' || next == until)) || (curr == until && next == until)) {
659+
// should skip:
660+
// 1) double \\ (backslash escaped with backslash)
661+
// 2) \[until] ([until] char, escaped with backslash)
662+
// 3) [until][until] ([until] char, escaped with [until])
663+
i++;
664+
continue;
665+
}
666+
}
667+
668+
if (curr == until) {
669+
return i;
670+
}
671+
}
672+
return chars.length;
673+
}
674+
675+
private static char lookahead(char[] chars, int pos) {
676+
pos = pos + 1;
677+
if (pos >= chars.length) {
678+
return '\0';
679+
}
680+
return chars[pos];
681+
}
682+
619683
}

‎jdbc-v2/src/main/java/com/clickhouse/jdbc/StatementImpl.java‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,14 +24,15 @@
2424
import java.util.List;
2525
import java.util.UUID;
2626
import java.util.concurrent.TimeUnit;
27+
import java.util.regex.Pattern;
2728

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

3132
ConnectionImpl connection;
3233
private int queryTimeout;
3334
protected boolean closed;
34-
private ResultSetImpl currentResultSet;
35+
protected ResultSetImpl currentResultSet;
3536
private OperationMetrics metrics;
3637
protected List<String> batch;
3738
private String lastSql;
@@ -69,7 +70,7 @@ protected static StatementType parseStatementType(String sql) {
6970
return StatementType.OTHER;
7071
}
7172

72-
trimmedSql = trimmedSql.replaceAll("/\\*.*?\\*/", "").trim(); // remove comments
73+
trimmedSql = BLOCK_COMMENT.matcher(trimmedSql).replaceAll("").trim(); // remove comments
7374
String[] lines = trimmedSql.split("\n");
7475
for (String line : lines) {
7576
String trimmedLine = line.trim();
@@ -171,7 +172,10 @@ public ResultSetImpl executeQuery(String sql, QuerySettings settings) throws SQL
171172
closePreviousResultSet();
172173

173174
QuerySettings mergedSettings = QuerySettings.merge(connection.getDefaultQuerySettings(), settings);
174-
175+
if (maxRows > 0) {
176+
mergedSettings.setOption(ClientConfigProperties.serverSetting(ServerSettings.MAX_RESULT_ROWS), maxRows);
177+
mergedSettings.setOption(ClientConfigProperties.serverSetting(ServerSettings.RESULT_OVERFLOW_MODE), "break");
178+
}
175179

176180
if (mergedSettings.getQueryId() != null) {
177181
lastQueryId = mergedSettings.getQueryId();
@@ -626,4 +630,6 @@ public String enquoteNCharLiteral(String val) throws SQLException {
626630
public String getLastQueryId() {
627631
return lastQueryId;
628632
}
633+
634+
private static final Pattern BLOCK_COMMENT = Pattern.compile("/\\*.*?\\*/", Pattern.DOTALL);
629635
}

‎jdbc-v2/src/test/java/com/clickhouse/jdbc/PreparedStatementTest.java‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,10 @@
66

77
import java.sql.Array;
88
import java.sql.Connection;
9+
import java.sql.JDBCType;
910
import java.sql.PreparedStatement;
1011
import java.sql.ResultSet;
12+
import java.sql.ResultSetMetaData;
1113
import java.sql.Statement;
1214
import java.sql.Types;
1315
import java.util.Arrays;
@@ -449,4 +451,44 @@ void testMetabaseBug01() throws Exception {
449451
}
450452
}
451453
}
454+
455+
@Test(groups = { "integration" })
456+
void testStatementSplit() throws Exception {
457+
try (Connection conn = getJdbcConnection()) {
458+
try (Statement stmt = conn.createStatement()) {
459+
stmt.execute("CREATE TABLE `with_complex_id` (`v?``1` Int32, \"v?\"\"2\" Int32,`v?\\`3` Int32, \"v?\\\"4\" Int32) ENGINE Memory;");
460+
}
461+
String insertQuery = "-- line comment1 ?\n"
462+
+ "# line comment2 ?\n"
463+
+ "#! line comment3 ?\n"
464+
+ "/* block comment ? \n */"
465+
+ "INSERT INTO `with_complex_id`(`v?``1`, \"v?\"\"2\",`v?\\`3`, \"v?\\\"4\") VALUES (?, ?, ?, ?);";
466+
try (PreparedStatement stmt = conn.prepareStatement(insertQuery)) {
467+
stmt.setInt(1, 1);
468+
stmt.setInt(2, 2);
469+
stmt.setInt(3, 3);
470+
stmt.setInt(4, 4);
471+
stmt.execute();
472+
}
473+
String selectQuery = "-- line comment ?\n"
474+
+ "/* block comment ? \n */"
475+
+ "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\" = ?";
476+
try (PreparedStatement stmt = conn.prepareStatement(selectQuery)) {
477+
stmt.setInt(1, 1);
478+
stmt.setInt(2, 2);
479+
stmt.setInt(3, 3);
480+
stmt.setInt(4, 4);
481+
try (ResultSet rs = stmt.executeQuery()) {
482+
assertTrue(rs.next());
483+
assertEquals(rs.getInt(1), 1);
484+
assertEquals(rs.getInt(2), 2);
485+
assertEquals(rs.getInt(3), 3);
486+
assertEquals(rs.getInt(4), 4);
487+
assertEquals(rs.getString(5), "test ' string1 ?");
488+
assertEquals(rs.getString(6), "test ' string2 ?");
489+
assertEquals(rs.getString(7), "test string3 ?\\");
490+
}
491+
}
492+
}
493+
}
452494
}

0 commit comments

Comments
 (0)