From d3984cfa07544f69e4576e93a0e0f4dd85cb6b42 Mon Sep 17 00:00:00 2001 From: Ilia Alshanetsky Date: Wed, 10 Jun 2026 16:36:29 -0400 Subject: [PATCH 1/4] client: add BeginInsert(const Query&) overload Add public overload to support fully configured Query (with settings, params, query_id, callbacks) for streaming inserts. Removed the legacy string overloads for BeginInsert (they were thin wrappers); string literals now resolve via the Query overload (implicit ctor, which is kept non-explicit for convenience). Includes the internal fix to pass the full Query to SendQuery (instead of GetText()) so that query_id etc. are preserved when using the rich form. This was the original local patch; exercised by bindings using settings on insert/writeStart. --- clickhouse/client.cpp | 10 +++------- clickhouse/client.h | 4 ++-- clickhouse/query.h | 4 ++-- 3 files changed, 7 insertions(+), 11 deletions(-) diff --git a/clickhouse/client.cpp b/clickhouse/client.cpp index b5d287bc..f560aedb 100644 --- a/clickhouse/client.cpp +++ b/clickhouse/client.cpp @@ -546,7 +546,7 @@ Block Client::Impl::BeginInsert(Query query) { return true; }); - SendQuery(query.GetText()); + SendQuery(query); // Wait for a data packet and return uint64_t server_packet = 0; @@ -1373,12 +1373,8 @@ void Client::Insert(const std::string& table_name, const std::string& query_id, impl_->Insert(table_name, query_id, block); } -Block Client::BeginInsert(const std::string& query) { - return impl_->BeginInsert(Query(query)); -} - -Block Client::BeginInsert(const std::string& query, const std::string& query_id) { - return impl_->BeginInsert(Query(query, query_id)); +Block Client::BeginInsert(const Query& query) { + return impl_->BeginInsert(query); } void Client::SendInsertBlock(const Block& block) { diff --git a/clickhouse/client.h b/clickhouse/client.h index a55fd748..766ce9a1 100644 --- a/clickhouse/client.h +++ b/clickhouse/client.h @@ -302,8 +302,8 @@ class Client { void Insert(const std::string& table_name, const std::string& query_id, const Block& block); /// Start an \p INSERT statement, insert batches of data, then finish the insert. - Block BeginInsert(const std::string& query); - Block BeginInsert(const std::string& query, const std::string& query_id); + /// Use BeginInsert(const Query&) for settings, query_id, callbacks etc. + Block BeginInsert(const Query& query); /// Insert data using a \p block returned by \p BeginInsert. void SendInsertBlock(const Block& block); diff --git a/clickhouse/query.h b/clickhouse/query.h index 0bec97f6..785eac5d 100644 --- a/clickhouse/query.h +++ b/clickhouse/query.h @@ -88,8 +88,8 @@ using ProfileCallback = std::function; class Query : public QueryEvents { public: Query(); - Query(const char* query, const char* query_id = nullptr); - Query(const std::string& query, const std::string& query_id = default_query_id); + explicit Query(const char* query, const char* query_id = nullptr); + explicit Query(const std::string& query, const std::string& query_id = default_query_id); ~Query() override; /// From 171ab90785cc95e42a20b9ae592d5dbd6cfadaa8 Mon Sep 17 00:00:00 2001 From: Ilia Alshanetsky Date: Wed, 10 Jun 2026 16:53:54 -0400 Subject: [PATCH 2/4] query: keep implicit Query constructors Making the Query ctors explicit broke every call site that passes string literals to Execute(), Select(), BeginInsert(), etc. The BeginInsert(const Query&) overload still accepts string literals via the implicit conversion, matching the PR intent. --- clickhouse/query.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clickhouse/query.h b/clickhouse/query.h index 785eac5d..0bec97f6 100644 --- a/clickhouse/query.h +++ b/clickhouse/query.h @@ -88,8 +88,8 @@ using ProfileCallback = std::function; class Query : public QueryEvents { public: Query(); - explicit Query(const char* query, const char* query_id = nullptr); - explicit Query(const std::string& query, const std::string& query_id = default_query_id); + Query(const char* query, const char* query_id = nullptr); + Query(const std::string& query, const std::string& query_id = default_query_id); ~Query() override; /// From 9af742b70f602c4ad57bcd5a310604f6c3ebe0cc Mon Sep 17 00:00:00 2001 From: Ilia Alshanetsky Date: Fri, 12 Jun 2026 08:50:00 -0400 Subject: [PATCH 3/4] client: keep two-arg BeginInsert(string, query_id) overload Restore BeginInsert(const std::string&, const std::string&) alongside the new BeginInsert(const Query&). Single-string callers already convert to Query implicitly, but the two-arg (query, query_id) form has no implicit conversion, so dropping it was a source break. It differs in arity from the Query overload, so there is no ambiguity. --- clickhouse/client.cpp | 4 ++++ clickhouse/client.h | 4 +++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/clickhouse/client.cpp b/clickhouse/client.cpp index f560aedb..7015f759 100644 --- a/clickhouse/client.cpp +++ b/clickhouse/client.cpp @@ -1377,6 +1377,10 @@ Block Client::BeginInsert(const Query& query) { return impl_->BeginInsert(query); } +Block Client::BeginInsert(const std::string& query, const std::string& query_id) { + return impl_->BeginInsert(Query(query, query_id)); +} + void Client::SendInsertBlock(const Block& block) { impl_->SendInsertBlock(block); } diff --git a/clickhouse/client.h b/clickhouse/client.h index 766ce9a1..135836a4 100644 --- a/clickhouse/client.h +++ b/clickhouse/client.h @@ -302,8 +302,10 @@ class Client { void Insert(const std::string& table_name, const std::string& query_id, const Block& block); /// Start an \p INSERT statement, insert batches of data, then finish the insert. - /// Use BeginInsert(const Query&) for settings, query_id, callbacks etc. + /// A bare string converts to a Query implicitly; use BeginInsert(const Query&) + /// for settings, params, callbacks etc. Block BeginInsert(const Query& query); + Block BeginInsert(const std::string& query, const std::string& query_id); /// Insert data using a \p block returned by \p BeginInsert. void SendInsertBlock(const Block& block); From 31f266769064e0171879e2ae3c6a930759a84cb0 Mon Sep 17 00:00:00 2001 From: Ilia Alshanetsky Date: Fri, 12 Jun 2026 13:48:47 -0400 Subject: [PATCH 4/4] client: reject Query callbacks in BeginInsert(const Query&) BeginInsert installs its own block-capture OnData but leaves every other handler on the passed Query live. EnsureNull points events_ at that Query for the whole call, so the insert handshake's packet loop can still drive OnServerException, OnProgress, OnProfile, OnServerLog, OnProfileEvents, and OnDataCancelable into the caller's handlers. Reject any Query that carries a callback instead of silently running it on a partially-supported path. Adds Query::HasEventCallbacks() and a unit test covering both the rejection and the settings/query_id streaming path. --- clickhouse/client.cpp | 4 ++++ clickhouse/client.h | 3 ++- clickhouse/query.h | 8 ++++++++ ut/client_ut.cpp | 39 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 53 insertions(+), 1 deletion(-) diff --git a/clickhouse/client.cpp b/clickhouse/client.cpp index 7015f759..dc0d9c09 100644 --- a/clickhouse/client.cpp +++ b/clickhouse/client.cpp @@ -531,6 +531,10 @@ Block Client::Impl::BeginInsert(Query query) { throw ValidationError("cannot execute query while executing another operation"); } + if (query.HasEventCallbacks()) { + throw ValidationError("Query callbacks are not supported in BeginInsert"); + } + EnsureNull en(static_cast(&query), &events_); if (options_.ping_before_query) { diff --git a/clickhouse/client.h b/clickhouse/client.h index 135836a4..7e7bac65 100644 --- a/clickhouse/client.h +++ b/clickhouse/client.h @@ -303,7 +303,8 @@ class Client { /// Start an \p INSERT statement, insert batches of data, then finish the insert. /// A bare string converts to a Query implicitly; use BeginInsert(const Query&) - /// for settings, params, callbacks etc. + /// to pass per-insert settings, params, and query_id. Event callbacks on the + /// Query are rejected (throws ValidationError): BeginInsert owns the data path. Block BeginInsert(const Query& query); Block BeginInsert(const std::string& query, const std::string& query_id); diff --git a/clickhouse/query.h b/clickhouse/query.h index 0bec97f6..e701638b 100644 --- a/clickhouse/query.h +++ b/clickhouse/query.h @@ -179,6 +179,14 @@ class Query : public QueryEvents { return *this; } + /// True if any server-event handler is installed. BeginInsert rejects such + /// a Query: it drives its own block-capture OnData and would otherwise let + /// the caller's handlers fire from the insert handshake's packet loop. + inline bool HasEventCallbacks() const { + return exception_cb_ || progress_cb_ || select_cb_ || select_cancelable_cb_ + || select_server_log_cb_ || profile_events_callback_cb_ || profile_callback_cb_; + } + static const std::string default_query_id; private: diff --git a/ut/client_ut.cpp b/ut/client_ut.cpp index 3b6b2d5c..a1bdd39e 100644 --- a/ut/client_ut.cpp +++ b/ut/client_ut.cpp @@ -541,6 +541,45 @@ TEST_P(ClientCase, InsertData) { EXPECT_EQ(exp, row); } +TEST_P(ClientCase, BeginInsertQuery) { + client_->Execute( + "CREATE TEMPORARY TABLE IF NOT EXISTS test_clickhouse_cpp_begin_insert_query (id UInt64)"); + + /// A Query carrying any event callback is rejected: BeginInsert drives its + /// own data path and the caller's handlers would otherwise fire from the + /// insert handshake's packet loop. + { + Query q("INSERT INTO test_clickhouse_cpp_begin_insert_query VALUES"); + q.OnException([](const Exception&) {}); + EXPECT_THROW(client_->BeginInsert(q), ValidationError); + } + + /// A Query with per-insert settings and a query_id (no callbacks) streams + /// normally through the overload. + { + Query q("INSERT INTO test_clickhouse_cpp_begin_insert_query VALUES", "begin-insert-query-id"); + q.SetSetting("max_block_size", {"100"}); + + auto block = client_->BeginInsert(q); + ASSERT_EQ(size_t(1), block.GetColumnCount()); + + auto id = block[0]->As(); + id->Append(42); + block.RefreshRowCount(); + client_->SendInsertBlock(block); + client_->EndInsert(); + } + + size_t row = 0; + client_->Select("SELECT id FROM test_clickhouse_cpp_begin_insert_query", + [&row](const Block& block) { + for (size_t c = 0; c < block.GetRowCount(); ++c, ++row) { + EXPECT_EQ(uint64_t(42), (*block[0]->As())[c]); + } + }); + EXPECT_EQ(size_t(1), row); +} + TEST_P(ClientCase, Nullable) { /// Create a table. client_->Execute(