jdbc-v2: an INSERT VALUES list holding a literal is routed to the beta RowBinary writer, which silently shifts the bound values
Los mantenedores suelen responder en 1 día
Nadie ha tomado este issue todavía.
Evaluación
- Dificultad
- 3/5
- Tiempo estimado
- 1-2 días
- Aptitud para principiantes
- 76/100
Línea de trabajo
Comienza en ConnectionImpl#prepareStatement y jdbc-v2/src/main/java/com/clickhouse/jdbc/internal/SqlParserFacade.java, especialmente en JavaCCParser#parsePreparedStatement y en el manejo de useFunction descrito en el issue. Reproduce los casos indicados de literales, escapes JDBC y parámetros de consulta con beta writer habilitado, y luego verifica que usen la ruta genérica, mientras que los valores que solo contienen marcadores de posición sigan usando WriterStatementImpl y conserven los datos de fila esperados.
Escrito por el modelo de indexación a partir del texto del issue.
Descripción
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
- Enable
beta.row_binary_for_simple_insert=truewith the defaultJAVACCparser backend. - Create a table whose last column is nullable.
- Prepare an
INSERT ... VALUESwhose first value is a literal and whose remaining values are?, bind the
placeholders, and execute. - 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 #3017 / PR #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 toPreparedStatementImpl), 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:
mainatab256198a(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 TABLEstatements 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.
- Lenguaje dominante
- Java
- Estrellas
- 1.6k
- Forks
- 638
- Merge medio
- 2 d 16 h
- PR fusionados (30 d)
- 42
Preparar el entorno
- Sin Dockerfile ni archivo de Docker Compose
- Tiene una plantilla de pull request
- Leer la guía de contribución
Primeros pasos
- Lee el issue completo y luego la guía de contribución del proyecto.
- Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
- Haz un fork del repositorio y trabaja en una rama.
- Abre un pull request que haga referencia al número del issue.
Más de ClickHouse/clickhouse-java
-
[examples] Remove old Spring examplePosiblemente ocupada @polyglotAI-bot la tomó hace 7 días. Abierto
Dificultad 1/5 Menos de una hora Aptitud para principiantes 85/100
ClickHouse/clickhouse-java#3111 · 1 comentario · 1 asignado ·
Los mantenedores suelen responder en 1 día
-
bug client-api-v2 test
Dificultad 2/5 1-3 horas Aptitud para principiantes 92/100
ClickHouse/clickhouse-java#3076 ·
Los mantenedores suelen responder en 1 día
-
area:sql-parser bug client-v1
Dificultad 1/5 1-3 horas Aptitud para principiantes 92/100
ClickHouse/clickhouse-java#3066 ·
Los mantenedores suelen responder en 1 día
-
area:general bug client-api-v2 jdbc jdbc-v2
Dificultad 2/5 1-3 horas Aptitud para principiantes 88/100
ClickHouse/clickhouse-java#3063 ·
Los mantenedores suelen responder en 1 día
-
bug client-api-v2 jdbc-v2
Dificultad 1/5 1-3 horas Aptitud para principiantes 78/100
ClickHouse/clickhouse-java#2957 · 1 comentario ·
Los mantenedores suelen responder en 1 día
Todos los issues de ClickHouse/clickhouse-java
Issues similares
-
bug
Dificultad 2/5 1-3 horas Aptitud para principiantes 78/100
liquid-java/liquidjava#321 ·
Los mantenedores suelen responder en 2 días
-
Dificultad 1/5 Menos de una hora Aptitud para principiantes 88/100
Los mantenedores suelen responder en 1 día
-
rename config parameterAbierto
Dificultad 2/5 1-3 horas Aptitud para principiantes 82/100
OpenConext/OpenConext-access#1015 ·
Los mantenedores suelen responder en 1 día
-
68000: FDBcc branch target is 2 bytes too low (uses inst_start+2 instead of the displacement word)Abierto
Dificultad 2/5 1-3 horas Aptitud para principiantes 82/100
NationalSecurityAgency/ghidra#9702 ·
Los mantenedores suelen responder en 1 día
-
Dificultad 2/5 1-3 horas Aptitud para principiantes 85/100
apache/rocketmq-dashboard#5358 ·
Los mantenedores suelen responder en 3 días