Description
While cross-checking a clickhouse-connect report about INSERT statement misparsing in the bulk-insert fast path (ClickHouse/clickhouse-connect#932), I found that jdbc-v2 handles most of the reported forms correctly, but not an INSERT whose target is a table function.
SqlParserFacade treats the table-function name as if it were a table name:
| statement |
JAVACC parser |
ANTLR4 parser |
INSERT INTO FUNCTION null('id UInt32') VALUES (?) |
table=unknown |
table=null (the function name) |
INSERT INTO TABLE FUNCTION null('id UInt32') VALUES (?) |
table=FUNCTION, valuesGroups=0 |
table=null |
With the default settings this is harmless, because PreparedStatementImpl batching is purely textual (prefix of the original SQL + the rendered value lists) and never uses the parsed table name.
But when beta.row_binary_for_simple_insert=true is set, ConnectionImpl.prepareStatement (jdbc-v2/src/main/java/com/clickhouse/jdbc/ConnectionImpl.java:439-457) sees isInsert() && assignValuesGroups == 1 && !isUseFunction() and calls client.getTableSchema(parsedStatement.getTable(), schema) with that bogus name. A statement the server accepts then fails with:
Code: 60. DB::Exception: Table default.`null` does not exist. (UNKNOWN_TABLE) # ANTLR4
Code: 60. DB::Exception: Table default.unknown does not exist. (UNKNOWN_TABLE) # JAVACC
There is no plain table to write RowBinary into here, so the statement should stay on the regular SQL path (as it already does with the beta flag off).
Related note (not user-visible today): with the JavaCC parser, INSERT INTO TABLE FUNCTION t(...) parses FUNCTION as the table name — the TABLE branch in jdbc-v2/src/main/javacc/ClickHouseSqlParser.jj:588-604 does not exclude a following FUNCTION token, so <TABLE> is consumed and tableIdentifier() swallows FUNCTION. The subsequent parse error is swallowed by the try/catch in dataClause(), which is why valuesGroups ends up 0 and the statement accidentally avoids the beta path.
The other forms from the upstream report are fine in jdbc-v2 (verified, see output below): the optional TABLE keyword, multi-line INSERTs, and double-quoted column names all parse correctly with both JAVACC and ANTLR4, because the driver uses real grammars rather than a regex, and ClickHouseSqlUtils.unescape strips \" as well as backticks.
ClickHouse server version
26.7.2.59 (official build), reached over HTTP at localhost:8123.
Reproduction
TestNG test (jdbc-v2), run against a local server:
package com.clickhouse.jdbc;
import org.testng.annotations.Test;
import java.sql.Connection;
import java.sql.PreparedStatement;
import java.sql.Statement;
import java.util.Arrays;
import java.util.Properties;
public class InsertIntoTableFunctionTest {
private Connection conn(String parser, boolean betaWriter) throws Exception {
Properties p = new Properties();
p.setProperty("user", "default");
p.setProperty("password", "");
p.setProperty("jdbc_sql_parser", parser);
p.setProperty("beta.row_binary_for_simple_insert", String.valueOf(betaWriter));
return new Driver().connect("jdbc:clickhouse://localhost:8123", p);
}
@Test
public void insertIntoTableFunction() throws Exception {
for (String parser : new String[]{"JAVACC", "ANTLR4"}) {
for (boolean beta : new boolean[]{false, true}) {
for (String sql : new String[]{
"INSERT INTO FUNCTION null('id UInt32') VALUES (?)",
"INSERT INTO TABLE FUNCTION null('id UInt32') VALUES (?)"}) {
try (Connection c = conn(parser, beta);
PreparedStatement ps = c.prepareStatement(sql)) {
ps.setInt(1, 13); ps.addBatch();
ps.setInt(1, 14); ps.addBatch();
System.out.println(parser + " beta=" + beta + " " + sql
+ " -> OK " + Arrays.toString(ps.executeBatch()));
} catch (Exception e) {
System.out.println(parser + " beta=" + beta + " " + sql
+ " -> FAILED " + e.getMessage());
}
}
}
}
}
}
Actual output:
JAVACC beta=false INSERT INTO FUNCTION null('id UInt32') VALUES (?) -> OK [1, 1]
JAVACC beta=false INSERT INTO TABLE FUNCTION null('id UInt32') VALUES (?) -> OK [1, 1]
ANTLR4 beta=false INSERT INTO FUNCTION null('id UInt32') VALUES (?) -> OK [1, 1]
ANTLR4 beta=false INSERT INTO TABLE FUNCTION null('id UInt32') VALUES (?) -> OK [1, 1]
JAVACC beta=true INSERT INTO FUNCTION null('id UInt32') VALUES (?) -> FAILED Code: 60. DB::Exception: Table default.unknown does not exist. (UNKNOWN_TABLE)
JAVACC beta=true INSERT INTO TABLE FUNCTION null('id UInt32') VALUES (?) -> OK [1, 1]
ANTLR4 beta=true INSERT INTO FUNCTION null('id UInt32') VALUES (?) -> FAILED Code: 60. DB::Exception: Table default.`null` does not exist. (UNKNOWN_TABLE)
ANTLR4 beta=true INSERT INTO TABLE FUNCTION null('id UInt32') VALUES (?) -> FAILED Code: 60. DB::Exception: Table default.`null` does not exist. (UNKNOWN_TABLE)
Expected: all eight combinations succeed (a table-function target falls back to the regular SQL path).
For completeness, the forms that already work (both parsers, beta on and off — all insert 2 rows into scratch_tbl with columns id, name):
INSERT INTO TABLE scratch_tbl (id, name) VALUES (?, ?)
INSERT INTO scratch_tbl\n (id, name)\nVALUES (?, ?)
INSERT INTO scratch_tbl ("id", "name") VALUES (?, ?)
Suggested fix
- Record on
ParsedPreparedStatement whether the INSERT target is a table function (both parsers already distinguish the branch: insertStmt in jdbc-v2/src/main/antlr4/com/clickhouse/jdbc/internal/parser/antlr4/ClickHouseParser.g4:406-408 and jdbc-v2/src/main/javacc/ClickHouseSqlParser.jj:588-604), and skip the WriterStatementImpl branch in ConnectionImpl.prepareStatement when it is set (or simply when getTable() is null/unset).
- In the ANTLR4 listener,
enterInsertStmt/enterTableExprIdentifier (jdbc-v2/src/main/java/com/clickhouse/jdbc/internal/SqlParserFacade.java) should not set the table from a FUNCTION tableFunctionExpr target.
- In the JavaCC grammar, add
FUNCTION to the exclusion list of the <TABLE> lookahead so INSERT INTO TABLE FUNCTION t(...) is routed to the functionExpr() branch instead of parsing FUNCTION as the table name.
Link
Upstream report this was cross-checked against: ClickHouse/clickhouse-connect#932
Description
While cross-checking a clickhouse-connect report about INSERT statement misparsing in the bulk-insert fast path (ClickHouse/clickhouse-connect#932), I found that jdbc-v2 handles most of the reported forms correctly, but not an INSERT whose target is a table function.
SqlParserFacadetreats the table-function name as if it were a table name:INSERT INTO FUNCTION null('id UInt32') VALUES (?)table=unknowntable=null(the function name)INSERT INTO TABLE FUNCTION null('id UInt32') VALUES (?)table=FUNCTION,valuesGroups=0table=nullWith the default settings this is harmless, because
PreparedStatementImplbatching is purely textual (prefix of the original SQL + the rendered value lists) and never uses the parsed table name.But when
beta.row_binary_for_simple_insert=trueis set,ConnectionImpl.prepareStatement(jdbc-v2/src/main/java/com/clickhouse/jdbc/ConnectionImpl.java:439-457) seesisInsert() && assignValuesGroups == 1 && !isUseFunction()and callsclient.getTableSchema(parsedStatement.getTable(), schema)with that bogus name. A statement the server accepts then fails with:There is no plain table to write RowBinary into here, so the statement should stay on the regular SQL path (as it already does with the beta flag off).
Related note (not user-visible today): with the JavaCC parser,
INSERT INTO TABLE FUNCTION t(...)parsesFUNCTIONas the table name — theTABLEbranch injdbc-v2/src/main/javacc/ClickHouseSqlParser.jj:588-604does not exclude a followingFUNCTIONtoken, so<TABLE>is consumed andtableIdentifier()swallowsFUNCTION. The subsequent parse error is swallowed by thetry/catchindataClause(), which is whyvaluesGroupsends up 0 and the statement accidentally avoids the beta path.The other forms from the upstream report are fine in jdbc-v2 (verified, see output below): the optional
TABLEkeyword, multi-line INSERTs, and double-quoted column names all parse correctly with both JAVACC and ANTLR4, because the driver uses real grammars rather than a regex, andClickHouseSqlUtils.unescapestrips\"as well as backticks.ClickHouse server version
26.7.2.59 (official build), reached over HTTP at localhost:8123.
Reproduction
TestNG test (jdbc-v2), run against a local server:
Actual output:
Expected: all eight combinations succeed (a table-function target falls back to the regular SQL path).
For completeness, the forms that already work (both parsers, beta on and off — all insert 2 rows into
scratch_tblwith columnsid,name):Suggested fix
ParsedPreparedStatementwhether the INSERT target is a table function (both parsers already distinguish the branch:insertStmtinjdbc-v2/src/main/antlr4/com/clickhouse/jdbc/internal/parser/antlr4/ClickHouseParser.g4:406-408andjdbc-v2/src/main/javacc/ClickHouseSqlParser.jj:588-604), and skip theWriterStatementImplbranch inConnectionImpl.prepareStatementwhen it is set (or simply whengetTable()is null/unset).enterInsertStmt/enterTableExprIdentifier(jdbc-v2/src/main/java/com/clickhouse/jdbc/internal/SqlParserFacade.java) should not set the table from aFUNCTION tableFunctionExprtarget.FUNCTIONto the exclusion list of the<TABLE>lookahead soINSERT INTO TABLE FUNCTION t(...)is routed to thefunctionExpr()branch instead of parsingFUNCTIONas the table name.Link
Upstream report this was cross-checked against: ClickHouse/clickhouse-connect#932