Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,13 @@

### Bug Fixes

- **[jdbc-v2]** Fixed the ANTLR4 lexer not nesting `/* */` block comments. ClickHouse (and the JavaCC parser backend)
raise the nesting level on an inner `/*` and close the comment only at the matching `*/`, while the ANTLR4 lexer ended
the comment at the first `*/` and lexed the rest of it as SQL. With the `ANTLR4` / `ANTLR4_PARAMS_PARSER` backends this
made statements the server accepts (e.g. `SELECT 1 /* ) /* ) */ ) */, 2`) report syntax errors, and made
`ANTLR4_PARAMS_PARSER` count a `?` inside the nested part of a comment as a bind parameter. Comments that do not nest
are unaffected; an unterminated block comment is now skipped to the end of the statement instead of being lexed as
stray tokens. (https://github.com/ClickHouse/clickhouse-java/issues/3021)
- **[jdbc-v2]** Fixed `PreparedStatement#executeBatch` sending a syntactically broken `INSERT` when an `ANTLR4` parser
backend is selected (`jdbc_sql_parser=ANTLR4` / `ANTLR4_PARAMS_PARSER`) and the values list contains a value
expression the bundled grammar cannot parse - a JDBC escape sequence (`{d '...'}`), or valid ClickHouse syntax the
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -507,6 +507,17 @@ UNDERSCORE : '_';

// Comments and whitespace

MULTI_LINE_COMMENT : '/*' -> skip, pushMode(IN_MULTI_LINE_COMMENT);
SINGLE_LINE_COMMENT : ('--' | '#!' | '#') ~('\n' | '\r')* ('\n' | '\r' | EOF) -> skip;
Comment thread
cursor[bot] marked this conversation as resolved.
Outdated
WHITESPACE : [ \u000B\u000C\t\r\n] -> skip; // '\n' can be part of multiline single query

// Block comments nest, as they do on the server: the mode stack holds the nesting level, so only the
// closer matching the outermost '/*' ends the comment.
mode IN_MULTI_LINE_COMMENT;

NESTED_MULTI_LINE_COMMENT : '/*' -> skip, pushMode(IN_MULTI_LINE_COMMENT);
MULTI_LINE_COMMENT_END : '*/' -> skip, popMode;
MULTI_LINE_COMMENT_BODY : . -> skip;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unterminated comment lacks EOF rule

Low Severity

IN_MULTI_LINE_COMMENT has no EOF alternative. An unterminated /* still reaches end-of-input in that mode, so ANTLR raises LexerNoViableAltException and the default console listener emits a token recognition error. The parse may still succeed without hasErrors, which conflicts with the intended quiet skip-to-EOF behavior for unterminated block comments.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit be5c96c. Configure here.

MULTI_LINE_COMMENT : '/*' .*? '*/' -> skip;
SINGLE_LINE_COMMENT : ('--' | '//' | '#!' | '#') ~('\n' | '\r')* ('\n' | '\r' | EOF) -> skip;
WHITESPACE : [ \u000B\u000C\t\r\n] -> skip; // '\n' can be part of multiline single query
WHITESPACE : [ \u000B\u000C\t\r\n] -> skip; // '\n' can be part of multiline single query
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import java.io.InputStreamReader;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Properties;
import java.util.stream.Collectors;
Expand Down Expand Up @@ -1029,6 +1030,52 @@ public static Object[][] testStatementWithoutResultSetDP() {
};
}

@Test(dataProvider = "testBlockCommentNestingDP")
public void testBlockCommentNesting(String sql, int[] paramPositions, boolean insert, boolean hasResultSet) {
ParsedPreparedStatement stmt = parser.parsePreparedStatement(sql);
Assert.assertFalse(stmt.isHasErrors(), "Statement has errors: " + sql);
assertEquals(stmt.getArgCount(), paramPositions.length, "Args do not match for: " + sql);
assertEquals(Arrays.copyOf(stmt.getParamPositions(), stmt.getArgCount()), paramPositions,
"Parameter positions do not match for: " + sql);
assertEquals(stmt.isInsert(), insert, "Insert type does not match for: " + sql);
assertEquals(stmt.isHasResultSet(), hasResultSet, "Result set expectation does not match for: " + sql);
}

@DataProvider
public static Object[][] testBlockCommentNestingDP() {
StringBuilder deeplyNested = new StringBuilder("SELECT ");
for (int i = 0; i < 64; i++) {
deeplyNested.append("/*");
}
deeplyNested.append(" x ");
for (int i = 0; i < 64; i++) {
deeplyNested.append("*/");
}
deeplyNested.append(" ?");

return new Object[][]{
// nested block comments
{"SELECT 1 /* ) /* ) */ ) */, ?", new int[]{28}, false, true},
{"SELECT /* a /* b */ c */ ?", new int[]{25}, false, true},
{"SELECT /* ? /* ? */ ? */ ?", new int[]{25}, false, true},
{"SELECT /* a\n/* b\n*/ c\n*/ ?", new int[]{25}, false, true},
{"SELECT /* /* /* x */ */ */ ?", new int[]{27}, false, true},
{"SELECT 1 /* ; /* ; */ ; */, ?", new int[]{28}, false, true},
{"/* a /* ? */ b */ SELECT ? /* c /* ? */ d */", new int[]{25}, false, true},
{"SELECT ? FROM t WHERE a = /* x /* ? */ y */ ? AND b = 1", new int[]{7, 44}, false, true},
{deeplyNested.toString(), new int[]{deeplyNested.length() - 1}, false, true},
{"INSERT INTO t (a, b) VALUES (1 /* ) /* ) */ ) */, ?)", new int[]{50}, true, false},
{"INSERT INTO t /* c1 /* c2 */ c3 */ (a) VALUES (?)", new int[]{47}, true, false},

// not nested - existing behaviour that must stay unchanged
{"SELECT /* plain comment ? */ ?", new int[]{29}, false, true},
{"SELECT /**/ ?", new int[]{12}, false, true},
{"SELECT 1 /*/ 2 */, ?", new int[]{19}, false, true},
{"SELECT '/* not a comment */' AS a, ?", new int[]{35}, false, true},
{"INSERT INTO t (a, b) VALUES (1 /* plain */, ?)", new int[]{44}, true, false},
};
}

/**
* Reads SQL keywords from the resource file.
* Keywords are listed one per line, comments start with #.
Expand Down
Loading