ClickHouse / ClickHouse/clickhouse-cpp

Client::Insert concatenates the raw table name into SQL — table names needing backquotes cannot be used

Abierto
#560 1 comentario 0 reacciones 0 asignados Ver en GitHub
Lenguaje dominante
C
Estrellas
382
Forks
209
Merge medio
2 d 19 h
PR fusionados (30 d)
14

Descripción

### Description

`Client::Insert(const std::string& table_name, const Block& block)` builds its `INSERT` statement by concatenating the raw table name:

https://github.com/ClickHouse/clickhouse-cpp/blob/master/clickhouse/client.cpp#L500

```cpp
Query query("INSERT INTO " + table_name + " ( " + fields_section.str() + " ) VALUES", query_id);
```

The inconsistency is inside this one function: the **column** names a few lines above are quoted and escaped with `NameToQueryString()` (`clickhouse/client.cpp:456`, backquotes + `` ` `` → ` `` `), but the **table** name is spliced in verbatim.

`clickhouse/client.h:315` documents the parameter as a table name ("Intends for insert block of data into a table \p table_name"), so a caller reasonably passes the plain name. For any legal-but-needs-quoting name (`my-table`, `user events`, a name starting with a digit, …) the generated SQL fails to parse server-side even though the table exists — `INSERT INTO my-table` parses `my` minus `table`.

Callers can work around it by pre-quoting (`client.Insert("\`my-table\`", block)`), and qualified names (`db.table`) are also passed through this parameter today, so blind quoting would be a breaking change — hence this is as much a contract question as a bug: either quote/escape internally with pass-through for already-quoted input (JDBC's `Statement.enquoteIdentifier` contract), or document that this parameter is a SQL fragment and the caller must quote it.

### ClickHouse server version

`26.8.1.2041` (official build), reached over HTTP at `localhost:8123`.

### Reproduction

```cpp
#include
#include

using namespace clickhouse;

int main() {
ClientOptions opts;
opts.SetHost("localhost").SetPort(9000).SetDefaultDatabase("default");
Client client(opts);

client.Execute("DROP TABLE IF EXISTS `my-table`");
client.Execute("CREATE TABLE `my-table` (id Int64) ENGINE = Memory");

Block block;
auto col = std::make_shared();
col->Append(42);
block.AppendColumn("id", col);

try {
client.Insert("my-table", block); // plain table name, as the javadoc-style comment implies
std::cout << "INSERT OK" << std::endl;
} catch (const std::exception& e) {
std::cout << "INSERT FAILED: " << e.what() << std::endl;
}

Client c2(opts);
c2.Insert("`my-table`", block); // pre-quoted workaround: succeeds
return 0;
}
```

Expected: the insert succeeds, since `` `my-table` `` exists and the library already knows how to quote identifiers.

Actual: `Client::Insert` emits

```
INSERT INTO my-table ( `id` ) VALUES
```

and the server rejects it. Sending that exact generated statement against 26.8.1.2041 while the table exists:

```
$ curl -sS 'http://localhost:8123/' --data-binary 'CREATE TABLE IF NOT EXISTS `my-table` (id Int64) ENGINE = Memory'
$ curl -sS 'http://localhost:8123/' --data-binary 'INSERT INTO my-table ( `id` ) VALUES (42)'
Code: 62. DB::Exception: Syntax error: failed at position 15 (-): -table ( `id` ) VALUES (42).
Expected one of: token, Dot, OpeningRoundBracket, FROM INFILE, SETTINGS, VALUES, FORMAT, SELECT, WITH, FROM. (SYNTAX_ERROR)

$ curl -sS 'http://localhost:8123/' --data-binary 'INSERT INTO `my-table` ( `id` ) VALUES (42)' # backquoted: succeeds, no output
```

Note on verification: the C++ program above compiles cleanly against the repo's static libs, but the sandbox used for this investigation does not permit executing built binaries, so the failure was confirmed by replaying the exact statement that `Client::Impl::Insert` constructs (verified by reading the string-building code) against a live server, as shown above. The client-side statement construction is deterministic — the table name is inserted with no quoting on any path.

### Suggested fix

`clickhouse/client.cpp:500` — run the table name through the same identifier-quoting path already used for column names, with an escape hatch for existing callers:

- detect an already-quoted identifier (`` `...` ``) and pass it through unchanged;
- handle the qualified `database.table` form by quoting each part separately, since a single `NameToQueryString("db.table")` would produce the wrong `` `db.table` `` ;
- otherwise apply `NameToQueryString()`.

Whatever is chosen, the contract for `table_name` should be spelled out in `clickhouse/client.h:315-317` (raw identifier vs. SQL fragment) — the current ambiguity is what makes the two behaviours indistinguishable to callers.

### Link

Same root cause as ClickHouse/clickhouse-java#3089 (client-v2 `getTableSchema` / `insert(tableName, …)`). Note this repo has no `DESCRIBE TABLE`-based schema introspection helper, so only the insert path is affected here.

Guía de contribución

No hay ninguna guía de contribución indexada para este repositorio

Línea de trabajo

Lee clickhouse/client.cpp alrededor de las líneas 456 y 500 para comparar el entrecomillado de columnas con la construcción del nombre de tabla de Client::Insert; después, revisa la documentación de los parámetros en clickhouse/client.h:315-317. Verifica el contrato elegido y las sentencias INSERT generadas para nombres simples, entrecomillados con backticks y database.table, incluido un nombre de tabla que requiera entrecomillado.

Escrito por el modelo de indexación a partir del texto del issue.

Evaluación

Stack tecnológico
cpp, sql
Área
api, databases
Tipo de issue
Error
Dificultad
3/5
Tiempo estimado
1-2 días
Estado de actividad
Activo
Claridad
Bastante claro
Aptitud para principiantes
72/100

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.