From d488d1952e9c5050feabbe9a618ca2a6ee89dd12 Mon Sep 17 00:00:00 2001 From: alen <7587499+alenstarx@users.noreply.github.com> Date: Tue, 18 Jul 2023 10:49:12 +0800 Subject: [PATCH 1/3] Update client.cpp Fix abnormal column names in gcc7.3.1 --- clickhouse/client.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clickhouse/client.cpp b/clickhouse/client.cpp index 3b75efc0..ca57190b 100644 --- a/clickhouse/client.cpp +++ b/clickhouse/client.cpp @@ -609,9 +609,9 @@ bool Client::Impl::ReadBlock(InputStream& input, Block* block) { CreateColumnByTypeSettings create_column_settings; create_column_settings.low_cardinality_as_wrapped_column = options_.backward_compatibility_lowcardinality_as_wrapped_column; - std::string name; - std::string type; for (size_t i = 0; i < num_columns; ++i) { + std::string name; + std::string type; if (!WireFormat::ReadString(input, &name)) { return false; } From b77a1b041841194e4f8c22ffb6d7878149b8c9be Mon Sep 17 00:00:00 2001 From: alenstar Date: Sat, 22 Jul 2023 15:51:13 +0800 Subject: [PATCH 2/3] add test for Fix abnormal column names in gcc7.3.1 #317 --- ut/CMakeLists.txt | 1 + ut/abnormal_column_names_test.cpp | 47 +++++++++++++++++++++++++++++++ ut/abnormal_column_names_test.h | 18 ++++++++++++ ut/client_ut.cpp | 17 +++++++++++ 4 files changed, 83 insertions(+) create mode 100644 ut/abnormal_column_names_test.cpp create mode 100644 ut/abnormal_column_names_test.h diff --git a/ut/CMakeLists.txt b/ut/CMakeLists.txt index 2c9f6ee5..6b395458 100644 --- a/ut/CMakeLists.txt +++ b/ut/CMakeLists.txt @@ -15,6 +15,7 @@ SET ( clickhouse-cpp-ut-src performance_tests.cpp tcp_server.cpp readonly_client_test.cpp + abnormal_column_names_test.cpp connection_failed_client_test.cpp array_of_low_cardinality_tests.cpp CreateColumnByType_ut.cpp diff --git a/ut/abnormal_column_names_test.cpp b/ut/abnormal_column_names_test.cpp new file mode 100644 index 00000000..93ed2343 --- /dev/null +++ b/ut/abnormal_column_names_test.cpp @@ -0,0 +1,47 @@ +#include "abnormal_column_names_test.h" +#include "utils.h" + +#include +#include +#include +#include + +namespace { + using namespace clickhouse; +} + +void AbnormalColumnNamesTest::SetUp() { + client_ = std::make_unique(std::get<0>(GetParam())); +} + +void AbnormalColumnNamesTest::TearDown() { + client_.reset(); +} + +// Sometimes gtest fails to detect that this test is instantiated elsewhere, suppress the error explicitly. +GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(AbnormalColumnNamesTest); +TEST_P(AbnormalColumnNamesTest, Select) { + + const auto & queries = std::get<1>(GetParam()); + for (const auto & query : queries) { + std::unordered_set names; + size_t count = 0; + client_->Select(query, + [& query,& names, & count](const Block& block) { + if (block.GetRowCount() == 0 || block.GetColumnCount() == 0) + return; + + std::cout << "query => " << query <<"\n" << PrettyPrintBlock{block}; + for (size_t i = 0; i < block.GetColumnCount(); ++i) + { + count++; + names.insert(block.GetColumnName(i)); + } + } + ); + EXPECT_EQ(count, names.size()); + for(auto& name: names) { + std::cout << name << ", count=" << count<< std::endl; + } + } +} diff --git a/ut/abnormal_column_names_test.h b/ut/abnormal_column_names_test.h new file mode 100644 index 00000000..be5bc53c --- /dev/null +++ b/ut/abnormal_column_names_test.h @@ -0,0 +1,18 @@ +#pragma once + +#include + +#include + +#include +#include +#include + +class AbnormalColumnNamesTest : public testing::TestWithParam< + std::tuple > /*queries*/> { +protected: + void SetUp() override; + void TearDown() override; + + std::unique_ptr client_; +}; diff --git a/ut/client_ut.cpp b/ut/client_ut.cpp index 3ed1092e..f023924a 100644 --- a/ut/client_ut.cpp +++ b/ut/client_ut.cpp @@ -1,6 +1,7 @@ #include #include "readonly_client_test.h" +#include "abnormal_column_names_test.h" #include "connection_failed_client_test.h" #include "utils.h" @@ -1196,6 +1197,22 @@ INSTANTIATE_TEST_SUITE_P(ClientLocalReadonly, ReadonlyClientTest, } )); +INSTANTIATE_TEST_SUITE_P(ColumnNames, AbnormalColumnNamesTest, + ::testing::Values(AbnormalColumnNamesTest::ParamType{ + ClientOptions() + .SetHost( getEnvOrDefault("CLICKHOUSE_HOST", "localhost")) + .SetPort( getEnvOrDefault("CLICKHOUSE_PORT", "9000")) + .SetUser( getEnvOrDefault("CLICKHOUSE_USER", "default")) + .SetPassword( getEnvOrDefault("CLICKHOUSE_PASSWORD", "")) + .SetDefaultDatabase(getEnvOrDefault("CLICKHOUSE_DB", "default")) + .SetSendRetries(1) + .SetPingBeforeQuery(true) + .SetCompressionMethod(CompressionMethod::None), + {"select 123,231,113", "select 'ABC','AAA','BBB','CCC'"} + } +)); + + INSTANTIATE_TEST_SUITE_P(ClientLocalFailed, ConnectionFailedClientTest, ::testing::Values(ConnectionFailedClientTest::ParamType{ ClientOptions() From 1701f1e27825f1806f7e489a81cf97bb559e074a Mon Sep 17 00:00:00 2001 From: alenstarx Date: Sat, 29 Jul 2023 12:28:43 +0800 Subject: [PATCH 3/3] rename AbnormalColumnNamesTest to AbnormalColumnNamesClientTest and move test suite instantiation right to the definition of the test itself --- ut/abnormal_column_names_test.cpp | 66 ++++++++++++++++++++++--------- ut/abnormal_column_names_test.h | 2 +- ut/client_ut.cpp | 16 -------- 3 files changed, 48 insertions(+), 36 deletions(-) diff --git a/ut/abnormal_column_names_test.cpp b/ut/abnormal_column_names_test.cpp index 93ed2343..cb8c81eb 100644 --- a/ut/abnormal_column_names_test.cpp +++ b/ut/abnormal_column_names_test.cpp @@ -10,38 +10,66 @@ namespace { using namespace clickhouse; } -void AbnormalColumnNamesTest::SetUp() { +void AbnormalColumnNamesClientTest::SetUp() { client_ = std::make_unique(std::get<0>(GetParam())); } -void AbnormalColumnNamesTest::TearDown() { +void AbnormalColumnNamesClientTest::TearDown() { client_.reset(); } // Sometimes gtest fails to detect that this test is instantiated elsewhere, suppress the error explicitly. -GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(AbnormalColumnNamesTest); -TEST_P(AbnormalColumnNamesTest, Select) { - +GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(AbnormalColumnNamesClientTest); +TEST_P(AbnormalColumnNamesClientTest, Select) { + static const std::vector expect_results { + "+-------+-------+-------+\n"\ + "| 123 | 231 | 113 |\n"\ + "+-------+-------+-------+\n"\ + "| UInt8 | UInt8 | UInt8 |\n"\ + "+-------+-------+-------+\n"\ + "| 123 | 231 | 113 |\n"\ + "+-------+-------+-------+\n", + "+--------+--------+--------+--------+\n"\ + "| 'ABC' | 'AAA' | 'BBB' | 'CCC' |\n"\ + "+--------+--------+--------+--------+\n"\ + "| String | String | String | String |\n"\ + "+--------+--------+--------+--------+\n"\ + "| ABC | AAA | BBB | CCC |\n"\ + "+--------+--------+--------+--------+\n" + }; const auto & queries = std::get<1>(GetParam()); - for (const auto & query : queries) { - std::unordered_set names; - size_t count = 0; + for (size_t i = 0; i < queries.size(); ++i) { + const auto & query = queries.at(i); client_->Select(query, - [& query,& names, & count](const Block& block) { + [& queries, i](const Block& block) { if (block.GetRowCount() == 0 || block.GetColumnCount() == 0) return; + EXPECT_EQ(1UL, block.GetRowCount()); + EXPECT_EQ(i == 0 ? 3UL: 4UL, block.GetColumnCount()); - std::cout << "query => " << query <<"\n" << PrettyPrintBlock{block}; - for (size_t i = 0; i < block.GetColumnCount(); ++i) - { - count++; - names.insert(block.GetColumnName(i)); - } + std::stringstream sstr; + sstr << PrettyPrintBlock{block}; + auto result = sstr.str(); + std::cout << "query => " << queries.at(i) <<"\n" << PrettyPrintBlock{block}; + ASSERT_EQ(expect_results.at(i), result); } ); - EXPECT_EQ(count, names.size()); - for(auto& name: names) { - std::cout << name << ", count=" << count<< std::endl; - } } } + + +INSTANTIATE_TEST_SUITE_P(ClientColumnNames, AbnormalColumnNamesClientTest, + ::testing::Values(AbnormalColumnNamesClientTest::ParamType{ + ClientOptions() + .SetHost( getEnvOrDefault("CLICKHOUSE_HOST", "localhost")) + .SetPort( getEnvOrDefault("CLICKHOUSE_PORT", "9000")) + .SetUser( getEnvOrDefault("CLICKHOUSE_USER", "default")) + .SetPassword( getEnvOrDefault("CLICKHOUSE_PASSWORD", "")) + .SetDefaultDatabase(getEnvOrDefault("CLICKHOUSE_DB", "default")) + .SetSendRetries(1) + .SetPingBeforeQuery(true) + .SetCompressionMethod(CompressionMethod::None), + {"select 123,231,113", "select 'ABC','AAA','BBB','CCC'"} + } +)); + diff --git a/ut/abnormal_column_names_test.h b/ut/abnormal_column_names_test.h index be5bc53c..b0f56c62 100644 --- a/ut/abnormal_column_names_test.h +++ b/ut/abnormal_column_names_test.h @@ -8,7 +8,7 @@ #include #include -class AbnormalColumnNamesTest : public testing::TestWithParam< +class AbnormalColumnNamesClientTest : public testing::TestWithParam< std::tuple > /*queries*/> { protected: void SetUp() override; diff --git a/ut/client_ut.cpp b/ut/client_ut.cpp index f023924a..d894cc36 100644 --- a/ut/client_ut.cpp +++ b/ut/client_ut.cpp @@ -1,7 +1,6 @@ #include #include "readonly_client_test.h" -#include "abnormal_column_names_test.h" #include "connection_failed_client_test.h" #include "utils.h" @@ -1197,21 +1196,6 @@ INSTANTIATE_TEST_SUITE_P(ClientLocalReadonly, ReadonlyClientTest, } )); -INSTANTIATE_TEST_SUITE_P(ColumnNames, AbnormalColumnNamesTest, - ::testing::Values(AbnormalColumnNamesTest::ParamType{ - ClientOptions() - .SetHost( getEnvOrDefault("CLICKHOUSE_HOST", "localhost")) - .SetPort( getEnvOrDefault("CLICKHOUSE_PORT", "9000")) - .SetUser( getEnvOrDefault("CLICKHOUSE_USER", "default")) - .SetPassword( getEnvOrDefault("CLICKHOUSE_PASSWORD", "")) - .SetDefaultDatabase(getEnvOrDefault("CLICKHOUSE_DB", "default")) - .SetSendRetries(1) - .SetPingBeforeQuery(true) - .SetCompressionMethod(CompressionMethod::None), - {"select 123,231,113", "select 'ABC','AAA','BBB','CCC'"} - } -)); - INSTANTIATE_TEST_SUITE_P(ClientLocalFailed, ConnectionFailedClientTest, ::testing::Values(ConnectionFailedClientTest::ParamType{