ClickHouse / ClickHouse/clickhouse-cpp

Allow optional QuerySettings on Client::Insert() for native-format inserts

Open
#565 0 comments 0 reactions 0 assignees View on GitHub
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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.