Skip to content

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

Description

@claude

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

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

#include <clickhouse/client.h>
#include <iostream>

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<ColumnInt64>();
    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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions