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

Open
#565 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
4/5
Estimated time
3-5 days
Newbie friendliness
62/100
Issue type
Feature
Clarity
Mostly clear
Activity status
Active
Tech stack
cpp

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.

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

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:

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:

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).

/// 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&):

Block BeginInsert(const Query& query);

That would let callers do:

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):

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:

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

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

Dominant language
C
Stars
382
Forks
208
Avg merge
2d 19h
Merged PRs (30d)
14

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from ClickHouse/clickhouse-cpp

All issues in ClickHouse/clickhouse-cpp

Similar issues

More C issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.