ClickHouse / ClickHouse/clickhouse-cpp
Client::Insert concatenates the raw table name into SQL — table names needing backquotes cannot be used
- Dominant language
- C
- Stars
- 382
- Forks
- 209
- Avg merge
- 3h 58m
- Merged PRs (30d)
- 12
Description
### 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.
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.