jdbc-v2: an INSERT VALUES list holding a literal is routed to the beta RowBinary writer, which silently shifts the bound values

[polyglotAI-bot]

Description

With the beta RowBinary writer enabled (beta.row_binary_for_simple_insert=true) and the default JAVACC parser
backend, an INSERT ... VALUES list that is not placeholders only - it holds a literal, a JDBC escape sequence
({fn now()}) or a ClickHouse query parameter ({p1:DateTime}) - is still routed to WriterStatementImpl, the
RowBinary writer.

That writer takes one value per column from the bound parameters, so a values list with fewer placeholders than
columns is written with the values shifted left. The literal is dropped and the trailing column receives no value.
When the trailing column is nullable the statement succeeds and stores wrong data; otherwise it fails with a
misleading error.

ConnectionImpl#prepareStatement selects the writer when the statement is a single-values-group INSERT that is not
an insert-from-select and does not use a function - the comment above that test states the intent is "a values list of
parameter placeholders only". ParsedPreparedStatement.useFunction only reports function calls, so a values list
holding non-placeholder values that are not function calls passes the test.

This is not #3027 (that one is ANTLR4 only, and is about a function call the grammar cannot match; here the JAVACC
backend parses the statement cleanly and still reports no function). INSERT INTO t VALUES (now(), ?) is handled
correctly - a function call is detected and the statement goes to PreparedStatementImpl.

Steps to reproduce

1. Enable beta.row_binary_for_simple_insert=true with the default JAVACC parser backend.
2. Create a table whose last column is nullable.
3. Prepare an INSERT ... VALUES whose first value is a literal and whose remaining values are ?, bind the
   placeholders, and execute.
4. Read the row back.

Error Log or Exception StackTrace

Nullable trailing column - no error, wrong data (see below).

Non-nullable trailing column, INSERT INTO bug_i (a, b, c) VALUES (7, ?, ?) on (a Int32, b Int32, c Int32):

java.sql.SQLException: java.lang.IllegalArgumentException: An attempt to write null into not nullable column 'c'

Type mismatch after the shift, INSERT INTO bug_t (a, b) VALUES ({fn now()}, ?) on (a DateTime, b Int32) with
setObject(1, 42):

java.sql.SQLException: java.lang.IllegalArgumentException: Cannot convert 42 to DateTime

Expected Behaviour

The values list is not placeholders only, so the statement must be given the generic parameter substitution path
(PreparedStatementImpl), as it is when the beta writer is disabled and as it is for VALUES (now(), ?).

Expected result of the reproduction below, and what the standard path produces:

ROW: a=7 b=20 c=30

Actual result on the beta writer path - the literal 7 is dropped, 20 and 30 shift into a and b, and c
becomes null, with no error:

ROW: a=20 b=30 c=null

Code Example

Properties p = new Properties();
p.setProperty("beta.row_binary_for_simple_insert", "true");

try (Connection c = DriverManager.getConnection(url, p)) {
    try (Statement s = c.createStatement()) {
        s.execute("CREATE TABLE bug_n (a Int32, b Int32, c Nullable(Int32)) ENGINE MergeTree ORDER BY tuple()");
    }
    try (PreparedStatement ps = c.prepareStatement("INSERT INTO bug_n (a, b, c) VALUES (7, ?, ?)")) {
        System.out.println(ps.getClass().getSimpleName()); // WriterStatementImpl
        ps.setObject(1, 20);
        ps.setObject(2, 30);
        ps.executeUpdate();                                // succeeds
    }
    try (Statement s = c.createStatement();
         ResultSet rs = s.executeQuery("SELECT a, b, c FROM bug_n")) {
        rs.next();
        // expected 7, 20, 30 - actual 20, 30, null
        System.out.println(rs.getString(1) + " " + rs.getString(2) + " " + rs.getString(3));
    }
}

Affected values lists, all reported by the JAVACC backend as useFunction=false, valueGroups=1, and all routed to
WriterStatementImpl:

useFunction=false groups=1 args=2 | INSERT INTO bug_i (a, b, c) VALUES (7, ?, ?)
useFunction=false groups=1 args=2 | INSERT INTO bug_i (a, b, c) VALUES (?, ?, 7)
useFunction=false groups=1 args=1 | INSERT INTO bug_t (a, b) VALUES ({fn now()}, ?)
useFunction=false groups=1 args=1 | INSERT INTO bug_t (a, b) VALUES ({p1:DateTime}, ?)
useFunction=false groups=1 args=1 | INSERT INTO bug_t (a, b) VALUES ('2020-01-01 00:00:00', ?)
useFunction=true  groups=1 args=1 | INSERT INTO bug_t (a, b) VALUES (now(), ?)          <- correct, not routed

Root cause

jdbc-v2/src/main/java/com/clickhouse/jdbc/internal/SqlParserFacade.java, JavaCCParser#parsePreparedStatement.

Lines 108-114 already scan the values list for anything that is not ?, , or whitespace and set useFunction for
it - a check that covers exactly the non-placeholder values above:

for (int i = startIndex + 1; i < endIndex; i++) {
    char ch = query.charAt(i);
    if (ch != '?' && ch != ',' && !Character.isWhitespace(ch)) {
        stmt.setUseFunction(true);
        break;
    }
}

Line 124 then overwrites that result unconditionally with the token manager's function flag, which is only set when
the grammar matches a function call (ClickHouseSqlParser.jj:834):

stmt.setUseFunction(parsedStmt.isFuncUsed());

so the scan above has no effect and every non-function, non-placeholder value is reported as useFunction=false.

Suggested fix

Do not discard the scan result - combine the two signals rather than overwriting, for example

stmt.setUseFunction(stmt.isUseFunction() || parsedStmt.isFuncUsed());

Two notes for whoever takes this:

• The values-list positions the scan uses are themselves not always offsets into the original SQL for statements
  containing a JDBC escape sequence or a {p:T} parameter - see jdbc-v2: values-list positions from the JavaCC parser are offsets into the rebuilt SQL, but PreparedStatementImpl slices the original SQL #3017 / PR Fix jdbc-v2: discard values list positions that do not address the original SQL #3018. A guard that does not depend on
  those positions, such as requiring the placeholder count to equal the column/schema count before selecting the
  writer, would be robust on its own; the two fixes are complementary.
• VALUES (now(), ?) must keep its current behavior (detected, routed to PreparedStatementImpl), and a values list
  of placeholders only must keep being routed to the writer - that is the feature.

Configuration

Client Configuration

Properties p = new Properties();
p.setProperty("beta.row_binary_for_simple_insert", "true");
// jdbc_sql_parser left at its default (JAVACC)

Environment

• Cloud
• Client version: main at ab256198a (0.11.0-rc1)
• Language version: OpenJDK 17
• OS: Ubuntu 24.04 (container)

ClickHouse Server

• ClickHouse Server version: 26.7.3.19
• ClickHouse Server non-default settings, if any: none relevant
• CREATE TABLE statements for tables involved:

CREATE TABLE bug_n (a Int32, b Int32, c Nullable(Int32)) ENGINE MergeTree ORDER BY tuple();
CREATE TABLE bug_i (a Int32, b Int32, c Int32) ENGINE MergeTree ORDER BY tuple();
CREATE TABLE bug_t (a DateTime, b Int32) ENGINE MergeTree ORDER BY b;

• Sample data: none needed, the reproduction inserts its own.

---

Found by automated analysis of jdbc-v2 while working on PR #3018, and verified end to end against a live ClickHouse
server rather than by inspection.