ClickHouse / ClickHouse/clickhouse-cpp
Allow optional QuerySettings on Client::Insert() for native-format inserts
- Dominant language
- C
- Stars
- 382
- Forks
- 209
- Avg merge
- 3h 58m
- Merged PRs (30d)
- 12
Description
## Summary
`Client::Insert()` sends data in native block format, but it does not accept per-query settings. `Query::SetSetting()` already exists and is serialized on the native protocol for `Execute()` / `Select()`, but `Insert(table, block)` builds its own `Query` internally and never attaches settings.
This makes it impossible to pass server settings that must apply to a **native-format insert**, for example:
- `insert_deduplication_token`
- `insert_deduplicate`
- `async_insert` / `wait_for_async_insert`
- `max_partitions_per_insert_block`
- `max_insert_block_size`
- other insert-time session settings
The README currently documents this as an unsupported case for async inserts. The same gap affects any setting that cannot be applied only at user/profile level.
## Problem
ClickHouse Replicated\*MergeTree (and MergeTree with `non_replicated_deduplication_window`) deduplicates inserts by a `block_id`. By default that id is a **hash of the inserted block data**.
That is correct for retries of the *same* logical insert, but it is wrong when independent inserts happen to carry identical column values. Typical cases:
- rollups / aggregations (several source intervals map to the same bucket keys and values)
- sparse or zero-filled metrics
- intentionally inserting the same payload as a new fact, not as a retry
The server then drops the later insert. This is recorded as error **389 `INSERT_WAS_DEDUPLICATED`** in `system.part_log`. The client insert usually still **succeeds**, so the application silently loses data.
ClickHouse already provides `insert_deduplication_token` for this: if the client sets a token, the server uses that token instead of the data hash.
- same token → retry is deduplicated
- different token → insert is accepted even if the payload matches a previous block
There is currently no way to send that setting (or any other) through `Client::Insert()`.
`query_id` is **not** a substitute. `Insert(table, query_id, block)` does not change deduplication.
## Current API
```cpp
void Insert(const std::string& table_name, const Block& block);
void Insert(const std::string& table_name, const std::string& query_id, const Block& block);
Block BeginInsert(const std::string& query);
Block BeginInsert(const std::string& query, const std::string& query_id);
```
`Query` already supports:
```cpp
Query& SetSetting(const std::string& key, const QuerySettingsField& value);
Query& SetQuerySettings(QuerySettings query_settings);
```
`SendQuery(const Query&)` already serializes `query.GetQuerySettings()` when the server revision supports string settings.
`Impl::Insert()` builds the query itself:
```cpp
Query query("INSERT INTO " + table_name + " ( " + fields + " ) VALUES", query_id);
SendQuery(query); // settings always empty
```
Workarounds today:
1. Put `SETTINGS ...` into SQL and use `Execute()` with **text** values — loses native block insert.
2. Put `SETTINGS ...` into the SQL string passed to `BeginInsert()` — works only if the server parses it from query text; settings on the `Query` object are still dropped because `BeginInsert` currently does `SendQuery(query.GetText())`.
3. Set the option in `users.xml` / `ALTER USER` — not usable for per-insert values such as `insert_deduplication_token`.
## Proposed API
Keep existing overloads unchanged. Add optional settings (and, if useful, a `Query`-based overload).
```cpp
/// Insert a block. Existing overloads keep current behavior (empty settings).
void Insert(const std::string& table_name, const Block& block);
void Insert(const std::string& table_name, const std::string& query_id, const Block& block);
/// Insert a block with per-query settings (native protocol Query packet).
void Insert(const std::string& table_name, const Block& block,
const QuerySettings& settings);
void Insert(const std::string& table_name, const std::string& query_id, const Block& block,
const QuerySettings& settings);
```
Optional, for consistency with `Execute(const Query&)`:
```cpp
Block BeginInsert(const Query& query);
```
That would let callers do:
```cpp
Query q("INSERT INTO db.table (c1, c2) VALUES");
q.SetSetting("insert_deduplication_token", {"my-token"});
auto block = client.BeginInsert(q);
```
## Suggested implementation
### `Insert()`
In `Client::Impl::Insert`, attach settings to the `Query` **before** `SendQuery(query)`:
```cpp
Query query("INSERT INTO " + table_name + " ( " + fields_section.str() + " ) VALUES", query_id);
query.SetQuerySettings(settings);
SendQuery(query);
```
Thread `settings` through the public overloads. Default / existing overloads pass empty `QuerySettings{}`.
Do not require callers to mark settings `IMPORTANT`. Unknown settings should follow normal ClickHouse behavior (ignored unless `IMPORTANT` is set).
### `BeginInsert()` (related bug)
`Impl::BeginInsert(Query query)` currently calls `SendQuery(query.GetText())`, which constructs a new `Query` from SQL only and **drops** settings, query id extras, tracing context, and params.
Change it to:
```cpp
SendQuery(query); // not SendQuery(query.GetText())
```
Then expose `BeginInsert(const Query&)` publicly.
### Compatibility
- Existing `Insert(table, block)` / `Insert(table, query_id, block)` behavior must stay identical.
- Server version: settings-as-strings already required by `SendQuery()` (`DBMS_MIN_REVISION_WITH_SETTINGS_SERIALIZED_AS_STRINGS`, ClickHouse >= 20.1.2.4). Same error as `Execute()` if the server is older and settings are non-empty.
- No protocol change; reuse the existing settings serialization.
## Example usage
```cpp
clickhouse::QuerySettings settings;
settings["insert_deduplication_token"] = clickhouse::QuerySettingsField{ token };
// Same token on retry of this block; a new token for a new logical insert.
client.Insert("db.table", block, settings);
```
Retry policy for the caller:
- generate the token once per logical insert
- reuse it when retrying the **same** block after a transport/server error
- use a new token for a later insert even if the payload is identical
## Tests
Please add unit tests similar to `ClientCase.QuerySettings`:
1. `Insert()` with `insert_deduplication_token = T` twice with the **same** payload → second insert is deduplicated (one part / one row set).
2. `Insert()` with tokens `T1` then `T2` and the **same** payload → both inserts are kept.
3. Existing `Insert(table, block)` without settings still works.
4. Unknown setting with `IMPORTANT` still throws `ServerException`.
5. If `BeginInsert(Query)` is added: settings on the `Query` object actually reach the server (not dropped via `GetText()`).
A temporary table with `ENGINE = MergeTree ... SETTINGS non_replicated_deduplication_window = 100` is enough to test this without a replicated cluster.
## Why not only SQL `SETTINGS`
Embedding `SETTINGS insert_deduplication_token='...'` in the insert SQL can work, but:
- callers must escape the token
- `Insert(table, block)` still cannot do it without changing how the SQL is built
- `Query::SetSetting()` is the supported native-protocol path and already used for `Execute()`
The library should expose that path on the native insert API rather than forcing text inserts.
## References
- [insert_deduplication_token](https://clickhouse.com/docs/operations/settings/settings#insert_deduplication_token)
- [insert_deduplicate](https://clickhouse.com/docs/operations/settings/settings#insert_deduplicate)
- Error code `389 INSERT_WAS_DEDUPLICATED`
- Current README note: native `Insert()` cannot pass async-insert settings; this change would cover that case as well
Contributor guide
No contributing guide indexed for this repository
Research direction
Start at Client::Impl::Insert and Impl::BeginInsert, comparing their query construction with SendQuery(const Query&) and the existing ClientCase.QuerySettings tests. Thread QuerySettings through the native Insert overloads, preserve existing behavior, and ensure BeginInsert(Query) does not drop query settings; add coverage for deduplication tokens, unknown IMPORTANT settings, and the unchanged no-settings path.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- backend-api-design, database
- Issue type
- Feature
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 62/100