Skip to content

Commit 97fec31

Browse files
chernserchernser
authored
Merge pull request #2785 from ClickHouse/03/11/26/fix_no_resultset
Fixed handling unknown statements with result set
2 parents 6788427 + 9e532cf commit 97fec31

5 files changed

Lines changed: 140 additions & 12 deletions

File tree

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ private String buildSQL() throws SQLException {
135135
public ResultSet executeQuery() throws SQLException {
136136
ensureOpen();
137137
String buildSQL = buildSQL();
138-
return super.executeQueryImpl(buildSQL, localSettings);
138+
return super.executeQuery(buildSQL);
139139
}
140140

141141
@Override
@@ -293,9 +293,10 @@ public void setObject(int parameterIndex, Object x) throws SQLException {
293293
@Override
294294
public boolean execute() throws SQLException {
295295
ensureOpen();
296+
currentUpdateCount = -1;
296297
if (parsedPreparedStatement.isHasResultSet()) {
297298
currentResultSet = super.executeQueryImpl(buildSQL(), localSettings);
298-
return true;
299+
return currentResultSet != null;
299300
} else {
300301
currentUpdateCount = super.executeUpdateImpl(buildSQL(), localSettings);
301302
return false;

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

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,10 @@ public ResultSet executeQuery(String sql) throws SQLException {
115115
ensureOpen();
116116
currentUpdateCount = -1;
117117
currentResultSet = executeQueryImpl(sql, localSettings);
118+
if (currentResultSet == null) {
119+
throw new SQLException("executeQuery() requires a ResultSet; use execute() for statements that do not produce one.",
120+
ExceptionUtils.SQL_STATE_CLIENT_ERROR);
121+
}
118122
return currentResultSet;
119123
}
120124

@@ -179,8 +183,26 @@ protected ResultSetImpl executeQueryImpl(String sql, QuerySettings settings) thr
179183
}
180184
ClickHouseBinaryFormatReader reader = connection.getClient().newBinaryFormatReader(response);
181185
if (reader.getSchema() == null) {
182-
reader.close();
183-
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);
186+
long writtenRows = 0L;
187+
try {
188+
writtenRows = response.getWrittenRows();
189+
} catch (Exception ignore) {
190+
// Best effort: leave writtenRows as 0 if we can't obtain it.
191+
}
192+
this.currentUpdateCount = (int) Math.min(writtenRows, Integer.MAX_VALUE);
193+
try {
194+
reader.close();
195+
} catch (Exception closeEx) {
196+
LOG.warn("Failed to close reader when schema is null", closeEx);
197+
} finally {
198+
try {
199+
response.close();
200+
} catch (Exception closeRespEx) {
201+
LOG.warn("Failed to close response when schema is null", closeRespEx);
202+
}
203+
}
204+
onResultSetClosed(null);
205+
return null;
184206
}
185207
return new ResultSetImpl(this, response, reader, this::handleSocketTimeoutException);
186208
} catch (Exception e) {
@@ -342,10 +364,9 @@ public boolean execute(String sql) throws SQLException {
342364
ensureOpen();
343365
parsedStatement = connection.getSqlParser().parsedStatement(sql);
344366
currentUpdateCount = -1;
345-
currentResultSet = null;
346367
if (parsedStatement.isHasResultSet()) {
347368
currentResultSet = executeQueryImpl(sql, localSettings);
348-
return true;
369+
return currentResultSet != null;
349370
} else {
350371
currentUpdateCount = executeUpdateImpl(sql, localSettings);
351372
postUpdateActions();

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

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -67,10 +67,18 @@ public ParsedStatement parsedStatement(String sql) {
6767
}
6868

6969
private boolean isStmtWithResultSet(ClickHouseSqlStatement parsedStmt) {
70-
return parsedStmt.getStatementType() == StatementType.SELECT || parsedStmt.getStatementType() == StatementType.SHOW
71-
|| parsedStmt.getStatementType() == StatementType.EXPLAIN || parsedStmt.getStatementType() == StatementType.DESCRIBE
72-
|| parsedStmt.getStatementType() == StatementType.EXISTS || parsedStmt.getStatementType() == StatementType.CHECK;
73-
70+
switch (parsedStmt.getStatementType()) {
71+
case SELECT:
72+
case SHOW:
73+
case EXPLAIN:
74+
case DESCRIBE:
75+
case EXISTS:
76+
case CHECK:
77+
case UNKNOWN:
78+
return true;
79+
default:
80+
return false;
81+
}
7482
}
7583

7684
@Override
@@ -162,14 +170,19 @@ public ANTLR4Parser(boolean saveRoles) {
162170
public ParsedStatement parsedStatement(String sql) {
163171
ParsedStatement stmt = new ParsedStatement();
164172
parseSQL(sql, new ParsedStatementListener(stmt, processUseRolesExpr));
173+
if (stmt.isHasErrors()) {
174+
stmt.setHasResultSet(true);
175+
}
165176
return stmt;
166177
}
167178

168179
@Override
169180
public ParsedPreparedStatement parsePreparedStatement(String sql) {
170181
ParsedPreparedStatement stmt = new ParsedPreparedStatement();
171182
parseSQL(sql, new ParsedPreparedStatementListener(stmt, processUseRolesExpr));
172-
183+
if (stmt.isHasErrors()) {
184+
stmt.setHasResultSet(true);
185+
}
173186
// Combine database and table like JavaCC does
174187
String tableName = stmt.getTable();
175188
if (stmt.getDatabase() != null && stmt.getTable() != null) {
@@ -395,7 +408,9 @@ public ANTLR4AndParamsParser(boolean saveRoles) {
395408
public ParsedPreparedStatement parsePreparedStatement(String sql) {
396409
ParsedPreparedStatement stmt = new ParsedPreparedStatement();
397410
parseSQL(sql, new ParseStatementAndParamsListener(stmt, processUseRolesExpr));
398-
411+
if (stmt.isHasErrors()) {
412+
stmt.setHasResultSet(true);
413+
}
399414
// Combine database and table like JavaCC does
400415
String tableName = stmt.getTable();
401416
if (stmt.getDatabase() != null && stmt.getTable() != null) {

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

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1785,4 +1785,51 @@ void testDateWithAndWithoutCalendar() throws Exception {
17851785
}
17861786
}
17871787
}
1788+
1789+
@Test(groups = {"integration"})
1790+
public void testUnknownStatement() throws Exception {
1791+
try (Connection conn = getJdbcConnection()) {
1792+
try (PreparedStatement stmt = conn.prepareStatement("SELECT number, FROM system.numbers LIMIT 3")) {
1793+
Assert.assertTrue(stmt.execute());
1794+
1795+
try (ResultSet rs = stmt.getResultSet()) {
1796+
for (int i = 0; i < 3; i++) {
1797+
Assert.assertTrue(rs.next());
1798+
Assert.assertEquals(rs.getLong(1), i);
1799+
}
1800+
}
1801+
}
1802+
1803+
try (Statement stmt = conn.createStatement()) {
1804+
Assert.assertTrue(stmt.execute("SELECT number, FROM system.numbers LIMIT 3"));
1805+
1806+
try (ResultSet rs = stmt.getResultSet()) {
1807+
for (int i = 0; i < 3; i++) {
1808+
Assert.assertTrue(rs.next());
1809+
Assert.assertEquals(rs.getLong(1), i);
1810+
}
1811+
}
1812+
}
1813+
1814+
// No-ResultSet path: DDL should not produce a ResultSet
1815+
String tmpTable = "tmp_no_result_" + RandomStringUtils.randomAlphanumeric(8);
1816+
// PreparedStatement: execute() should return false, executeQuery() should throw
1817+
try (PreparedStatement stmt = conn.prepareStatement(
1818+
"CREATE TABLE " + tmpTable + " (x Int32) Engine MergeTree ORDER BY()")) {
1819+
Assert.assertFalse(stmt.execute(), "DDL should not produce a ResultSet");
1820+
Assert.assertNull(stmt.getResultSet(), "ResultSet should be null for DDL");
1821+
assertThrows(SQLException.class, stmt::executeQuery);
1822+
}
1823+
// Statement: execute() should return false, executeQuery() should throw
1824+
String tmpTable2 = "tmp_no_result_" + RandomStringUtils.randomAlphanumeric(8);
1825+
try (Statement stmt = conn.createStatement()) {
1826+
Assert.assertFalse(
1827+
stmt.execute("CREATE TABLE " + tmpTable2 + " (x Int32) Engine MergeTree ORDER BY()"),
1828+
"DDL should not produce a ResultSet");
1829+
Assert.assertNull(stmt.getResultSet(), "ResultSet should be null for DDL");
1830+
assertThrows(SQLException.class,
1831+
() -> stmt.executeQuery("CREATE TABLE " + tmpTable2 + " (x Int32) Engine MergeTree ORDER BY()"));
1832+
}
1833+
}
1834+
}
17881835
}

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

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1322,6 +1322,50 @@ public void testDescribeStatement() throws Exception {
13221322
}
13231323
}
13241324

1325+
@Test(groups = {"integration"}, dataProvider = "testUnknownStatementTest_DP")
1326+
public void testUnknownStatement(String parserName) throws Exception {
1327+
Properties properties = new Properties();
1328+
properties.setProperty(DriverProperties.SQL_PARSER.getKey(), parserName);
1329+
try (Connection conn = getJdbcConnection(properties)) {
1330+
1331+
try (Statement stmt = conn.createStatement()) {
1332+
Assert.assertTrue(stmt.execute("SELECT number, FROM system.numbers LIMIT 3"));
1333+
1334+
try (ResultSet rs = stmt.getResultSet()) {
1335+
for (int i = 0; i < 3; i++) {
1336+
Assert.assertTrue(rs.next());
1337+
Assert.assertEquals(rs.getLong(1), i);
1338+
}
1339+
}
1340+
1341+
1342+
stmt.execute("DROP TABLE IF EXISTS test_unknown_statement_test");
1343+
stmt.execute("CREATE TABLE test_unknown_statement_test (v Int32) Engine MergeTree ORDER BY ()");
1344+
1345+
// INSERT via execute(...) must not produce a ResultSet and should return false
1346+
boolean hasResultSet = stmt.execute("INSERT INTO test_unknown_statement_test VALUES (1);");
1347+
assertFalse(hasResultSet);
1348+
assertEquals(stmt.getUpdateCount(), 1);
1349+
1350+
stmt.executeUpdate("INSERT INTO test_unknown_statement_test VALUES (2);");
1351+
assertEquals(stmt.getUpdateCount(), 1);
1352+
1353+
assertThrows(SQLException.class,
1354+
() -> stmt.executeQuery("INSERT INTO test_unknown_statement_test VALUES (3);"));
1355+
1356+
}
1357+
}
1358+
}
1359+
1360+
@DataProvider
1361+
public static Object[][] testUnknownStatementTest_DP() {
1362+
return new Object[][] {
1363+
{SqlParserFacade.SQLParser.ANTLR4.name()},
1364+
{SqlParserFacade.SQLParser.ANTLR4_PARAMS_PARSER.name()},
1365+
{SqlParserFacade.SQLParser.JAVACC.name()},
1366+
};
1367+
}
1368+
13251369
private static String getDBName(Statement stmt) throws SQLException {
13261370
try (ResultSet rs = stmt.executeQuery("SELECT database()")) {
13271371
rs.next();

0 commit comments

Comments
 (0)