From b8185ac0b5eae25a5456301ffbffe3688bace3a0 Mon Sep 17 00:00:00 2001 From: ilandry <4749655+ilandry@users.noreply.github.com> Date: Mon, 22 Sep 2025 05:47:50 +0800 Subject: [PATCH 1/6] clickhouse client move support --- clickhouse/client.cpp | 11 +++++++---- clickhouse/client.h | 8 +++++++- tests/simple/main.cpp | 7 ++++++- 3 files changed, 20 insertions(+), 6 deletions(-) diff --git a/clickhouse/client.cpp b/clickhouse/client.cpp index 3d57ab5a..77c69641 100644 --- a/clickhouse/client.cpp +++ b/clickhouse/client.cpp @@ -446,7 +446,7 @@ void Client::Impl::SendBlockData(const Block& block) { if (compression_ == CompressionState::Enable) { std::unique_ptr compressed_output = std::make_unique(output_.get(), options_.max_compression_chunk_size, options_.compression_method); BufferedOutput buffered(std::move(compressed_output), options_.max_compression_chunk_size); - + WriteBlock(block, buffered); } else { WriteBlock(block, *output_); @@ -902,7 +902,7 @@ bool Client::Impl::ReadBlock(InputStream& input, Block* block) { if (!WireFormat::ReadString(input, &type)) { return false; } - + if (server_info_.revision >= DBMS_MIN_REVISION_WITH_CUSTOM_SERIALIZATION) { uint8_t custom_format_len; if (!WireFormat::ReadFixed(input, &custom_format_len)) { @@ -911,7 +911,7 @@ bool Client::Impl::ReadBlock(InputStream& input, Block* block) { if (custom_format_len > 0) { throw UnimplementedError(std::string("unsupported custom serialization")); } - } + } if (ColumnRef col = CreateColumnByType(type, create_column_settings)) { if (num_rows && !col->Load(&input, num_rows)) { @@ -1101,7 +1101,7 @@ void Client::Impl::SendQuery(const Query& query, bool finalize) { } WireFormat::WriteString(*output_, std::string()); // empty string after last param } - + if (finalize) { FinalizeQuery(); } @@ -1292,6 +1292,9 @@ Client::Client(const ClientOptions& opts, Client::~Client() { } +Client::Client(Client&&) = default; +Client& Client::operator=(Client&&) = default; + void Client::Execute(const Query& query) { impl_->ExecuteQuery(query); } diff --git a/clickhouse/client.h b/clickhouse/client.h index f1d50057..af2501a8 100644 --- a/clickhouse/client.h +++ b/clickhouse/client.h @@ -246,6 +246,12 @@ class Client { std::unique_ptr socket_factory); ~Client(); + // movable only + Client(Client&&); + Client& operator=(Client&&); + Client(const Client&) = delete; + Client& operator=(const Client&) = delete; + /// Intends for execute arbitrary queries. void Execute(const Query& query); @@ -346,7 +352,7 @@ class Client { static Version GetVersion(); private: - const ClientOptions options_; + ClientOptions options_; class Impl; std::unique_ptr impl_; diff --git a/tests/simple/main.cpp b/tests/simple/main.cpp index adc2c909..1c674f51 100644 --- a/tests/simple/main.cpp +++ b/tests/simple/main.cpp @@ -242,7 +242,7 @@ inline void ParamExample(Client& client) { Query query("insert into test_client values ({id: UInt64}, {name: String})"); query.SetParam("id", "1").SetParam("name", "NAME"); - client.Execute(query); + client.Execute(query); query.SetParam("id", "123").SetParam("name", "FromParam"); client.Execute(query); @@ -589,6 +589,8 @@ int main() { .SetPingBeforeQuery(true)); RunTests(client); std::cout << "current endpoint : " << client.GetCurrentEndpoint().value().host << "\n"; + Client client2 = std::move(client); + RunTests(client2); } { @@ -603,3 +605,6 @@ int main() { return 0; } + +static_assert( !std::is_copy_constructible_v ); +static_assert( std::is_move_constructible_v ); From 3fa24fdade92c7978dc75c060cd5f6a65656823c Mon Sep 17 00:00:00 2001 From: ilandry <4749655+ilandry@users.noreply.github.com> Date: Sat, 27 Sep 2025 23:58:40 +0800 Subject: [PATCH 2/6] client move unit tests --- tests/simple/main.cpp | 7 +- ut/client_ut.cpp | 197 +++++++++++++++++++++++++++++++++++++++++- 2 files changed, 197 insertions(+), 7 deletions(-) diff --git a/tests/simple/main.cpp b/tests/simple/main.cpp index 1c674f51..adc2c909 100644 --- a/tests/simple/main.cpp +++ b/tests/simple/main.cpp @@ -242,7 +242,7 @@ inline void ParamExample(Client& client) { Query query("insert into test_client values ({id: UInt64}, {name: String})"); query.SetParam("id", "1").SetParam("name", "NAME"); - client.Execute(query); + client.Execute(query); query.SetParam("id", "123").SetParam("name", "FromParam"); client.Execute(query); @@ -589,8 +589,6 @@ int main() { .SetPingBeforeQuery(true)); RunTests(client); std::cout << "current endpoint : " << client.GetCurrentEndpoint().value().host << "\n"; - Client client2 = std::move(client); - RunTests(client2); } { @@ -605,6 +603,3 @@ int main() { return 0; } - -static_assert( !std::is_copy_constructible_v ); -static_assert( std::is_move_constructible_v ); diff --git a/ut/client_ut.cpp b/ut/client_ut.cpp index f1e0e6c4..d71bb28a 100644 --- a/ut/client_ut.cpp +++ b/ut/client_ut.cpp @@ -20,6 +20,7 @@ #include #include #include +#include #include using namespace clickhouse; @@ -1998,7 +1999,7 @@ TEST_P(ClientCase, ClientName) { FlushLogs(); - std::string query_log_query + std::string query_log_query = "SELECT CAST(client_name, 'String') FROM system.query_log WHERE query_id = '" + query_id + "'"; size_t total_rows = 0; @@ -2011,3 +2012,197 @@ TEST_P(ClientCase, ClientName) { }); ASSERT_GT(total_rows, 0UL) << "Query with query_id " << query_id << " is not found"; } + + +namespace clickhouse { + +// in cpp20, the equality operators can be defaulted and this code removed + +bool operator==( + const ClientOptions::SSLOptions::CommandAndValue& lhs, + const ClientOptions::SSLOptions::CommandAndValue& rhs) { + return lhs.command == rhs.command && + lhs.value == rhs.value; +} + +bool operator!=( + const ClientOptions::SSLOptions::CommandAndValue& lhs, + const ClientOptions::SSLOptions::CommandAndValue& rhs) { + return !(lhs == rhs); +} + +auto tied(const ClientOptions::SSLOptions& o) { + return std::tie( + o.ssl_context, + o.use_default_ca_locations, + o.path_to_ca_files, + o.path_to_ca_directory, + o.min_protocol_version, + o.max_protocol_version, + o.context_options, + o.use_sni, + o.skip_verification, + o.host_flags, + o.configuration + ); +} + +bool operator==( + const ClientOptions::SSLOptions& lhs, + const ClientOptions::SSLOptions& rhs) { + return tied(lhs) == tied(rhs); +} + +bool operator!=( + const ClientOptions::SSLOptions& lhs, + const ClientOptions::SSLOptions& rhs) { + return !(lhs == rhs); +} + + +auto tied(const ClientOptions& o) { + return std::tie( + o.host, + o.port, + o.endpoints, + o.default_database, + o.user, + o.password, + o.rethrow_exceptions, + o.ping_before_query, + o.send_retries, + o.retry_timeout, + o.tcp_keepalive, + o.tcp_keepalive_idle, + o.tcp_keepalive_intvl, + o.tcp_keepalive_cnt, + o.tcp_nodelay, + o.connection_connect_timeout, + o.connection_recv_timeout, + o.connection_send_timeout, + o.backward_compatibility_lowcardinality_as_wrapped_column, + o.max_compression_chunk_size, + o.ssl_options + ); +} + +bool operator==(const ClientOptions& lhs, const ClientOptions& rhs) { + return tied(lhs) == tied(rhs); +} + +bool operator!=(const ClientOptions& lhs, const ClientOptions& rhs) { + return !(lhs == rhs); +} + +} // namespace clickhouse + + +TEST_P(ClientCase, ClientOptionsMove) { + using secs = std::chrono::seconds; + + // purposely initialize with values different than default + auto sslopt = ClientOptions::SSLOptions{} + .SetUseDefaultCALocations(false) + .SetPathToCAFiles({"path"}) + .SetPathToCADirectory({"pathdir"}) + .SetMinProtocolVersion(42) + .SetMaxProtocolVersion(99) + .SetContextOptions(33) + .SetUseSNI(false) + .SetSkipVerification(true) + .SetHostVerifyFlags(66) + .SetConfiguration({{"cmd", "value"}}); + + auto opt = ClientOptions{} + .SetHost("host") + .SetPort(static_cast(9010)) + .SetEndpoints({{"ehost", 7777}}) + .SetUser("user") + .SetPassword("pwd") + .SetDefaultDatabase("db") + .SetRethrowException(false) + .SetPingBeforeQuery(true) + .SetSendRetries(2) + .SetRetryTimeout(secs(6)) + .TcpKeepAlive(true) + .SetTcpKeepAliveIdle(secs(7)) + .SetTcpKeepAliveCount(4) + .TcpNoDelay(false) + .SetConnectionConnectTimeout(secs(8)) + .SetConnectionRecvTimeout(secs(1)) + .SetConnectionSendTimeout(secs(1)) + .SetMaxCompressionChunkSize(30000) +#ifdef WITH_OPENSSL + .SetSSLOptions(sslopt) +#endif + ; + + ASSERT_NE(opt, ClientOptions{}); + auto ogOpt = opt; + ASSERT_EQ(opt, ogOpt); + auto moveOptCtor{std::move(opt)}; + ASSERT_EQ(moveOptCtor, ogOpt); + ASSERT_NE(moveOptCtor, opt); + ASSERT_NE(moveOptCtor, ClientOptions{}); + + ASSERT_NE(opt, ogOpt); + opt = ogOpt; + ASSERT_EQ(opt, ogOpt); + auto moveOptAssign = std::move(opt); + ASSERT_EQ(moveOptAssign, moveOptCtor); + ASSERT_EQ(moveOptAssign, ogOpt); + ASSERT_NE(moveOptAssign, opt); + ASSERT_NE(moveOptAssign, ClientOptions{}); +} + +TEST_P(ClientCase, ClientMoveConstructor) { + Client client{std::move(*client_.get())}; + ::createTableWithOneColumn(client, table_name, column_name); + + client.Execute("INSERT INTO " + table_name + " (*) VALUES (\'Foo\'), (\'Bar\')"); + + std::array data{"Foo", "Bar"}; + + size_t total_rows = 0; + client.Select(getOneColumnSelectQuery(), [&total_rows, &data](const Block& block) { + total_rows += block.GetRowCount(); + if (block.GetRowCount() == 0) { + return; + } + + ASSERT_EQ(1U, block.GetColumnCount()); + if (auto col = block[0]->As>()) { + ASSERT_EQ(data.size(), col->Size()); + for (size_t i = 0; i < col->Size(); ++i) { + EXPECT_EQ(data[i], (*col)[i]) << " at index: " << i; + } + } + }); + ASSERT_EQ(total_rows, 2U); +} + +TEST_P(ClientCase, ClientMoveAssign) { + Client client = std::move(*client_.get()); + ::createTableWithOneColumn(client, table_name, column_name); + + client.Execute("INSERT INTO " + table_name + " (*) VALUES (\'Foo\'), (\'Bar\')"); + + std::array data{"Foo", "Bar"}; + + size_t total_rows = 0; + client.Select(getOneColumnSelectQuery(), [&total_rows, &data](const Block& block) { + total_rows += block.GetRowCount(); + if (block.GetRowCount() == 0) { + return; + } + + ASSERT_EQ(1U, block.GetColumnCount()); + if (auto col = block[0]->As>()) { + ASSERT_EQ(data.size(), col->Size()); + for (size_t i = 0; i < col->Size(); ++i) { + EXPECT_EQ(data[i], (*col)[i]) << " at index: " << i; + } + } + }); + ASSERT_EQ(total_rows, 2U); +} From 0753103f3f35754c0f408560cb631d18aee06e9d Mon Sep 17 00:00:00 2001 From: ilandry <4749655+ilandry@users.noreply.github.com> Date: Sun, 28 Sep 2025 00:03:14 +0800 Subject: [PATCH 3/6] client move unit tests --- ut/client_ut.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ut/client_ut.cpp b/ut/client_ut.cpp index d71bb28a..21e8ca33 100644 --- a/ut/client_ut.cpp +++ b/ut/client_ut.cpp @@ -2171,7 +2171,7 @@ TEST_P(ClientCase, ClientMoveConstructor) { } ASSERT_EQ(1U, block.GetColumnCount()); - if (auto col = block[0]->As>()) { + if (auto col = block[0]->As()) { ASSERT_EQ(data.size(), col->Size()); for (size_t i = 0; i < col->Size(); ++i) { EXPECT_EQ(data[i], (*col)[i]) << " at index: " << i; @@ -2197,7 +2197,7 @@ TEST_P(ClientCase, ClientMoveAssign) { } ASSERT_EQ(1U, block.GetColumnCount()); - if (auto col = block[0]->As>()) { + if (auto col = block[0]->As()) { ASSERT_EQ(data.size(), col->Size()); for (size_t i = 0; i < col->Size(); ++i) { EXPECT_EQ(data[i], (*col)[i]) << " at index: " << i; From a26afa0d0276658a820505917f0212da406cfb15 Mon Sep 17 00:00:00 2001 From: Andrew Slabko Date: Thu, 9 Jul 2026 11:18:31 +0200 Subject: [PATCH 4/6] Remove client_options_ from clickhouse::Client They are not used anywhere, and go straight to clickhouse::Client::Impl during initialization --- clickhouse/client.cpp | 6 ++---- clickhouse/client.h | 2 -- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/clickhouse/client.cpp b/clickhouse/client.cpp index 77c69641..4bd16c43 100644 --- a/clickhouse/client.cpp +++ b/clickhouse/client.cpp @@ -1277,15 +1277,13 @@ void Client::Impl::RetryGuard(std::function func) { } Client::Client(const ClientOptions& opts) - : options_(opts) - , impl_(new Impl(opts)) + : impl_(new Impl(opts)) { } Client::Client(const ClientOptions& opts, std::unique_ptr socket_factory) - : options_(opts) - , impl_(new Impl(opts, std::move(socket_factory))) + : impl_(new Impl(opts, std::move(socket_factory))) { } diff --git a/clickhouse/client.h b/clickhouse/client.h index af2501a8..154bb9a9 100644 --- a/clickhouse/client.h +++ b/clickhouse/client.h @@ -352,8 +352,6 @@ class Client { static Version GetVersion(); private: - ClientOptions options_; - class Impl; std::unique_ptr impl_; }; From 891b9ef16390aef0b46206032a060b062b413f93 Mon Sep 17 00:00:00 2001 From: Andrew Slabko Date: Thu, 9 Jul 2026 11:19:57 +0200 Subject: [PATCH 5/6] Simplify tests --- ut/client_ut.cpp | 198 +++++++---------------------------------------- 1 file changed, 27 insertions(+), 171 deletions(-) diff --git a/ut/client_ut.cpp b/ut/client_ut.cpp index 21e8ca33..8ff874a0 100644 --- a/ut/client_ut.cpp +++ b/ut/client_ut.cpp @@ -20,7 +20,6 @@ #include #include #include -#include #include using namespace clickhouse; @@ -2013,196 +2012,53 @@ TEST_P(ClientCase, ClientName) { ASSERT_GT(total_rows, 0UL) << "Query with query_id " << query_id << " is not found"; } - -namespace clickhouse { - -// in cpp20, the equality operators can be defaulted and this code removed - -bool operator==( - const ClientOptions::SSLOptions::CommandAndValue& lhs, - const ClientOptions::SSLOptions::CommandAndValue& rhs) { - return lhs.command == rhs.command && - lhs.value == rhs.value; -} - -bool operator!=( - const ClientOptions::SSLOptions::CommandAndValue& lhs, - const ClientOptions::SSLOptions::CommandAndValue& rhs) { - return !(lhs == rhs); -} - -auto tied(const ClientOptions::SSLOptions& o) { - return std::tie( - o.ssl_context, - o.use_default_ca_locations, - o.path_to_ca_files, - o.path_to_ca_directory, - o.min_protocol_version, - o.max_protocol_version, - o.context_options, - o.use_sni, - o.skip_verification, - o.host_flags, - o.configuration - ); -} - -bool operator==( - const ClientOptions::SSLOptions& lhs, - const ClientOptions::SSLOptions& rhs) { - return tied(lhs) == tied(rhs); -} - -bool operator!=( - const ClientOptions::SSLOptions& lhs, - const ClientOptions::SSLOptions& rhs) { - return !(lhs == rhs); -} - - -auto tied(const ClientOptions& o) { - return std::tie( - o.host, - o.port, - o.endpoints, - o.default_database, - o.user, - o.password, - o.rethrow_exceptions, - o.ping_before_query, - o.send_retries, - o.retry_timeout, - o.tcp_keepalive, - o.tcp_keepalive_idle, - o.tcp_keepalive_intvl, - o.tcp_keepalive_cnt, - o.tcp_nodelay, - o.connection_connect_timeout, - o.connection_recv_timeout, - o.connection_send_timeout, - o.backward_compatibility_lowcardinality_as_wrapped_column, - o.max_compression_chunk_size, - o.ssl_options - ); -} - -bool operator==(const ClientOptions& lhs, const ClientOptions& rhs) { - return tied(lhs) == tied(rhs); -} - -bool operator!=(const ClientOptions& lhs, const ClientOptions& rhs) { - return !(lhs == rhs); -} - -} // namespace clickhouse - - -TEST_P(ClientCase, ClientOptionsMove) { - using secs = std::chrono::seconds; - - // purposely initialize with values different than default - auto sslopt = ClientOptions::SSLOptions{} - .SetUseDefaultCALocations(false) - .SetPathToCAFiles({"path"}) - .SetPathToCADirectory({"pathdir"}) - .SetMinProtocolVersion(42) - .SetMaxProtocolVersion(99) - .SetContextOptions(33) - .SetUseSNI(false) - .SetSkipVerification(true) - .SetHostVerifyFlags(66) - .SetConfiguration({{"cmd", "value"}}); - - auto opt = ClientOptions{} - .SetHost("host") - .SetPort(static_cast(9010)) - .SetEndpoints({{"ehost", 7777}}) - .SetUser("user") - .SetPassword("pwd") - .SetDefaultDatabase("db") - .SetRethrowException(false) - .SetPingBeforeQuery(true) - .SetSendRetries(2) - .SetRetryTimeout(secs(6)) - .TcpKeepAlive(true) - .SetTcpKeepAliveIdle(secs(7)) - .SetTcpKeepAliveCount(4) - .TcpNoDelay(false) - .SetConnectionConnectTimeout(secs(8)) - .SetConnectionRecvTimeout(secs(1)) - .SetConnectionSendTimeout(secs(1)) - .SetMaxCompressionChunkSize(30000) -#ifdef WITH_OPENSSL - .SetSSLOptions(sslopt) -#endif - ; - - ASSERT_NE(opt, ClientOptions{}); - auto ogOpt = opt; - ASSERT_EQ(opt, ogOpt); - auto moveOptCtor{std::move(opt)}; - ASSERT_EQ(moveOptCtor, ogOpt); - ASSERT_NE(moveOptCtor, opt); - ASSERT_NE(moveOptCtor, ClientOptions{}); - - ASSERT_NE(opt, ogOpt); - opt = ogOpt; - ASSERT_EQ(opt, ogOpt); - auto moveOptAssign = std::move(opt); - ASSERT_EQ(moveOptAssign, moveOptCtor); - ASSERT_EQ(moveOptAssign, ogOpt); - ASSERT_NE(moveOptAssign, opt); - ASSERT_NE(moveOptAssign, ClientOptions{}); -} - TEST_P(ClientCase, ClientMoveConstructor) { Client client{std::move(*client_.get())}; - ::createTableWithOneColumn(client, table_name, column_name); - - client.Execute("INSERT INTO " + table_name + " (*) VALUES (\'Foo\'), (\'Bar\')"); - - std::array data{"Foo", "Bar"}; size_t total_rows = 0; - client.Select(getOneColumnSelectQuery(), [&total_rows, &data](const Block& block) { + client.Select("SELECT 'foobar'", [&total_rows](const Block& block) { total_rows += block.GetRowCount(); if (block.GetRowCount() == 0) { return; } ASSERT_EQ(1U, block.GetColumnCount()); - if (auto col = block[0]->As()) { - ASSERT_EQ(data.size(), col->Size()); - for (size_t i = 0; i < col->Size(); ++i) { - EXPECT_EQ(data[i], (*col)[i]) << " at index: " << i; - } - } + auto col = block[0]->As(); + ASSERT_TRUE(col); + ASSERT_EQ(1U, col->Size()); + EXPECT_EQ("foobar", (*col)[0]); }); - ASSERT_EQ(total_rows, 2U); + ASSERT_EQ(total_rows, 1U); } TEST_P(ClientCase, ClientMoveAssign) { - Client client = std::move(*client_.get()); - ::createTableWithOneColumn(client, table_name, column_name); - - client.Execute("INSERT INTO " + table_name + " (*) VALUES (\'Foo\'), (\'Bar\')"); + client_->Execute("SET param_session_id = 'initial'"); - std::array data{"Foo", "Bar"}; + ClientOptions opt = GetParam(); + Client client{opt}; + client.Execute("SET param_session_id = 'new'"); size_t total_rows = 0; - client.Select(getOneColumnSelectQuery(), [&total_rows, &data](const Block& block) { + client.Select("SELECT {session_id:String}", [&total_rows](const Block& block) { total_rows += block.GetRowCount(); - if (block.GetRowCount() == 0) { - return; + for (size_t i = 0; i < block.GetRowCount(); ++i) { + EXPECT_EQ("new", block[0]->AsStrict()->At(i)); + } + }); + ASSERT_EQ(total_rows, 1U); - ASSERT_EQ(1U, block.GetColumnCount()); - if (auto col = block[0]->As()) { - ASSERT_EQ(data.size(), col->Size()); - for (size_t i = 0; i < col->Size(); ++i) { - EXPECT_EQ(data[i], (*col)[i]) << " at index: " << i; - } + client = std::move(*client_.get()); + + total_rows = 0; + client.Select("SELECT {session_id:String}", [&total_rows](const Block& block) { + total_rows += block.GetRowCount(); + for (size_t i = 0; i < block.GetRowCount(); ++i) { + EXPECT_EQ("initial", block[0]->AsStrict()->At(i)); + // ^ + // The value has changed to the session id of the original client } }); - ASSERT_EQ(total_rows, 2U); + ASSERT_EQ(total_rows, 1U); + } From 93fa35cf859610362e759bc40ba3647915caf35f Mon Sep 17 00:00:00 2001 From: Andrew Slabko Date: Thu, 9 Jul 2026 12:50:15 +0200 Subject: [PATCH 6/6] Make Client nothrow movable --- clickhouse/client.cpp | 5 +++-- clickhouse/client.h | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/clickhouse/client.cpp b/clickhouse/client.cpp index 4bd16c43..0a8950c7 100644 --- a/clickhouse/client.cpp +++ b/clickhouse/client.cpp @@ -1290,8 +1290,9 @@ Client::Client(const ClientOptions& opts, Client::~Client() { } -Client::Client(Client&&) = default; -Client& Client::operator=(Client&&) = default; + +Client::Client(Client&&) noexcept = default; +Client& Client::operator=(Client&&) noexcept = default; void Client::Execute(const Query& query) { impl_->ExecuteQuery(query); diff --git a/clickhouse/client.h b/clickhouse/client.h index 154bb9a9..a9ccf5f2 100644 --- a/clickhouse/client.h +++ b/clickhouse/client.h @@ -247,8 +247,8 @@ class Client { ~Client(); // movable only - Client(Client&&); - Client& operator=(Client&&); + Client(Client&&) noexcept; + Client& operator=(Client&&) noexcept; Client(const Client&) = delete; Client& operator=(const Client&) = delete;