diff --git a/jdbc-v2/src/main/java/com/clickhouse/jdbc/PreparedStatementImpl.java b/jdbc-v2/src/main/java/com/clickhouse/jdbc/PreparedStatementImpl.java index a29a220f1..bce734a6a 100644 --- a/jdbc-v2/src/main/java/com/clickhouse/jdbc/PreparedStatementImpl.java +++ b/jdbc-v2/src/main/java/com/clickhouse/jdbc/PreparedStatementImpl.java @@ -135,7 +135,7 @@ private String buildSQL() throws SQLException { public ResultSet executeQuery() throws SQLException { ensureOpen(); String buildSQL = buildSQL(); - return super.executeQueryImpl(buildSQL, localSettings); + return super.executeQuery(buildSQL); } @Override @@ -293,9 +293,10 @@ public void setObject(int parameterIndex, Object x) throws SQLException { @Override public boolean execute() throws SQLException { ensureOpen(); + currentUpdateCount = -1; if (parsedPreparedStatement.isHasResultSet()) { currentResultSet = super.executeQueryImpl(buildSQL(), localSettings); - return true; + return currentResultSet != null; } else { currentUpdateCount = super.executeUpdateImpl(buildSQL(), localSettings); return false; diff --git a/jdbc-v2/src/main/java/com/clickhouse/jdbc/StatementImpl.java b/jdbc-v2/src/main/java/com/clickhouse/jdbc/StatementImpl.java index a814718ad..f50546393 100644 --- a/jdbc-v2/src/main/java/com/clickhouse/jdbc/StatementImpl.java +++ b/jdbc-v2/src/main/java/com/clickhouse/jdbc/StatementImpl.java @@ -115,6 +115,10 @@ public ResultSet executeQuery(String sql) throws SQLException { ensureOpen(); currentUpdateCount = -1; currentResultSet = executeQueryImpl(sql, localSettings); + if (currentResultSet == null) { + throw new SQLException("executeQuery() requires a ResultSet; use execute() for statements that do not produce one.", + ExceptionUtils.SQL_STATE_CLIENT_ERROR); + } return currentResultSet; } @@ -179,8 +183,26 @@ protected ResultSetImpl executeQueryImpl(String sql, QuerySettings settings) thr } ClickHouseBinaryFormatReader reader = connection.getClient().newBinaryFormatReader(response); if (reader.getSchema() == null) { - reader.close(); - 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); + long writtenRows = 0L; + try { + writtenRows = response.getWrittenRows(); + } catch (Exception ignore) { + // Best effort: leave writtenRows as 0 if we can't obtain it. + } + this.currentUpdateCount = (int) Math.min(writtenRows, Integer.MAX_VALUE); + try { + reader.close(); + } catch (Exception closeEx) { + LOG.warn("Failed to close reader when schema is null", closeEx); + } finally { + try { + response.close(); + } catch (Exception closeRespEx) { + LOG.warn("Failed to close response when schema is null", closeRespEx); + } + } + onResultSetClosed(null); + return null; } return new ResultSetImpl(this, response, reader, this::handleSocketTimeoutException); } catch (Exception e) { @@ -342,10 +364,9 @@ public boolean execute(String sql) throws SQLException { ensureOpen(); parsedStatement = connection.getSqlParser().parsedStatement(sql); currentUpdateCount = -1; - currentResultSet = null; if (parsedStatement.isHasResultSet()) { currentResultSet = executeQueryImpl(sql, localSettings); - return true; + return currentResultSet != null; } else { currentUpdateCount = executeUpdateImpl(sql, localSettings); postUpdateActions(); diff --git a/jdbc-v2/src/main/java/com/clickhouse/jdbc/internal/SqlParserFacade.java b/jdbc-v2/src/main/java/com/clickhouse/jdbc/internal/SqlParserFacade.java index f819fea49..406ec013d 100644 --- a/jdbc-v2/src/main/java/com/clickhouse/jdbc/internal/SqlParserFacade.java +++ b/jdbc-v2/src/main/java/com/clickhouse/jdbc/internal/SqlParserFacade.java @@ -67,10 +67,18 @@ public ParsedStatement parsedStatement(String sql) { } private boolean isStmtWithResultSet(ClickHouseSqlStatement parsedStmt) { - return parsedStmt.getStatementType() == StatementType.SELECT || parsedStmt.getStatementType() == StatementType.SHOW - || parsedStmt.getStatementType() == StatementType.EXPLAIN || parsedStmt.getStatementType() == StatementType.DESCRIBE - || parsedStmt.getStatementType() == StatementType.EXISTS || parsedStmt.getStatementType() == StatementType.CHECK; - + switch (parsedStmt.getStatementType()) { + case SELECT: + case SHOW: + case EXPLAIN: + case DESCRIBE: + case EXISTS: + case CHECK: + case UNKNOWN: + return true; + default: + return false; + } } @Override @@ -162,6 +170,9 @@ public ANTLR4Parser(boolean saveRoles) { public ParsedStatement parsedStatement(String sql) { ParsedStatement stmt = new ParsedStatement(); parseSQL(sql, new ParsedStatementListener(stmt, processUseRolesExpr)); + if (stmt.isHasErrors()) { + stmt.setHasResultSet(true); + } return stmt; } @@ -169,7 +180,9 @@ public ParsedStatement parsedStatement(String sql) { public ParsedPreparedStatement parsePreparedStatement(String sql) { ParsedPreparedStatement stmt = new ParsedPreparedStatement(); parseSQL(sql, new ParsedPreparedStatementListener(stmt, processUseRolesExpr)); - + if (stmt.isHasErrors()) { + stmt.setHasResultSet(true); + } // Combine database and table like JavaCC does String tableName = stmt.getTable(); if (stmt.getDatabase() != null && stmt.getTable() != null) { @@ -395,7 +408,9 @@ public ANTLR4AndParamsParser(boolean saveRoles) { public ParsedPreparedStatement parsePreparedStatement(String sql) { ParsedPreparedStatement stmt = new ParsedPreparedStatement(); parseSQL(sql, new ParseStatementAndParamsListener(stmt, processUseRolesExpr)); - + if (stmt.isHasErrors()) { + stmt.setHasResultSet(true); + } // Combine database and table like JavaCC does String tableName = stmt.getTable(); if (stmt.getDatabase() != null && stmt.getTable() != null) { diff --git a/jdbc-v2/src/test/java/com/clickhouse/jdbc/PreparedStatementTest.java b/jdbc-v2/src/test/java/com/clickhouse/jdbc/PreparedStatementTest.java index b644ad52a..d5bfb7978 100644 --- a/jdbc-v2/src/test/java/com/clickhouse/jdbc/PreparedStatementTest.java +++ b/jdbc-v2/src/test/java/com/clickhouse/jdbc/PreparedStatementTest.java @@ -1785,4 +1785,51 @@ void testDateWithAndWithoutCalendar() throws Exception { } } } + + @Test(groups = {"integration"}) + public void testUnknownStatement() throws Exception { + try (Connection conn = getJdbcConnection()) { + try (PreparedStatement stmt = conn.prepareStatement("SELECT number, FROM system.numbers LIMIT 3")) { + Assert.assertTrue(stmt.execute()); + + try (ResultSet rs = stmt.getResultSet()) { + for (int i = 0; i < 3; i++) { + Assert.assertTrue(rs.next()); + Assert.assertEquals(rs.getLong(1), i); + } + } + } + + try (Statement stmt = conn.createStatement()) { + Assert.assertTrue(stmt.execute("SELECT number, FROM system.numbers LIMIT 3")); + + try (ResultSet rs = stmt.getResultSet()) { + for (int i = 0; i < 3; i++) { + Assert.assertTrue(rs.next()); + Assert.assertEquals(rs.getLong(1), i); + } + } + } + + // No-ResultSet path: DDL should not produce a ResultSet + String tmpTable = "tmp_no_result_" + RandomStringUtils.randomAlphanumeric(8); + // PreparedStatement: execute() should return false, executeQuery() should throw + try (PreparedStatement stmt = conn.prepareStatement( + "CREATE TABLE " + tmpTable + " (x Int32) Engine MergeTree ORDER BY()")) { + Assert.assertFalse(stmt.execute(), "DDL should not produce a ResultSet"); + Assert.assertNull(stmt.getResultSet(), "ResultSet should be null for DDL"); + assertThrows(SQLException.class, stmt::executeQuery); + } + // Statement: execute() should return false, executeQuery() should throw + String tmpTable2 = "tmp_no_result_" + RandomStringUtils.randomAlphanumeric(8); + try (Statement stmt = conn.createStatement()) { + Assert.assertFalse( + stmt.execute("CREATE TABLE " + tmpTable2 + " (x Int32) Engine MergeTree ORDER BY()"), + "DDL should not produce a ResultSet"); + Assert.assertNull(stmt.getResultSet(), "ResultSet should be null for DDL"); + assertThrows(SQLException.class, + () -> stmt.executeQuery("CREATE TABLE " + tmpTable2 + " (x Int32) Engine MergeTree ORDER BY()")); + } + } + } } diff --git a/jdbc-v2/src/test/java/com/clickhouse/jdbc/StatementTest.java b/jdbc-v2/src/test/java/com/clickhouse/jdbc/StatementTest.java index 3aba8b652..575dde388 100644 --- a/jdbc-v2/src/test/java/com/clickhouse/jdbc/StatementTest.java +++ b/jdbc-v2/src/test/java/com/clickhouse/jdbc/StatementTest.java @@ -1322,6 +1322,50 @@ public void testDescribeStatement() throws Exception { } } + @Test(groups = {"integration"}, dataProvider = "testUnknownStatementTest_DP") + public void testUnknownStatement(String parserName) throws Exception { + Properties properties = new Properties(); + properties.setProperty(DriverProperties.SQL_PARSER.getKey(), parserName); + try (Connection conn = getJdbcConnection(properties)) { + + try (Statement stmt = conn.createStatement()) { + Assert.assertTrue(stmt.execute("SELECT number, FROM system.numbers LIMIT 3")); + + try (ResultSet rs = stmt.getResultSet()) { + for (int i = 0; i < 3; i++) { + Assert.assertTrue(rs.next()); + Assert.assertEquals(rs.getLong(1), i); + } + } + + + stmt.execute("DROP TABLE IF EXISTS test_unknown_statement_test"); + stmt.execute("CREATE TABLE test_unknown_statement_test (v Int32) Engine MergeTree ORDER BY ()"); + + // INSERT via execute(...) must not produce a ResultSet and should return false + boolean hasResultSet = stmt.execute("INSERT INTO test_unknown_statement_test VALUES (1);"); + assertFalse(hasResultSet); + assertEquals(stmt.getUpdateCount(), 1); + + stmt.executeUpdate("INSERT INTO test_unknown_statement_test VALUES (2);"); + assertEquals(stmt.getUpdateCount(), 1); + + assertThrows(SQLException.class, + () -> stmt.executeQuery("INSERT INTO test_unknown_statement_test VALUES (3);")); + + } + } + } + + @DataProvider + public static Object[][] testUnknownStatementTest_DP() { + return new Object[][] { + {SqlParserFacade.SQLParser.ANTLR4.name()}, + {SqlParserFacade.SQLParser.ANTLR4_PARAMS_PARSER.name()}, + {SqlParserFacade.SQLParser.JAVACC.name()}, + }; + } + private static String getDBName(Statement stmt) throws SQLException { try (ResultSet rs = stmt.executeQuery("SELECT database()")) { rs.next();