diff --git a/CHANGELOG.md b/CHANGELOG.md index 52c92a57a..88fc5c218 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,6 +47,12 @@ - **[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) +- **[jdbc-v2]** Fixed the default JavaCC SQL parser aborting on a heredoc string (`$$body$$`, `$tag$body$tag$`) + whose body contains a character that is not a valid SQL token on its own, such as `!`, `&`, `|` or `~`. The + lexer had no heredoc token, so such a body raised a lexer error that left the statement classified as + `UNKNOWN` — an INSERT was reported as a result-set-bearing statement with no table name and no values-list + positions, which disables the batch values template and the table-name based paths. A heredoc is now lexed + as a single string literal. (https://github.com/ClickHouse/clickhouse-java/issues/3029) - **[client-v2, jdbc-v2]** Reduced noisy and potentially sensitive logging; SQL that fails to parse is no longer logged at `WARN` (it could contain credentials/PII). (https://github.com/ClickHouse/clickhouse-java/issues/2970) - **[client-v2]** Fixed `BigDecimal` values written into a `Dynamic` column being silently truncated when the diff --git a/jdbc-v2/src/main/javacc/ClickHouseSqlParser.jj b/jdbc-v2/src/main/javacc/ClickHouseSqlParser.jj index d0c088615..7456f863f 100644 --- a/jdbc-v2/src/main/javacc/ClickHouseSqlParser.jj +++ b/jdbc-v2/src/main/javacc/ClickHouseSqlParser.jj @@ -1007,6 +1007,7 @@ Token literal(): { Token t = null; } { t = dateLiteral() | t = numberLiteral() | t = + | t = | t = ) { return t; } @@ -1296,6 +1297,15 @@ TOKEN: { ( ~[] | ~["'", "\\"] | "''")* > } +// heredoc string literal: $$body$$ or $tag$body$tag$ +// Matched loosely, like the rest of this grammar: the opening and closing tags are not required to +// be equal and a body cannot contain '$'. An unterminated tag (e.g. `$foo$bar`) does not match and +// keeps being lexed as an identifier, which is also how the server reads it. +TOKEN: { + (~["$"])* > + | <#HEREDOC_TAG: ( | | )* > +} + TOKEN: { | | | ) ( | | | )* diff --git a/jdbc-v2/src/test/java/com/clickhouse/jdbc/internal/BaseSqlParserFacadeTest.java b/jdbc-v2/src/test/java/com/clickhouse/jdbc/internal/BaseSqlParserFacadeTest.java index 945701ad0..0355b60c2 100644 --- a/jdbc-v2/src/test/java/com/clickhouse/jdbc/internal/BaseSqlParserFacadeTest.java +++ b/jdbc-v2/src/test/java/com/clickhouse/jdbc/internal/BaseSqlParserFacadeTest.java @@ -257,6 +257,74 @@ private void testCase(String sql, String expectedTableName) { Assert.assertEquals(stmt.getTable(), expectedTableName, "Table name mismatch for: " + sql); } + @Test(dataProvider = "heredocStatementsDP") + public void testHeredocStatements(String sql, boolean insert, String expectedTable, String expectedValuesList) { + ParsedPreparedStatement stmt = parser.parsePreparedStatement(sql); + Assert.assertFalse(stmt.isHasErrors(), "Query should parse without errors: " + sql); + Assert.assertEquals(stmt.isInsert(), insert, "Insert type mismatch for: " + sql); + Assert.assertEquals(stmt.isHasResultSet(), !insert, "Result set flag mismatch for: " + sql); + Assert.assertEquals(stmt.getTable(), expectedTable, "Table name mismatch for: " + sql); + if (expectedValuesList == null) { + Assert.assertEquals(stmt.getAssignValuesListStartPosition(), -1, "Should have no values list: " + sql); + } else { + Assert.assertEquals(sql.substring(stmt.getAssignValuesListStartPosition(), + stmt.getAssignValuesListStopPosition() + 1), expectedValuesList, + "Values list mismatch for: " + sql); + } + } + + @DataProvider + public static Object[][] heredocStatementsDP() { + return new Object[][] { + // A heredoc body is opaque: characters that are not valid SQL tokens on their own + // must not break the statement classification + {"INSERT INTO t VALUES ($$a!b$$, 1)", true, "t", "($$a!b$$, 1)"}, + {"INSERT INTO t VALUES ($$a&b$$, 1)", true, "t", "($$a&b$$, 1)"}, + {"INSERT INTO t VALUES ($$a|b$$, 1)", true, "t", "($$a|b$$, 1)"}, + {"INSERT INTO t VALUES ($$a~b$$, 1)", true, "t", "($$a~b$$, 1)"}, + {"INSERT INTO t VALUES ($$a@b$$, 1)", true, "t", "($$a@b$$, 1)"}, + // Tagged form and a body with whitespace + {"INSERT INTO t (c1, c2) VALUES ($tag_1$a!b$tag_1$, 1)", true, "t", "($tag_1$a!b$tag_1$, 1)"}, + {"INSERT INTO t VALUES ($$a b$$, 1)", true, "t", "($$a b$$, 1)"}, + // Parentheses and commas in a body must not shift the values list positions + {"INSERT INTO t VALUES ($$a(b,c)$$, 1)", true, "t", "($$a(b,c)$$, 1)"}, + // Two heredocs in one values list are two separate literals + {"INSERT INTO t VALUES ($$a!b$$, $$c!d$$)", true, "t", "($$a!b$$, $$c!d$$)"}, + // A heredoc is a value expression anywhere a string literal is accepted + {"SELECT $$a!b$$ AS x FROM t", false, "t", null}, + // Contrast: an unterminated tag is not a heredoc and stays an identifier + {"SELECT $foo$bar FROM t", false, "t", null}, + {"SELECT a$b FROM t", false, "t", null}, + // Contrast: a quoted string literal keeps its existing handling + {"INSERT INTO t VALUES ('a!b', 1)", true, "t", "('a!b', 1)"}, + }; + } + + @Test(dataProvider = "javaCcHeredocStatementsDP") + public void testHeredocStatementsJavaCcOnly(String sql, String expectedValuesList) { + // The ANTLR4 grammars do not accept these two heredoc bodies yet, so the expectations only + // hold for the JavaCC backend. + if (!javaCcBackend) { + return; + } + ParsedPreparedStatement stmt = parser.parsePreparedStatement(sql); + Assert.assertFalse(stmt.isHasErrors(), "Query should parse without errors: " + sql); + Assert.assertTrue(stmt.isInsert(), "Should be an INSERT: " + sql); + Assert.assertEquals(sql.substring(stmt.getAssignValuesListStartPosition(), + stmt.getAssignValuesListStopPosition() + 1), expectedValuesList, + "Values list mismatch for: " + sql); + } + + @DataProvider + public static Object[][] javaCcHeredocStatementsDP() { + return new Object[][] { + // A statement separator inside a heredoc body must not split the statement + {"INSERT INTO t VALUES ($$a;b$$, 1)", "($$a;b$$, 1)"}, + // Empty body + {"INSERT INTO t VALUES ($$$$, 1)", "($$$$, 1)"}, + }; + } + @Test public void testInsertColumnNamesAreUnescaped() { /*