ClickHouse / ClickHouse/clickhouse-java

jdbc-v2: ANTLR4 parser backends report a source table as the INSERT target, writing rows into the wrong table

Open
#3,129 0 comments 0 reactions 0 assignees View on GitHub
area:jdbc-insert area:sql-parser bug jdbc-v2
Dominant language
Java
Stars
1.6k
Forks
636
Avg merge
2d 16h
Merged PRs (30d)
28

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.

Contributor guide

Open the contributing guide

Research direction

Start in jdbc-v2/src/main/java/com/clickhouse/jdbc/internal/SqlParserFacade.java, especially ParsedPreparedStatementListener.enterInsertStmt and enterTableExprIdentifier, then run the parser-level example from the issue with both ANTLR4 backends. Done means INSERT targets remain dst or db1.dst when source tables, CTEs, or scalar subqueries follow, while SELECT statements and table-function handling retain their current behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
java, sql
Domain
backend, databases
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
78/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.