Skip to content

Commit 751ddb1

Browse files
chernserchernser
authored
Merge pull request #2521 from ClickHouse/fix_connection_impl
[JDBC] enabled tests. added some more for ConnectionImpl. implementated nati…
2 parents 9f19d82 + c74f47d commit 751ddb1

14 files changed

+251
-145
lines changed

‎jdbc-v2/src/main/java/com/clickhouse/jdbc/internal/ClientInfoProperties.java renamed to ‎jdbc-v2/src/main/java/com/clickhouse/jdbc/ClientInfoProperties.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
package com.clickhouse.jdbc.internal;
1+
package com.clickhouse.jdbc;
22

33
public enum ClientInfoProperties {
44

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

Lines changed: 66 additions & 54 deletions
Large diffs are not rendered by default.

‎jdbc-v2/src/main/java/com/clickhouse/jdbc/internal/DriverProperties.java renamed to ‎jdbc-v2/src/main/java/com/clickhouse/jdbc/DriverProperties.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
1-
package com.clickhouse.jdbc.internal;
1+
package com.clickhouse.jdbc;
22

33
import java.util.Collections;
44
import java.util.List;
55

66
/**
7-
* JDBC driver specific properties. Should not include any of ClientConfigProperties.
7+
* JDBC driver specific properties. Does not include anything from ClientConfigProperties.
88
* Processing logic should be the follows
99
* 1. If property is among DriverProperties then Driver handles it specially and will not pass to a client
1010
* 2. If property is not among DriverProperties then it is passed to a client

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

Lines changed: 29 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
import com.clickhouse.client.api.query.QueryResponse;
77
import com.clickhouse.client.api.query.QuerySettings;
88
import com.clickhouse.client.api.sql.SQLUtils;
9-
import com.clickhouse.jdbc.internal.DriverProperties;
109
import com.clickhouse.jdbc.internal.ExceptionUtils;
1110
import com.clickhouse.jdbc.internal.ParsedStatement;
1211
import org.slf4j.Logger;
@@ -66,27 +65,34 @@ protected void ensureOpen() throws SQLException {
6665
}
6766
}
6867

69-
7068
private String parseJdbcEscapeSyntax(String sql) {
7169
LOG.trace("Original SQL: {}", sql);
7270
if (escapeProcessingEnabled) {
73-
// Replace {d 'YYYY-MM-DD'} with corresponding SQL date format
74-
sql = sql.replaceAll("\\{d '([^']*)'\\}", "toDate('$1')");
71+
sql = escapedSQLToNative(sql);
72+
}
73+
LOG.trace("Escaped SQL: {}", sql);
74+
return sql;
75+
}
7576

76-
// Replace {ts 'YYYY-MM-DD HH:mm:ss'} with corresponding SQL timestamp format
77-
sql = sql.replaceAll("\\{ts '([^']*)'\\}", "timestamp('$1')");
77+
public static String escapedSQLToNative(String sql) {
78+
if (sql == null) {
79+
throw new IllegalArgumentException("SQL may not be null");
80+
}
81+
// Replace {d 'YYYY-MM-DD'} with corresponding SQL date format
82+
sql = sql.replaceAll("\\{d '([^']*)'\\}", "toDate('$1')");
7883

79-
// Replace function escape syntax {fn <function>} (e.g., {fn UCASE(name)})
80-
sql = sql.replaceAll("\\{fn ([^\\}]*)\\}", "$1");
84+
// Replace {ts 'YYYY-MM-DD HH:mm:ss'} with corresponding SQL timestamp format
85+
sql = sql.replaceAll("\\{ts '([^']*)'\\}", "timestamp('$1')");
8186

82-
// Handle outer escape syntax
83-
//sql = sql.replaceAll("\\{escape '([^']*)'\\}", "'$1'");
87+
// Replace function escape syntax {fn <function>} (e.g., {fn UCASE(name)})
88+
sql = sql.replaceAll("\\{fn ([^\\}]*)\\}", "$1");
89+
90+
// Handle outer escape syntax
91+
//sql = sql.replaceAll("\\{escape '([^']*)'\\}", "'$1'");
92+
93+
// Note: do not remove new lines because they may be used to delimit comments
94+
// Add more replacements as needed for other JDBC escape sequences
8495

85-
// Clean new empty lines in sql
86-
sql = sql.replaceAll("(?m)^\\s*$\\n?", "");
87-
// Add more replacements as needed for other JDBC escape sequences
88-
}
89-
LOG.trace("Escaped SQL: {}", sql);
9096
return sql;
9197
}
9298

@@ -138,17 +144,18 @@ protected ResultSetImpl executeQueryImpl(String sql, QuerySettings settings) thr
138144
lastStatementSql = parseJdbcEscapeSyntax(sql);
139145
LOG.trace("SQL Query: {}", lastStatementSql); // this is not secure for create statements because of passwords
140146
if (queryTimeout == 0) {
141-
response = connection.client.query(lastStatementSql, mergedSettings).get();
147+
response = connection.getClient().query(lastStatementSql, mergedSettings).get();
142148
} else {
143-
response = connection.client.query(lastStatementSql, mergedSettings).get(queryTimeout, TimeUnit.SECONDS);
149+
response = connection.getClient().query(lastStatementSql, mergedSettings).get(queryTimeout, TimeUnit.SECONDS);
144150
}
145151

146152
if (response.getFormat().isText()) {
147153
throw new SQLException("Only RowBinaryWithNameAndTypes is supported for output format. Please check your query.",
148154
ExceptionUtils.SQL_STATE_CLIENT_ERROR);
149155
}
150-
ClickHouseBinaryFormatReader reader = connection.client.newBinaryFormatReader(response);
156+
ClickHouseBinaryFormatReader reader = connection.getClient().newBinaryFormatReader(response);
151157
if (reader.getSchema() == null) {
158+
reader.close();
152159
throw new SQLException("Called method expects empty or filled result set but query has returned none. Consider using `java.sql.Statement.execute(java.lang.String)`", ExceptionUtils.SQL_STATE_CLIENT_ERROR);
153160
}
154161
return new ResultSetImpl(this, response, reader);
@@ -191,8 +198,8 @@ protected long executeUpdateImpl(String sql, QuerySettings settings) throws SQLE
191198
lastStatementSql = parseJdbcEscapeSyntax(sql);
192199
LOG.trace("SQL Query: {}", lastStatementSql);
193200
int updateCount = 0;
194-
try (QueryResponse response = queryTimeout == 0 ? connection.client.query(lastStatementSql, mergedSettings).get()
195-
: connection.client.query(lastStatementSql, mergedSettings).get(queryTimeout, TimeUnit.SECONDS)) {
201+
try (QueryResponse response = queryTimeout == 0 ? connection.getClient().query(lastStatementSql, mergedSettings).get()
202+
: connection.getClient().query(lastStatementSql, mergedSettings).get(queryTimeout, TimeUnit.SECONDS)) {
196203
updateCount = Math.max(0, (int) response.getWrittenRows()); // when statement alters schema no result rows returned.
197204
lastQueryId = response.getQueryId();
198205
} catch (Exception e) {
@@ -202,7 +209,7 @@ protected long executeUpdateImpl(String sql, QuerySettings settings) throws SQLE
202209
return updateCount;
203210
}
204211

205-
protected void postUpdateActions() {
212+
protected void postUpdateActions() throws SQLException {
206213
if (parsedStatement.getUseDatabase() != null) {
207214
this.localSettings.setDatabase(parsedStatement.getUseDatabase());
208215
}
@@ -278,7 +285,7 @@ public void cancel() throws SQLException {
278285
return;
279286
}
280287

281-
try (QueryResponse response = connection.client.query(String.format("KILL QUERY%sWHERE query_id = '%s'",
288+
try (QueryResponse response = connection.getClient().query(String.format("KILL QUERY%sWHERE query_id = '%s'",
282289
connection.onCluster ? " ON CLUSTER " + connection.cluster + " " : " ",
283290
lastQueryId), connection.getDefaultQuerySettings()).get()){
284291
LOG.debug("Query {} was killed by {}", lastQueryId, response.getQueryId());

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -116,8 +116,8 @@ public long executeLargeUpdate() throws SQLException {
116116
InputStream in = new ByteArrayInputStream(out.toByteArray());
117117
InsertSettings settings = new InsertSettings();
118118
try (InsertResponse response = queryTimeout == 0 ?
119-
connection.client.insert(tableSchema.getTableName(),in, writer.getFormat(), settings).get()
120-
: connection.client.insert(tableSchema.getTableName(),in, writer.getFormat(), settings).get(queryTimeout, TimeUnit.SECONDS)) {
119+
connection.getClient().insert(tableSchema.getTableName(),in, writer.getFormat(), settings).get()
120+
: connection.getClient().insert(tableSchema.getTableName(),in, writer.getFormat(), settings).get(queryTimeout, TimeUnit.SECONDS)) {
121121
updateCount = Math.max(0, (int) response.getWrittenRows()); // when statement alters schema no result rows returned.
122122
lastQueryId = response.getQueryId();
123123
} catch (Exception e) {

‎jdbc-v2/src/main/java/com/clickhouse/jdbc/internal/JdbcConfiguration.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,14 @@
55
import com.clickhouse.client.api.http.ClickHouseHttpProto;
66
import com.clickhouse.data.ClickHouseDataType;
77
import com.clickhouse.jdbc.Driver;
8+
import com.clickhouse.jdbc.DriverProperties;
89
import com.google.common.collect.ImmutableMap;
910

1011
import java.io.UnsupportedEncodingException;
1112
import java.net.URI;
1213
import java.net.URISyntaxException;
1314
import java.net.URLDecoder;
1415
import java.nio.charset.StandardCharsets;
15-
import java.nio.charset.UnsupportedCharsetException;
1616
import java.sql.DriverPropertyInfo;
1717
import java.sql.SQLException;
1818
import java.util.Comparator;

‎jdbc-v2/src/main/java/com/clickhouse/jdbc/metadata/DatabaseMetaDataImpl.java

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,11 @@
77
import com.clickhouse.jdbc.Driver;
88
import com.clickhouse.jdbc.JdbcV2Wrapper;
99
import com.clickhouse.jdbc.ResultSetImpl;
10-
import com.clickhouse.jdbc.StatementImpl;
11-
import com.clickhouse.jdbc.internal.ClientInfoProperties;
12-
import com.clickhouse.jdbc.internal.DriverProperties;
10+
import com.clickhouse.jdbc.ClientInfoProperties;
11+
import com.clickhouse.jdbc.DriverProperties;
1312
import com.clickhouse.jdbc.internal.ExceptionUtils;
1413
import com.clickhouse.jdbc.internal.JdbcUtils;
1514
import com.clickhouse.jdbc.internal.MetadataResultSet;
16-
import com.clickhouse.jdbc.internal.SqlParser;
1715
import com.clickhouse.logging.Logger;
1816
import com.clickhouse.logging.LoggerFactory;
1917

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

Lines changed: 95 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,10 @@
77
import com.clickhouse.client.api.ClientConfigProperties;
88
import com.clickhouse.client.api.ServerException;
99
import com.clickhouse.client.api.internal.ServerSettings;
10-
import com.clickhouse.jdbc.internal.ClientInfoProperties;
11-
import com.clickhouse.jdbc.internal.DriverProperties;
1210
import com.github.tomakehurst.wiremock.WireMockServer;
1311
import com.github.tomakehurst.wiremock.client.WireMock;
1412
import com.github.tomakehurst.wiremock.common.ConsoleNotifier;
1513
import com.github.tomakehurst.wiremock.core.WireMockConfiguration;
16-
1714
import org.testng.Assert;
1815
import org.testng.annotations.DataProvider;
1916
import org.testng.annotations.Test;
@@ -32,31 +29,75 @@
3229
import java.util.Properties;
3330
import java.util.UUID;
3431

32+
import static org.testng.Assert.assertEquals;
33+
import static org.testng.Assert.assertNull;
3534
import static org.testng.Assert.assertThrows;
3635
import static org.testng.Assert.fail;
3736

3837
public class ConnectionTest extends JdbcIntegrationTest {
3938

40-
@Test(groups = { "integration" }, enabled = false)
39+
@Test(groups = { "integration" })
4140
public void createAndCloseStatementTest() throws SQLException {
42-
Connection localConnection = this.getJdbcConnection();
43-
Statement statement = localConnection.createStatement();
44-
Assert.assertNotNull(statement);
41+
Connection conn = getJdbcConnection();
42+
Statement stmt = conn.createStatement();
43+
PreparedStatement pStmt = conn.prepareStatement("SELECT ? as v");
44+
pStmt.setString(1, "test string");
45+
conn.close();
46+
conn.close(); // check second attempt doesn't throw anything
47+
assertThrows(SQLException.class, conn::createStatement);
48+
49+
try {
50+
stmt.executeQuery("SELECT 1");
51+
fail("Exception expected");
52+
} catch (SQLException e) {
53+
Assert.assertTrue(e.getMessage().contains("closed"));
54+
}
55+
56+
try {
57+
pStmt.executeQuery();
58+
fail("Exception expected");
59+
} catch (SQLException e) {
60+
Assert.assertTrue(e.getMessage().contains("closed"));
4561

46-
assertThrows(SQLFeatureNotSupportedException.class, () -> localConnection.createStatement(ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_READ_ONLY));
47-
assertThrows(SQLFeatureNotSupportedException.class, () -> localConnection.createStatement(ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_READ_ONLY, ResultSet.CLOSE_CURSORS_AT_COMMIT));
62+
}
4863
}
4964

50-
@Test(groups = { "integration" }, enabled = false)
51-
public void prepareStatementTest() throws SQLException {
52-
Connection localConnection = this.getJdbcConnection();
53-
PreparedStatement statement = localConnection.prepareStatement("SELECT 1");
54-
Assert.assertNotNull(statement);
55-
assertThrows(SQLFeatureNotSupportedException.class, () -> localConnection.prepareStatement("SELECT 1", Statement.RETURN_GENERATED_KEYS));
56-
assertThrows(SQLFeatureNotSupportedException.class, () -> localConnection.prepareStatement("SELECT 1", new int[] { 1 }));
57-
assertThrows(SQLFeatureNotSupportedException.class, () -> localConnection.prepareStatement("SELECT 1", ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_READ_ONLY));
58-
assertThrows(SQLFeatureNotSupportedException.class, () -> localConnection.prepareStatement("SELECT 1", ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_READ_ONLY, ResultSet.CLOSE_CURSORS_AT_COMMIT));
59-
assertThrows(SQLFeatureNotSupportedException.class, () -> localConnection.prepareStatement("SELECT 1", new String[] { "1" }));
65+
@Test(groups = { "integration" })
66+
public void testCreateUnsupportedStatements() throws Throwable {
67+
68+
boolean[] throwUnsupportedException = new boolean[] {false, true};
69+
70+
for (boolean flag : throwUnsupportedException) {
71+
Properties props = new Properties();
72+
if (flag) {
73+
props.setProperty(DriverProperties.IGNORE_UNSUPPORTED_VALUES.getKey(), "true");
74+
}
75+
76+
try (Connection conn = this.getJdbcConnection(props)) {
77+
Assert.ThrowingRunnable[] createStatements = new Assert.ThrowingRunnable[]{
78+
() -> conn.createStatement(ResultSet.TYPE_SCROLL_INSENSITIVE, ResultSet.CONCUR_READ_ONLY),
79+
() -> conn.createStatement(ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_UPDATABLE),
80+
() -> conn.createStatement(ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_READ_ONLY, ResultSet.HOLD_CURSORS_OVER_COMMIT),
81+
() -> conn.prepareStatement("SELECT 1", Statement.RETURN_GENERATED_KEYS),
82+
() -> conn.prepareStatement("SELECT 1", new int[]{1}),
83+
() -> conn.prepareStatement("SELECT 1", new String[]{"1"}),
84+
() -> conn.prepareStatement("SELECT 1", ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_UPDATABLE),
85+
() -> conn.prepareStatement("SELECT 1", ResultSet.TYPE_SCROLL_INSENSITIVE, ResultSet.CONCUR_READ_ONLY),
86+
() -> conn.prepareStatement("SELECT 1", ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_READ_ONLY, ResultSet.HOLD_CURSORS_OVER_COMMIT),
87+
conn::setSavepoint,
88+
() -> conn.setSavepoint("save point"),
89+
() -> conn.createStruct("simple", null),
90+
};
91+
92+
for (Assert.ThrowingRunnable createStatement : createStatements) {
93+
if (!flag) {
94+
Assert.assertThrows(SQLFeatureNotSupportedException.class, createStatement );
95+
} else {
96+
createStatement.run();
97+
}
98+
}
99+
}
100+
}
60101
}
61102

62103
@Test(groups = { "integration" })
@@ -67,12 +108,16 @@ public void prepareCallTest() throws SQLException {
67108
assertThrows(SQLFeatureNotSupportedException.class, () -> localConnection.prepareCall("SELECT 1", ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_READ_ONLY, ResultSet.CLOSE_CURSORS_AT_COMMIT));
68109
}
69110

70-
@Test(groups = { "integration" }, enabled = false)
111+
@Test(groups = { "integration" })
71112
public void nativeSQLTest() throws SQLException {
72-
// TODO: implement
73-
Connection localConnection = this.getJdbcConnection();
74-
String sql = "SELECT 1";
75-
Assert.assertEquals(localConnection.nativeSQL(sql), sql);
113+
try (Connection conn = this.getJdbcConnection()) {
114+
String escapedSQL = "SELECT \n{ts '2024-01-02 02:01:01'} as v1,\n {d '2024-01-02 02:01:01'} as v2,\n {d ?} as v3";
115+
String nativeSQL = "SELECT \ntimestamp('2024-01-02 02:01:01') as v1,\n toDate('2024-01-02 02:01:01') as v2,\n {d ?} as v3";
116+
Assert.assertEquals(conn.nativeSQL(escapedSQL), nativeSQL);
117+
118+
Assert.expectThrows(IllegalArgumentException.class, () -> conn.nativeSQL(null));
119+
Assert.assertEquals(conn.nativeSQL("SELECT 1 as t"), "SELECT 1 as t");
120+
}
76121
}
77122

78123
@Test(groups = { "integration" })
@@ -97,8 +142,8 @@ public void setAutoCommitTest() throws SQLException {
97142
@Test(groups = { "integration" })
98143
public void testCommitRollback() throws SQLException {
99144
try (Connection localConnection = this.getJdbcConnection()) {
100-
assertThrows(SQLFeatureNotSupportedException.class, () -> localConnection.commit());
101-
assertThrows(SQLFeatureNotSupportedException.class, () -> localConnection.rollback());
145+
assertThrows(SQLFeatureNotSupportedException.class, localConnection::commit);
146+
assertThrows(SQLFeatureNotSupportedException.class, localConnection::rollback);
102147
assertThrows(SQLFeatureNotSupportedException.class, () -> localConnection.rollback(null));
103148
}
104149

@@ -183,7 +228,7 @@ public void clearWarningsTest() throws SQLException {
183228
@Test(groups = { "integration" })
184229
public void getTypeMapTest() throws SQLException {
185230
Connection localConnection = this.getJdbcConnection();
186-
assertThrows(SQLFeatureNotSupportedException.class, () -> localConnection.getTypeMap());
231+
assertThrows(SQLFeatureNotSupportedException.class, localConnection::getTypeMap);
187232
}
188233

189234
@Test(groups = { "integration" })
@@ -207,7 +252,7 @@ public void getHoldabilityTest() throws SQLException {
207252
@Test(groups = { "integration" })
208253
public void setSavepointTest() throws SQLException {
209254
Connection localConnection = this.getJdbcConnection();
210-
assertThrows(SQLFeatureNotSupportedException.class, () -> localConnection.setSavepoint());
255+
assertThrows(SQLFeatureNotSupportedException.class, localConnection::setSavepoint);
211256
assertThrows(SQLFeatureNotSupportedException.class, () -> localConnection.setSavepoint("savepoint-name"));
212257
}
213258

@@ -268,7 +313,6 @@ public void setAndGetClientInfoTest(String clientName) throws SQLException {
268313
try (ResultSet rs = stmt.executeQuery(logQuery)) {
269314
Assert.assertTrue(rs.next());
270315
String userAgent = rs.getString("http_user_agent");
271-
System.out.println(userAgent);
272316
if (clientName != null && !clientName.isEmpty()) {
273317
Assert.assertTrue(userAgent.startsWith(clientName), "Expected to start with '" + clientName + "' but value was '" + userAgent + "'");
274318
}
@@ -353,7 +397,7 @@ public void setNetworkTimeoutTest() throws SQLException {
353397
@Test(groups = { "integration" })
354398
public void getNetworkTimeoutTest() throws SQLException {
355399
Connection localConnection = this.getJdbcConnection();
356-
assertThrows(SQLFeatureNotSupportedException.class, () -> localConnection.getNetworkTimeout());
400+
assertThrows(SQLFeatureNotSupportedException.class, localConnection::getNetworkTimeout);
357401
}
358402

359403
@Test(groups = { "integration" })
@@ -606,4 +650,25 @@ private static Object[][] createValidDatabaseNames() {
606650
};
607651
}
608652

653+
@Test(groups = {"integration"})
654+
public void testClientInfoProperties() throws Exception {
655+
try (Connection conn = this.getJdbcConnection()) {
656+
657+
Properties properties = conn.getClientInfo();
658+
assertEquals(properties.get(ClientInfoProperties.APPLICATION_NAME.getKey()), "");
659+
660+
properties.put(ClientInfoProperties.APPLICATION_NAME.getKey(), "test");
661+
conn.setClientInfo(properties);
662+
663+
assertEquals(properties.get(ClientInfoProperties.APPLICATION_NAME.getKey()), "test");
664+
665+
conn.setClientInfo(new Properties());
666+
assertNull(conn.getClientInfo(ClientInfoProperties.APPLICATION_NAME.getKey()));
667+
668+
conn.setClientInfo(ClientInfoProperties.APPLICATION_NAME.getKey(), "test 2");
669+
assertEquals(conn.getClientInfo(ClientInfoProperties.APPLICATION_NAME.getKey()), "test 2");
670+
671+
assertNull(conn.getClientInfo("unknown"));
672+
}
673+
}
609674
}

0 commit comments

Comments
 (0)