Skip to content

Commit 49d5322

Browse files
chernserchernser
authored
Merge pull request #2328 from ClickHouse/jdbc_fix_prepared_statement
[jdbc-v2] Disabled calling some Statement method from PreparedStatement.
2 parents 5ae9b83 + c4b8d21 commit 49d5322

File tree

4 files changed

+186
-28
lines changed

4 files changed

+186
-28
lines changed

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

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

33
import com.clickhouse.client.api.metadata.TableSchema;
4+
import com.clickhouse.client.api.query.QuerySettings;
45
import com.clickhouse.data.Tuple;
56
import com.clickhouse.jdbc.internal.ExceptionUtils;
67
import com.clickhouse.jdbc.internal.JdbcUtils;
@@ -101,13 +102,14 @@ private String compileSql(String []segments) {
101102
@Override
102103
public ResultSet executeQuery() throws SQLException {
103104
checkClosed();
104-
return executeQuery(compileSql(sqlSegments));
105+
return super.executeQueryImpl(compileSql(sqlSegments), new QuerySettings().setDatabase(connection.getSchema()));
105106
}
106107

107108
@Override
108109
public int executeUpdate() throws SQLException {
109110
checkClosed();
110-
return executeUpdate(compileSql(sqlSegments));
111+
return super.executeUpdateImpl(compileSql(sqlSegments), statementType,
112+
new QuerySettings().setDatabase(connection.getSchema()));
111113
}
112114

113115
@Override
@@ -234,17 +236,18 @@ public void setObject(int parameterIndex, Object x) throws SQLException {
234236
@Override
235237
public boolean execute() throws SQLException {
236238
checkClosed();
237-
return execute(compileSql(sqlSegments));
239+
return super.executeImpl(compileSql(sqlSegments), statementType,
240+
new QuerySettings().setDatabase(connection.getSchema()));
238241
}
239242

240243
@Override
241244
public void addBatch() throws SQLException {
242245
checkClosed();
243246
if (statementType == StatementType.INSERT) {
244247
// adding values to the end of big INSERT statement.
245-
addBatch(compileSql(valueSegments));
248+
super.addBatch(compileSql(valueSegments));
246249
} else {
247-
addBatch(compileSql(sqlSegments));
250+
super.addBatch(compileSql(sqlSegments));
248251
}
249252
}
250253

@@ -259,7 +262,8 @@ public int[] executeBatch() throws SQLException {
259262
sb.append(sql).append(",");
260263
}
261264
sb.setCharAt(sb.length() - 1, ';');
262-
int rowsInserted = executeUpdate(sb.toString());
265+
int rowsInserted = executeUpdateImpl(sb.toString(), statementType,
266+
new QuerySettings().setDatabase(connection.getSchema()));
263267
// clear batch and re-add insert into
264268
int[] results = new int[batch.size()];
265269
if (rowsInserted == batch.size()) {
@@ -274,18 +278,22 @@ public int[] executeBatch() throws SQLException {
274278
return results;
275279
} else {
276280
// run executeBatch
277-
return super.executeBatch();
281+
return executeBatchImpl().stream().mapToInt(Integer::intValue).toArray();
278282
}
279283
}
280284

281285
@Override
282286
public long[] executeLargeBatch() throws SQLException {
283-
int[] results = executeBatch();
284-
long[] longResults = new long[results.length];
285-
for (int i = 0; i < results.length; i++) {
286-
longResults[i] = results[i];
287+
return executeBatchImpl().stream().mapToLong(Integer::longValue).toArray();
288+
}
289+
290+
private List<Integer> executeBatchImpl() throws SQLException {
291+
List<Integer> results = new ArrayList<>();
292+
QuerySettings settings = new QuerySettings().setDatabase(connection.getSchema());
293+
for (String sql : batch) {
294+
results.add(executeUpdateImpl(sql, statementType, settings));
287295
}
288-
return longResults;
296+
return results;
289297
}
290298

291299
@Override
@@ -412,7 +420,8 @@ public ParameterMetaData getParameterMetaData() throws SQLException {
412420
@Override
413421
public void setRowId(int parameterIndex, RowId x) throws SQLException {
414422
checkClosed();
415-
parameters[parameterIndex - 1] = encodeObject(x);
423+
throw new SQLException("ROWID type is not supported by ClickHouse.",
424+
ExceptionUtils.SQL_STATE_FEATURE_NOT_SUPPORTED);
416425
}
417426

418427
@Override
@@ -540,6 +549,118 @@ public long executeLargeUpdate() throws SQLException {
540549
return executeUpdate();
541550
}
542551

552+
@Override
553+
public final void addBatch(String sql) throws SQLException {
554+
checkClosed();
555+
throw new SQLException(
556+
"addBatch(String) cannot be called in PreparedStatement or CallableStatement!",
557+
ExceptionUtils.SQL_STATE_WRONG_OBJECT_TYPE);
558+
}
559+
560+
@Override
561+
public final boolean execute(String sql) throws SQLException {
562+
checkClosed();
563+
throw new SQLException(
564+
"execute(String) cannot be called in PreparedStatement or CallableStatement!",
565+
ExceptionUtils.SQL_STATE_WRONG_OBJECT_TYPE);
566+
}
567+
568+
@Override
569+
public final boolean execute(String sql, int autoGeneratedKeys) throws SQLException {
570+
checkClosed();
571+
throw new SQLException(
572+
"execute(String, int) cannot be called in PreparedStatement or CallableStatement!",
573+
ExceptionUtils.SQL_STATE_WRONG_OBJECT_TYPE);
574+
}
575+
576+
@Override
577+
public final boolean execute(String sql, int[] columnIndexes) throws SQLException {
578+
checkClosed();
579+
throw new SQLException(
580+
"execute(String, int[]) cannot be called in PreparedStatement or CallableStatement!",
581+
ExceptionUtils.SQL_STATE_WRONG_OBJECT_TYPE);
582+
}
583+
584+
@Override
585+
public final boolean execute(String sql, String[] columnNames) throws SQLException {
586+
checkClosed();
587+
throw new SQLException(
588+
"execute(String, String[]) cannot be called in PreparedStatement or CallableStatement!",
589+
ExceptionUtils.SQL_STATE_WRONG_OBJECT_TYPE);
590+
}
591+
592+
@Override
593+
public final long executeLargeUpdate(String sql) throws SQLException {
594+
checkClosed();
595+
throw new SQLException(
596+
"executeLargeUpdate(String) cannot be called in PreparedStatement or CallableStatement!",
597+
ExceptionUtils.SQL_STATE_WRONG_OBJECT_TYPE);
598+
}
599+
600+
@Override
601+
public final long executeLargeUpdate(String sql, int autoGeneratedKeys) throws SQLException {
602+
checkClosed();
603+
throw new SQLException(
604+
"executeLargeUpdate(String, int) cannot be called in PreparedStatement or CallableStatement!",
605+
ExceptionUtils.SQL_STATE_WRONG_OBJECT_TYPE);
606+
}
607+
608+
@Override
609+
public final long executeLargeUpdate(String sql, int[] columnIndexes) throws SQLException {
610+
checkClosed();
611+
throw new SQLException(
612+
"executeLargeUpdate(String, int[]) cannot be called in PreparedStatement or CallableStatement!",
613+
ExceptionUtils.SQL_STATE_WRONG_OBJECT_TYPE);
614+
}
615+
616+
@Override
617+
public final long executeLargeUpdate(String sql, String[] columnNames) throws SQLException {
618+
checkClosed();
619+
throw new SQLException(
620+
"executeLargeUpdate(String, String[]) cannot be called in PreparedStatement or CallableStatement!",
621+
ExceptionUtils.SQL_STATE_WRONG_OBJECT_TYPE);
622+
}
623+
624+
@Override
625+
public final ResultSet executeQuery(String sql) throws SQLException {
626+
checkClosed();
627+
throw new SQLException(
628+
"executeQuery(String) cannot be called in PreparedStatement or CallableStatement!",
629+
ExceptionUtils.SQL_STATE_WRONG_OBJECT_TYPE);
630+
}
631+
632+
@Override
633+
public final int executeUpdate(String sql) throws SQLException {
634+
checkClosed();
635+
throw new SQLException(
636+
"executeUpdate(String) cannot be called in PreparedStatement or CallableStatement!",
637+
ExceptionUtils.SQL_STATE_WRONG_OBJECT_TYPE);
638+
}
639+
640+
@Override
641+
public final int executeUpdate(String sql, int autoGeneratedKeys) throws SQLException {
642+
checkClosed();
643+
throw new SQLException(
644+
"executeUpdate(String, int) cannot be called in PreparedStatement or CallableStatement!",
645+
ExceptionUtils.SQL_STATE_WRONG_OBJECT_TYPE);
646+
}
647+
648+
@Override
649+
public final int executeUpdate(String sql, int[] columnIndexes) throws SQLException {
650+
checkClosed();
651+
throw new SQLException(
652+
"executeUpdate(String, int[]) cannot be called in PreparedStatement or CallableStatement!",
653+
ExceptionUtils.SQL_STATE_WRONG_OBJECT_TYPE);
654+
}
655+
656+
@Override
657+
public final int executeUpdate(String sql, String[] columnNames) throws SQLException {
658+
checkClosed();
659+
throw new SQLException(
660+
"executeUpdate(String, String[]) cannot be called in PreparedStatement or CallableStatement!",
661+
ExceptionUtils.SQL_STATE_WRONG_OBJECT_TYPE);
662+
}
663+
543664
private static String encodeObject(Object x) throws SQLException {
544665
LOG.trace("Encoding object: {}", x);
545666

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

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ protected void checkClosed() throws SQLException {
5656
}
5757
}
5858

59-
protected enum StatementType {
59+
public enum StatementType {
6060
SELECT, INSERT, DELETE, UPDATE, CREATE, DROP, ALTER, TRUNCATE, USE, SHOW, DESCRIBE, EXPLAIN, SET, KILL, OTHER, INSERT_INTO_SELECT
6161
}
6262

@@ -148,7 +148,7 @@ protected String getLastSql() {
148148
@Override
149149
public ResultSet executeQuery(String sql) throws SQLException {
150150
checkClosed();
151-
return executeQuery(sql, new QuerySettings().setDatabase(schema));
151+
return executeQueryImpl(sql, new QuerySettings().setDatabase(schema));
152152
}
153153

154154
private void closePreviousResultSet() {
@@ -165,7 +165,7 @@ private void closePreviousResultSet() {
165165
}
166166
}
167167

168-
public ResultSetImpl executeQuery(String sql, QuerySettings settings) throws SQLException {
168+
public ResultSetImpl executeQueryImpl(String sql, QuerySettings settings) throws SQLException {
169169
checkClosed();
170170
// Closing before trying to do next request. Otherwise, deadlock because previous connection will not be
171171
// release before this one completes.
@@ -213,13 +213,12 @@ public ResultSetImpl executeQuery(String sql, QuerySettings settings) throws SQL
213213
@Override
214214
public int executeUpdate(String sql) throws SQLException {
215215
checkClosed();
216-
return executeUpdate(sql, new QuerySettings().setDatabase(schema));
216+
return executeUpdateImpl(sql, parseStatementType(sql), new QuerySettings().setDatabase(schema));
217217
}
218218

219-
public int executeUpdate(String sql, QuerySettings settings) throws SQLException {
220-
// TODO: close current result set?
219+
protected int executeUpdateImpl(String sql, StatementType type, QuerySettings settings) throws SQLException {
221220
checkClosed();
222-
StatementType type = parseStatementType(sql);
221+
223222
if (type == StatementType.SELECT || type == StatementType.SHOW || type == StatementType.DESCRIBE || type == StatementType.EXPLAIN) {
224223
throw new SQLException("executeUpdate() cannot be called with a SELECT/SHOW/DESCRIBE/EXPLAIN statement", ExceptionUtils.SQL_STATE_SQL_ERROR);
225224
}
@@ -344,18 +343,16 @@ public void setCursorName(String name) throws SQLException {
344343
@Override
345344
public boolean execute(String sql) throws SQLException {
346345
checkClosed();
347-
return execute(sql, new QuerySettings().setDatabase(schema));
346+
return executeImpl(sql, parseStatementType(sql), new QuerySettings().setDatabase(schema));
348347
}
349348

350-
public boolean execute(String sql, QuerySettings settings) throws SQLException {
349+
public boolean executeImpl(String sql, StatementType type, QuerySettings settings) throws SQLException {
351350
checkClosed();
352-
StatementType type = parseStatementType(sql);
353-
354351
if (type == StatementType.SELECT || type == StatementType.SHOW || type == StatementType.DESCRIBE || type == StatementType.EXPLAIN) {
355-
executeQuery(sql, settings); // keep open to allow getResultSet()
352+
executeQueryImpl(sql, settings); // keep open to allow getResultSet()
356353
return true;
357354
} else if(type == StatementType.SET) {
358-
executeUpdate(sql, settings);
355+
executeUpdateImpl(sql, type, settings);
359356
//SET ROLE
360357
List<String> tokens = JdbcUtils.tokenizeSQL(sql);
361358
if (JdbcUtils.containsIgnoresCase(tokens, "ROLE")) {
@@ -379,14 +376,14 @@ public boolean execute(String sql, QuerySettings settings) throws SQLException {
379376
}
380377
return false;
381378
} else if (type == StatementType.USE) {
382-
executeUpdate(sql, settings);
379+
executeUpdateImpl(sql, type, settings);
383380
//USE Database
384381
List<String> tokens = JdbcUtils.tokenizeSQL(sql);
385382
this.schema = tokens.get(1).replace("\"", "");
386383
LOG.debug("Changed statement schema {}", schema);
387384
return false;
388385
} else {
389-
executeUpdate(sql, settings);
386+
executeUpdateImpl(sql, type, settings);
390387
return false;
391388
}
392389
}

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,10 @@ public final class ExceptionUtils {
2424
public static final String SQL_STATE_INVALID_SCHEMA = "3F000";
2525
public static final String SQL_STATE_INVALID_TX_STATE = "25000";
2626
public static final String SQL_STATE_DATA_EXCEPTION = "22000";
27+
// Used only when feature is not supported
2728
public static final String SQL_STATE_FEATURE_NOT_SUPPORTED = "0A000";
29+
// Used only when method is called on wrong object type (for example, PreparedStatement.addBatch(String))
30+
public static final String SQL_STATE_WRONG_OBJECT_TYPE = "42809";
2831

2932
private ExceptionUtils() {}//Private constructor
3033

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

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

3+
import com.clickhouse.client.api.query.QuerySettings;
34
import org.apache.commons.lang3.RandomStringUtils;
45
import org.testng.Assert;
56
import org.testng.annotations.DataProvider;
@@ -11,6 +12,7 @@
1112
import java.sql.PreparedStatement;
1213
import java.sql.ResultSet;
1314
import java.sql.ResultSetMetaData;
15+
import java.sql.SQLException;
1416
import java.sql.Statement;
1517
import java.sql.Types;
1618
import java.util.*;
@@ -628,4 +630,39 @@ void testWriteCollection() throws Exception {
628630
}
629631

630632
}
633+
634+
@Test
635+
void testMethodsNotAllowedToBeCalled() throws Exception {
636+
/* Story About Broken API
637+
* There is a Statement interface. It is designed to operate with single statements.
638+
* So there are method like execute(String) and addBatch(String).
639+
* Some statements may be repeated over and over again. And they should be constructed
640+
* over and over again. PreparedStatement was created to solve the issue by accepting
641+
* an SQL statement as constructor parameter and making its method work in context of
642+
* one, prepared SQL statement.
643+
* But someone missed their OOP classes and done this:
644+
* "interface PreparedStatement extends Statement"
645+
* and
646+
* declared some method from Statement interface not to be called on PreparedStatement
647+
* instances.
648+
* That is how today we have a great confusion and have to check it in all implementations.
649+
*/
650+
String sql = "SELECT number FROM system.numbers WHERE number = ?";
651+
try (Connection conn = getJdbcConnection();
652+
PreparedStatementImpl ps = (PreparedStatementImpl) conn.prepareStatement(sql)) {
653+
654+
Assert.assertThrows(SQLException.class, () -> ps.addBatch(sql));
655+
Assert.assertThrows(SQLException.class, () -> ps.executeQuery(sql));
656+
Assert.assertThrows(SQLException.class, () -> ps.executeQueryImpl(sql, null));
657+
Assert.assertThrows(SQLException.class, () -> ps.execute(sql));
658+
Assert.assertThrows(SQLException.class, () -> ps.execute(sql, new int[]{0}));
659+
Assert.assertThrows(SQLException.class, () -> ps.execute(sql, new String[]{""}));
660+
Assert.assertThrows(SQLException.class, () -> ps.executeUpdate(sql));
661+
Assert.assertThrows(SQLException.class, () -> ps.executeUpdate(sql, new int[]{0}));
662+
Assert.assertThrows(SQLException.class, () -> ps.executeUpdate(sql, new String[]{""}));
663+
Assert.assertThrows(SQLException.class, () -> ps.executeLargeUpdate(sql));
664+
Assert.assertThrows(SQLException.class, () -> ps.executeLargeUpdate(sql, new int[]{0}));
665+
Assert.assertThrows(SQLException.class, () -> ps.executeLargeUpdate(sql, new String[]{""}));
666+
}
667+
}
631668
}

0 commit comments

Comments
 (0)