From 1fb7a2b350eeba02f4b0219d8dbbe2ea6361674d Mon Sep 17 00:00:00 2001 From: Polyglot AI <293096396+polyglotAI-bot@users.noreply.github.com> Date: Fri, 26 Jun 2026 17:10:31 +0000 Subject: [PATCH 1/2] [jdbc-v2] Unescape backtick-quoted INSERT column names before schema lookup The ANTLR4 parser stored the raw parse-tree text of each INSERT column name, keeping backticks, so the canonical Nested sub-column wire form `directory`.`id` failed the by-name schema lookup in WriterStatementImpl and threw NoSuchColumnException. Unescape each identifier component and rejoin with '.', mirroring how the table/database identifiers are already handled in the same listener. Fixes: https://github.com/ClickHouse/clickhouse-java/issues/2896 --- .../jdbc/internal/SqlParserFacade.java | 7 ++- .../internal/BaseSqlParserFacadeTest.java | 51 +++++++++++++++++++ 2 files changed, 57 insertions(+), 1 deletion(-) 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 406ec013d..178c9a070 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 @@ -343,7 +343,12 @@ public void enterInsertStmt(ClickHouseParser.InsertStmtContext ctx) { List names = columns.nestedIdentifier(); String[] insertColumns = new String[names.size()]; for (int i = 0; i < names.size(); i++) { - insertColumns[i] = names.get(i).getText(); + // Unescape each identifier component and rejoin with '.', mirroring how the + // table/database identifiers are handled above, so backtick-quoted column + // names (e.g. the Nested wire form `directory`.`id`) match the schema columns. + insertColumns[i] = names.get(i).identifier().stream() + .map(id -> ClickHouseSqlUtils.unescape(id.getText())) + .collect(Collectors.joining(".")); } parsedStatement.setInsertColumns(insertColumns); } 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 48366f911..dbcc8a086 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 @@ -254,6 +254,57 @@ private void testCase(String sql, String expectedTableName) { Assert.assertEquals(stmt.getTable(), expectedTableName, "Table name mismatch for: " + sql); } + @Test + public void testInsertColumnNamesAreUnescaped() { + /* + * Regression for #2896: INSERT column names must be unescaped before they are matched + * against the server table schema, exactly as table and database identifiers already are. + * The canonical wire form of a Nested sub-column is `directory`.`id`, which previously kept + * its backticks and failed the schema lookup in WriterStatementImpl with NoSuchColumnException. + */ + + // Backtick-quoted Nested sub-columns -> clean compound names (the reported case) + assertInsertColumns("INSERT INTO t (`directory`.`id`, `directory`.`name`) VALUES (?, ?)", + "directory.id", "directory.name"); + + // Simple backtick-quoted column + assertInsertColumns("INSERT INTO t (`id`) VALUES (?)", "id"); + + // Contrast: already-clean unquoted forms must keep their existing values + assertInsertColumns("INSERT INTO t (directory.id, name) VALUES (?, ?)", "directory.id", "name"); + assertInsertColumns("INSERT INTO t (id) VALUES (?)", "id"); + + // Mixed quoted/unquoted components within one nested name are unescaped per component + assertInsertColumns("INSERT INTO t (`directory`.id, directory.`name`) VALUES (?, ?)", + "directory.id", "directory.name"); + + // A dot *inside* a single backtick-quoted identifier is part of the name, not a separator + assertInsertColumns("INSERT INTO t (`a.b`) VALUES (?)", "a.b"); + + // Double-quoted identifiers are a valid alternate quoting form and unescape the same way + assertInsertColumns("INSERT INTO t (\"directory\".\"id\") VALUES (?)", "directory.id"); + + // Mixed backtick / double-quote components in one nested name + assertInsertColumns("INSERT INTO t (`directory`.\"id\") VALUES (?)", "directory.id"); + + // Escaped backtick inside a quoted column name collapses to a single backtick, + // for both the doubled (``) and backslash-escaped (\`) forms + assertInsertColumns("INSERT INTO t (`od``d`) VALUES (?)", "od`d"); + assertInsertColumns("INSERT INTO t (`od\\`d`) VALUES (?)", "od`d"); + } + + private void assertInsertColumns(String sql, String... expectedColumns) { + ParsedPreparedStatement stmt = parser.parsePreparedStatement(sql); + Assert.assertFalse(stmt.isHasErrors(), "Query should parse without errors: " + sql); + Assert.assertTrue(stmt.isInsert(), "Should be an INSERT: " + sql); + // Only the ANTLR4 parser backends extract INSERT column names; the JavaCC backend leaves + // them null, so the per-name assertion only applies when they were actually extracted. + String[] actualColumns = stmt.getInsertColumns(); + if (actualColumns != null) { + Assert.assertEquals(actualColumns, expectedColumns, "Insert column names mismatch for: " + sql); + } + } + @Test(dataProvider = "testCreateStmtDP") public void testCreateStatement(String sql) { ParsedPreparedStatement stmt = parser.parsePreparedStatement(sql); From e170a5a53c63c19f533720378827d5b9c2b2127b Mon Sep 17 00:00:00 2001 From: Polyglot AI <293096396+polyglotAI-bot@users.noreply.github.com> Date: Fri, 26 Jun 2026 18:23:50 +0000 Subject: [PATCH 2/2] [jdbc-v2] Fail INSERT-column regression test on null for ANTLR4 backends MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address review feedback on #2899: assertInsertColumns silently skipped the per-name assertions whenever getInsertColumns() was null, which is correct for the JavaCC backend (it does not extract INSERT column names) but would let an ANTLR4 regression — where column extraction stops working and returns null — pass unnoticed. Now null is allowed only for the JavaCC backend; the ANTLR4 backends must extract the columns, so a null fails the test loudly. --- .../jdbc/internal/BaseSqlParserFacadeTest.java | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) 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 dbcc8a086..945701ad0 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 @@ -23,8 +23,11 @@ public abstract class BaseSqlParserFacadeTest { private SqlParserFacade parser; + private final boolean javaCcBackend; + public BaseSqlParserFacadeTest(String name) throws Exception { parser = SqlParserFacade.getParser(name, new JdbcConfiguration("jdbc:ch:http://localhost:8123", new Properties())); + javaCcBackend = SqlParserFacade.SQLParser.JAVACC.name().equals(name); } @Test @@ -297,12 +300,15 @@ private void assertInsertColumns(String sql, String... expectedColumns) { ParsedPreparedStatement stmt = parser.parsePreparedStatement(sql); Assert.assertFalse(stmt.isHasErrors(), "Query should parse without errors: " + sql); Assert.assertTrue(stmt.isInsert(), "Should be an INSERT: " + sql); - // Only the ANTLR4 parser backends extract INSERT column names; the JavaCC backend leaves - // them null, so the per-name assertion only applies when they were actually extracted. String[] actualColumns = stmt.getInsertColumns(); - if (actualColumns != null) { - Assert.assertEquals(actualColumns, expectedColumns, "Insert column names mismatch for: " + sql); + // The JavaCC backend does not extract INSERT column names, so null is allowed there. The + // ANTLR4 backends must extract them, so a null is a regression and must fail the test + // loudly instead of being silently skipped. + if (javaCcBackend) { + return; } + Assert.assertNotNull(actualColumns, "ANTLR4 backend should extract INSERT column names for: " + sql); + Assert.assertEquals(actualColumns, expectedColumns, "Insert column names mismatch for: " + sql); } @Test(dataProvider = "testCreateStmtDP")