diff --git a/CHANGELOG.md b/CHANGELOG.md index 52c92a57a..472fc7b41 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,6 +44,14 @@ ### Bug Fixes +- **[clickhouse-jdbc]** Fixed `Connection#prepareStatement` throwing a `NullPointerException` for an + `INSERT ... VALUES (...)` statement whose values list the JavaCC parser cannot parse — most commonly one + containing a heredoc string (`$$...$$`), which the grammar has no token for, but also any other unparsable token + inside the list. The parser's error recovery left the values list's start position recorded without its matching end + position, which was then unboxed unguarded. Both positions are now dropped together, so the driver falls back to its + generic parameter-substitution path instead of failing, and a statement such as + `insert into t values ($$a@b$$, ?)` is prepared and executed successfully. + (https://github.com/ClickHouse/clickhouse-java/issues/3033) - **[client-v2]** Fixed LZ4 input streams not closing their underlying HTTP response stream. Closing an LZ4 stream returned by `QueryResponse.getInputStream()` now releases the wrapped transport stream, including after a partial read. (https://github.com/ClickHouse/clickhouse-java/issues/2985) diff --git a/clickhouse-jdbc/src/main/java/com/clickhouse/jdbc/internal/ClickHouseConnectionImpl.java b/clickhouse-jdbc/src/main/java/com/clickhouse/jdbc/internal/ClickHouseConnectionImpl.java index 154e3dcdf..72354dc3c 100644 --- a/clickhouse-jdbc/src/main/java/com/clickhouse/jdbc/internal/ClickHouseConnectionImpl.java +++ b/clickhouse-jdbc/src/main/java/com/clickhouse/jdbc/internal/ClickHouseConnectionImpl.java @@ -821,9 +821,10 @@ public PreparedStatement prepareStatement(String sql, int resultSetType, int res String query = parsedStmt.getSQL(); boolean useStream = false; Integer startIndex = parsedStmt.getPositions().get(ClickHouseSqlStatement.KEYWORD_VALUES_START); - if (startIndex != null) { + Integer stopIndex = parsedStmt.getPositions().get(ClickHouseSqlStatement.KEYWORD_VALUES_END); + if (startIndex != null && stopIndex != null) { useStream = true; - int endIndex = parsedStmt.getPositions().get(ClickHouseSqlStatement.KEYWORD_VALUES_END); + int endIndex = stopIndex; for (int i = startIndex + 1; i < endIndex; i++) { char ch = query.charAt(i); if (ch != '?' && ch != ',' && !Character.isWhitespace(ch)) { diff --git a/clickhouse-jdbc/src/main/javacc/ClickHouseSqlParser.jj b/clickhouse-jdbc/src/main/javacc/ClickHouseSqlParser.jj index e3eadced5..7cfb0f87c 100644 --- a/clickhouse-jdbc/src/main/javacc/ClickHouseSqlParser.jj +++ b/clickhouse-jdbc/src/main/javacc/ClickHouseSqlParser.jj @@ -592,6 +592,10 @@ void dataClause(): {} { { token_source.format = token.image; } )? (anyExprList())? } catch (ParseException e) { // FIXME introduce a lexical state in next release with consideration of delimiter from the context + // The values list was abandoned mid-way, so its start/end positions can only be recorded partially. + // Drop both so consumers either get a complete pair or none at all. + token_source.removePosition(ClickHouseSqlStatement.KEYWORD_VALUES_START); + token_source.removePosition(ClickHouseSqlStatement.KEYWORD_VALUES_END); Token nextToken; do { nextToken = getNextToken(); diff --git a/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/ClickHousePreparedStatementTest.java b/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/ClickHousePreparedStatementTest.java index 3226471b2..0f2c06abd 100644 --- a/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/ClickHousePreparedStatementTest.java +++ b/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/ClickHousePreparedStatementTest.java @@ -665,6 +665,27 @@ public void testReadWriteString() throws SQLException { } } + @Test(groups = "integration") + public void testInsertWithHeredocValue() throws SQLException { + try (ClickHouseConnection conn = newConnection(new Properties()); + ClickHouseStatement s = conn.createStatement()) { + s.execute("drop table if exists test_insert_heredoc;" + + "CREATE TABLE test_insert_heredoc(s String, n Int32) ENGINE = MergeTree() ORDER BY n"); + try (PreparedStatement ps = conn + .prepareStatement("insert into test_insert_heredoc values ($$a@b$$, ?)")) { + ps.setInt(1, 42); + Assert.assertEquals(ps.executeUpdate(), 1); + } + + try (ResultSet rs = s.executeQuery("select s, n from test_insert_heredoc")) { + Assert.assertTrue(rs.next()); + Assert.assertEquals(rs.getString(1), "a@b"); + Assert.assertEquals(rs.getInt(2), 42); + Assert.assertFalse(rs.next()); + } + } + } + @Test(groups = "integration") public void testInsertQueryDateTime64() throws SQLException { try (ClickHouseConnection conn = newConnection(new Properties()); diff --git a/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/parser/ClickHouseSqlParserFacadeTest.java b/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/parser/ClickHouseSqlParserFacadeTest.java index cfcfb211e..2ca9a0d83 100644 --- a/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/parser/ClickHouseSqlParserFacadeTest.java +++ b/clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/parser/ClickHouseSqlParserFacadeTest.java @@ -1,6 +1,7 @@ package com.clickhouse.jdbc.parser; import org.testng.Assert; +import org.testng.annotations.DataProvider; import org.testng.annotations.Test; import static org.testng.Assert.assertEquals; @@ -862,6 +863,50 @@ public void testSETRoleStatements() { } } + @Test(groups = "unit", dataProvider = "unparsableValuesListProvider") + public void testInsertWithUnparsableValuesList(String sql) { + ClickHouseSqlStatement s = parse(sql)[0]; + assertEquals(s.getStatementType(), StatementType.INSERT); + + Integer start = s.getPositions().get(ClickHouseSqlStatement.KEYWORD_VALUES_START); + Integer end = s.getPositions().get(ClickHouseSqlStatement.KEYWORD_VALUES_END); + assertEquals(start == null, end == null, + "Values list start and end positions should be both set or both unset"); + if (start != null) { + Assert.assertTrue(end > start, "Values list should end after it starts"); + assertEquals(sql.charAt(start), '('); + assertEquals(sql.charAt(end), ')'); + } + } + + @DataProvider(name = "unparsableValuesListProvider") + private static Object[][] getUnparsableValuesLists() { + return new Object[][] { + { "INSERT INTO t VALUES ($$a@b$$, ?)" }, + { "INSERT INTO t VALUES ($$a@b$$, ?);" }, + { "INSERT INTO t VALUES ($$?$$, ?)" }, + { "INSERT INTO t VALUES (?, )" }, + { "INSERT INTO t VALUES (@@, ?)" }, + { "INSERT INTO t VALUES (1, ?), (@@, ?)" }, + }; + } + + @Test(groups = "unit", dataProvider = "parsableValuesListProvider") + public void testInsertWithParsableValuesList(String sql, int start, int end) { + ClickHouseSqlStatement s = parse(sql)[0]; + assertEquals(s.getPositions().get(ClickHouseSqlStatement.KEYWORD_VALUES_START), Integer.valueOf(start)); + assertEquals(s.getPositions().get(ClickHouseSqlStatement.KEYWORD_VALUES_END), Integer.valueOf(end)); + } + + @DataProvider(name = "parsableValuesListProvider") + private static Object[][] getParsableValuesLists() { + return new Object[][] { + { "INSERT INTO t VALUES (?, ?)", 21, 26 }, + { "INSERT INTO t (a, b) VALUES (1, ?)", 28, 33 }, + { "INSERT INTO t VALUES ('a@b', ?)", 21, 30 }, + }; + } + // known issue public void testTernaryOperator() { String sql = "select x > 2 ? 'a' : 'b' from (select number as x from system.numbers limit ?)";