ClickHouse / ClickHouse/clickhouse-java
jdbc-v2: ANTLR4 parser backends report a source table as the INSERT target, writing rows into the wrong table
- Langage dominant
- Java
- Étoiles
- 1.6k
- Forks
- 636
- Merge moyen
- 2 j 23 h
- PR mergées (30 j)
- 29
Description
## Description
With either ANTLR4 parser backend (`jdbc_sql_parser=ANTLR4` or `ANTLR4_PARAMS_PARSER`),
`ParsedPreparedStatement.getTable()` of an `INSERT` returns the **last table identifier that appears anywhere in the
statement** instead of the insert target. Any table read by the statement - a CTE name, a `FROM` table, a table in a
scalar subquery inside the `VALUES` list - replaces the target. Parsing reports no errors, so the wrong target is
silent. The default `JAVACC` backend is correct in every case below.
Observed on `main` (91ec4d3) with server 26.8.2.7:
| SQL | JAVACC | ANTLR4 / ANTLR4_PARAMS_PARSER |
|---|---|---|
| `INSERT INTO dst WITH r AS (SELECT 1 AS n) SELECT n FROM r` | `dst` | `r` (CTE name) |
| `INSERT INTO dst SELECT * FROM src` | `dst` | `src` |
| `INSERT INTO db1.dst SELECT * FROM db2.src` | `db1.dst` | `db2.src` (database also overwritten) |
| `INSERT INTO dst WITH r AS (SELECT 1 AS n) SELECT n FROM r JOIN other USING (n)` | `dst` | `other` |
| `INSERT INTO dst VALUES (?, (SELECT max(x) FROM src))` | `dst` | `src` |
| `INSERT INTO dst (a) VALUES (?)` | `dst` | `dst` (correct - no source table) |
| `INSERT INTO dst SELECT 1` | `dst` | `dst` (correct - no source table) |
The last row of the table is the harmful one. `ConnectionImpl#prepareStatement` uses `getTable()` to resolve the
schema for the beta RowBinary writer, and its guard (`!isInsertWithSelect() && getAssignValuesGroups() == 1 &&
!isUseFunction()`) does not exclude a `VALUES` list holding a scalar subquery. So with
`beta.row_binary_for_simple_insert=true` the writer is built against the **source** table and the row is inserted
into it. `executeUpdate()` returns normally and the target table stays empty - **silent data loss plus a write into a
table the statement only reads.**
Both non-default options are needed for the wrong write (`jdbc_sql_parser=ANTLR4*` plus the beta writer). The wrong
table name itself is returned by the parser regardless of the writer setting.
This is not #3083 (that one is the default `JAVACC` backend and is about values being shifted within the correct
target table) and not #3015 / #3027 (table functions and unparsable value expressions).
### Steps to reproduce
1. `CREATE TABLE src (x Int32) ENGINE=Memory; CREATE TABLE dst (a Int32, b Int32) ENGINE=Memory;`
`INSERT INTO src VALUES (7),(9);`
2. Open a connection with `jdbc_sql_parser=ANTLR4` and `beta.row_binary_for_simple_insert=true`.
3. `prepareStatement("INSERT INTO dst VALUES (?, (SELECT max(x) FROM src))")`, `setInt(1, 42)`, `executeUpdate()`.
4. `SELECT * FROM dst` and `SELECT * FROM src`.
### Error Log or Exception StackTrace
No error. `executeUpdate()` reports success.
```
### parser=JAVACC
stmt class = WriterStatementImpl
EXCEPTION: java.sql.SQLException: java.lang.IllegalArgumentException: An attempt to write null into not nullable column 'b'
### parser=ANTLR4
stmt class = WriterStatementImpl
executeUpdate OK
### parser=ANTLR4_PARAMS_PARSER
stmt class = WriterStatementImpl
executeUpdate OK
```
Table contents after the three runs - `dst` is empty, `src` holds the two rows that the ANTLR4 runs wrote:
```
-- dst:
-- src:
7
9
42
42
```
(The `JAVACC` line is the separate, already reported #3083 behaviour: the writer is chosen for a values list that is
not placeholders only. It at least targets the correct table.)
### Expected Behaviour
`getTable()` of an `INSERT` is the insert target, for every backend - `dst` in all rows of the table above, and
`db1.dst` for the qualified case. The server accepts all of these statements and writes into the target only, e.g.
```
$ curl --data-binary "INSERT INTO dst VALUES (1, (SELECT max(x) FROM src))" http://server:8123/
$ curl --data-binary "SELECT * FROM dst FORMAT TSV" http://server:8123/
1 9
$ curl --data-binary "INSERT INTO dst WITH r AS (SELECT 1 AS n) SELECT n, n FROM r" http://server:8123/
$ curl --data-binary "SELECT * FROM dst ORDER BY a FORMAT TSV" http://server:8123/
1 9
1 1
```
### Root cause
`jdbc-v2/src/main/java/com/clickhouse/jdbc/internal/SqlParserFacade.java`
* `ParsedPreparedStatementListener.enterTableExprIdentifier` (line 441) calls
`extractAndSetDatabaseAndTable` for **every** `tableExprIdentifier` in the tree - that rule matches the tables a
query reads, not the insert target.
* `enterInsertStmt` (line 448) sets the target correctly, but `insertStmt` is entered before its nested
`tableExprIdentifier` nodes, so each source table seen later overwrites the target through the shared
`parsedStatement.setTable` / `setDatabase` (line 488).
* `INSERT INTO dst SELECT 1` and a plain `VALUES` insert keep the correct name only because no `tableExprIdentifier`
follows.
The JavaCC backend keeps the target because it assigns the table from the insert production only.
### Suggested fix
Make the insert target win over source tables in the ANTLR4 listener - for example have
`enterTableExprIdentifier` skip assignment once an insert target has been recorded (or record source tables
separately from `table`/`database`), so `enterInsertStmt` remains authoritative for an `INSERT`.
Contrast cases that must keep their current behaviour:
* `SELECT * FROM src` -> `src`, and `WITH r AS (SELECT 1 AS n) SELECT n FROM r` -> `r`. For a statement that is not an
insert, `tableExprIdentifier` is the only source of the name and both backends agree today.
* `INSERT INTO [TABLE] FUNCTION f(...)` must stay `useFunction=true` and off the writer path (#3015 / #3016).
* `INSERT INTO db1.dst SELECT ...` must report database `db1`, not `db2`.
Separately, `ConnectionImpl#prepareStatement`'s writer guard could reject a `VALUES` list that contains a subquery;
that part is the same missing-guard family as #3083.
### Code Example
```java
Properties p = new Properties();
p.setProperty("jdbc_sql_parser", "ANTLR4"); // or ANTLR4_PARAMS_PARSER
p.setProperty("beta.row_binary_for_simple_insert", "true");
try (Connection c = DriverManager.getConnection(url, p);
PreparedStatement ps = c.prepareStatement("INSERT INTO dst VALUES (?, (SELECT max(x) FROM src))")) {
ps.setInt(1, 42);
ps.executeUpdate(); // succeeds; 42 lands in src, dst stays empty
}
```
Parser level, no server needed:
```java
SqlParserFacade parser = SqlParserFacade.getParser("ANTLR4",
new JdbcConfiguration("jdbc:ch:http://localhost:8123", new Properties()));
ParsedPreparedStatement s = parser.parsePreparedStatement(
"INSERT INTO dst WITH r AS (SELECT 1 AS n) SELECT n FROM r");
assert s.isInsert();
assert !s.isHasErrors();
assert "dst".equals(s.getTable()); // fails: returns "r"
```
### Configuration
#### Client Configuration
```java
jdbc_sql_parser = ANTLR4 // or ANTLR4_PARAMS_PARSER; JAVACC (default) is unaffected
beta.row_binary_for_simple_insert = true // only needed for the wrong write, not for the wrong name
```
#### Environment
* [ ] Cloud
* Client version: 0.11.0-rc1 (`main`, 91ec4d3)
* Language version: OpenJDK 17.0.18
* OS: Ubuntu 24.04 (container)
#### ClickHouse Server
* ClickHouse Server version: 26.8.2.7
* ClickHouse Server non-default settings, if any: none
* `CREATE TABLE` statements for tables involved:
```sql
CREATE TABLE src (x Int32) ENGINE = Memory;
CREATE TABLE dst (a Int32, b Int32) ENGINE = Memory;
```
* Sample data: `INSERT INTO src VALUES (7),(9);`
---
Found by automated analysis of this client while working on #3122 / #3128 (`WITH RECURSIVE` grammar gap), then
verified end to end against a live 26.8.2.7 server rather than by code inspection.
Guide de contribution
Ouvrir le guide de contribution
Piste de recherche
Commencez dans jdbc-v2/src/main/java/com/clickhouse/jdbc/internal/SqlParserFacade.java, en particulier dans ParsedPreparedStatementListener.enterInsertStmt et enterTableExprIdentifier, puis exécutez l’exemple au niveau du parser fourni dans l’issue avec les deux backends ANTLR4. C’est terminé lorsque les cibles d’INSERT restent dst ou db1.dst lorsque des tables sources, des CTE ou des sous-requêtes scalaires suivent, tandis que les instructions SELECT et la gestion des fonctions de table conservent leur comportement actuel.
Rédigé par le modèle d'indexation à partir du texte de l'issue.
Évaluation
- Stack technique
- java, sql
- Domaine
- backend, databases
- Type d'issue
- Bug
- Difficulté
- 3/5
- Temps estimé
- 1-2 jours
- Activité
- Active
- Clarté
- Clairement spécifiée
- Accessibilité débutants
- 78/100