From 47dcf49930523bfcc02edffc90e39addc1f8dcac Mon Sep 17 00:00:00 2001 From: Vasily Nemkov Date: Sat, 24 Feb 2024 10:05:04 +0100 Subject: [PATCH 1/7] Removed deprecated, switched to C++20 removed: - string_view.h - string_utils.h - nasty client option backward_compatibility_lowcardinality_as_wrapped_column --- .github/workflows/windows_mingw.yml | 1 - CMakeLists.txt | 4 +- clickhouse/CMakeLists.txt | 6 +- clickhouse/base/string_utils.h | 28 ---- clickhouse/base/string_view.h | 142 --------------------- clickhouse/client.cpp | 1 - clickhouse/client.h | 9 -- clickhouse/columns/factory.cpp | 45 ++----- clickhouse/columns/factory.h | 1 - clickhouse/columns/lowcardinalityadaptor.h | 63 --------- clickhouse/types/type_parser.cpp | 42 +++--- clickhouse/types/type_parser.h | 7 +- cmake/{cpp17.cmake => cpp20.cmake} | 6 +- ut/CreateColumnByType_ut.cpp | 11 -- ut/client_ut.cpp | 47 ------- ut/low_cardinality_nullable_tests.cpp | 6 +- ut/roundtrip_tests.cpp | 7 +- 17 files changed, 51 insertions(+), 375 deletions(-) delete mode 100644 clickhouse/base/string_utils.h delete mode 100644 clickhouse/base/string_view.h delete mode 100644 clickhouse/columns/lowcardinalityadaptor.h rename cmake/{cpp17.cmake => cpp20.cmake} (69%) diff --git a/.github/workflows/windows_mingw.yml b/.github/workflows/windows_mingw.yml index 6cff77fd..27c363b9 100644 --- a/.github/workflows/windows_mingw.yml +++ b/.github/workflows/windows_mingw.yml @@ -17,7 +17,6 @@ env: BUILD_TYPE: Release CLICKHOUSE_USER: clickhouse_cpp_cicd CLICKHOUSE_PASSWORD: clickhouse_cpp_cicd - # # CLICKHOUSE_HOST: localhost # CLICKHOUSE_PORT: 9000 diff --git a/CMakeLists.txt b/CMakeLists.txt index 26ee4bec..3ab7009d 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -2,7 +2,7 @@ CMAKE_MINIMUM_REQUIRED (VERSION 3.5.2) LIST (APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake") -INCLUDE (cpp17) +INCLUDE (cpp20) INCLUDE (subdirs) INCLUDE (openssl) INCLUDE (version) @@ -22,7 +22,7 @@ PROJECT (CLICKHOUSE-CLIENT DESCRIPTION "ClickHouse C++ client library" ) -USE_CXX17 () +USE_CXX20 () USE_OPENSSL () IF (CLICKHOUSE_CPP_CHECK_VERSION) diff --git a/clickhouse/CMakeLists.txt b/clickhouse/CMakeLists.txt index 5a70403f..0c013e3b 100644 --- a/clickhouse/CMakeLists.txt +++ b/clickhouse/CMakeLists.txt @@ -45,8 +45,8 @@ SET ( clickhouse-cpp-lib-src base/singleton.h base/socket.h base/sslsocket.h - base/string_utils.h - base/string_view.h + + base/uuid.h base/wire_format.h @@ -61,7 +61,7 @@ SET ( clickhouse-cpp-lib-src columns/ip6.h columns/itemview.h columns/lowcardinality.h - columns/lowcardinalityadaptor.h + columns/map.h columns/nothing.h columns/nullable.h diff --git a/clickhouse/base/string_utils.h b/clickhouse/base/string_utils.h deleted file mode 100644 index 4d19485d..00000000 --- a/clickhouse/base/string_utils.h +++ /dev/null @@ -1,28 +0,0 @@ -#pragma once - -#include "string_view.h" - -#include -#include - -namespace clickhouse { - -template -[[deprecated("Not used by clickhosue-cpp itself, and will be removed in next major release (3.0) ")]] -inline T FromString(const std::string& s) { - std::istringstream iss(s); - T result; - iss >> result; - return result; -} - -template -[[deprecated("Not used by clickhosue-cpp itself, and will be removed in next major release (3.0) ")]] -inline T FromString(const StringView& s) { - std::istringstream iss((std::string(s))); - T result; - iss >> result; - return result; -} - -} diff --git a/clickhouse/base/string_view.h b/clickhouse/base/string_view.h deleted file mode 100644 index ad71907e..00000000 --- a/clickhouse/base/string_view.h +++ /dev/null @@ -1,142 +0,0 @@ -#pragma once - -#include -#include -#include - -/** - * A lightweight non-owning read-only view into a subsequence of a string. - */ -template < - typename TChar, - typename TTraits = std::char_traits -> -class -[[deprecated("Obsolete due to C++17's std::string_view. Will be removed in next major release (3.0) ")]] -StringViewImpl { -public: - using size_type = size_t; - using traits_type = TTraits; - using value_type = typename TTraits::char_type; - - static constexpr size_type npos = size_type(-1); - -public: - inline StringViewImpl() noexcept - : data_(nullptr) - , size_(0) - { - } - - constexpr inline StringViewImpl(const TChar* data, size_t len) noexcept - : data_(data) - , size_(len) - { - } - - template - constexpr inline StringViewImpl(const TChar (&str)[len]) noexcept - : data_(str) - , size_(len - 1) - { - } - - inline StringViewImpl(const TChar* begin, const TChar* end) noexcept - : data_(begin) - , size_(end - begin) - { - assert(begin <= end); - } - - inline StringViewImpl(const std::basic_string& str) noexcept - : data_(str.data()) - , size_(str.size()) - { - } - - inline TChar at(size_type pos) const { - if (pos >= size_) - throw std::out_of_range("pos must be less than len"); - return data_[pos]; - } - - inline const TChar* data() const noexcept { - return data_; - } - - inline bool empty() const noexcept { - return size_ == 0; - } - - inline bool null() const noexcept { - assert(size_ == 0); - return data_ == nullptr; - } - - inline size_type size() const noexcept { - return size_; - } - - // to mimic std::string and std::string_view - inline size_type length() const noexcept { - return size(); - } - -public: - // Returns a substring [pos, pos + count). - // If the requested substring extends past the end of the string, - // or if count == npos, the returned substring is [pos, size()). - StringViewImpl substr(size_type pos, size_type count = npos) const { - if (pos >= size_) - throw std::out_of_range("pos must be less than len"); - if (pos + count >= size_ || count == npos) - return StringViewImpl(data_ + pos, size_ - pos); - else - return StringViewImpl(data_ + pos, count); - } - - inline const std::basic_string to_string() const { - return std::basic_string(data_, size_); - } - -public: - inline operator bool () const noexcept { - return !empty(); - } - - inline explicit operator const std::basic_string () const { - return to_string(); - } - - inline TChar operator [] (size_type pos) const noexcept { - return data_[pos]; - } - - inline bool operator < (const StringViewImpl& other) const noexcept { - if (size_ < other.size_) - return true; - if (size_ > other.size_) - return false; - return TTraits::compare(data_, other.data_, size_) < 0; - } - - inline bool operator == (const StringViewImpl& other) const noexcept { - if (size_ == other.size_) - return TTraits::compare(data_, other.data_, size_) == 0; - return false; - } - -private: - const TChar* data_; - size_t size_; -}; - - -// It creates StringView from literal constant at compile time. -template -constexpr inline StringViewImpl MakeStringView(const TChar (&str)[size]) { - return StringViewImpl(str, size - 1); -} - - -using StringView = StringViewImpl; diff --git a/clickhouse/client.cpp b/clickhouse/client.cpp index 248f7b0a..0085a2ff 100644 --- a/clickhouse/client.cpp +++ b/clickhouse/client.cpp @@ -606,7 +606,6 @@ 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; for (size_t i = 0; i < num_columns; ++i) { std::string name; diff --git a/clickhouse/client.h b/clickhouse/client.h index 450246e3..00808d4d 100644 --- a/clickhouse/client.h +++ b/clickhouse/client.h @@ -117,15 +117,6 @@ struct ClientOptions { DECLARE_FIELD(connection_recv_timeout, std::chrono::milliseconds, SetConnectionRecvTimeout, std::chrono::milliseconds(0)); DECLARE_FIELD(connection_send_timeout, std::chrono::milliseconds, SetConnectionSendTimeout, std::chrono::milliseconds(0)); - /** It helps to ease migration of the old codebases, which can't afford to switch - * to using ColumnLowCardinalityT or ColumnLowCardinality directly, - * but still want to benefit from smaller on-wire LowCardinality bandwidth footprint. - * - * @see LowCardinalitySerializationAdaptor, CreateColumnByType - */ - [[deprecated("Makes implementation of LC(X) harder and code uglier. Will be removed in next major release (3.0) ")]] - DECLARE_FIELD(backward_compatibility_lowcardinality_as_wrapped_column, bool, SetBakcwardCompatibilityFeatureLowCardinalityAsWrappedColumn, false); - /** Set max size data to compress if compression enabled. * * Allows choosing tradeoff between RAM\CPU: diff --git a/clickhouse/columns/factory.cpp b/clickhouse/columns/factory.cpp index aeacdabc..c5a987f9 100644 --- a/clickhouse/columns/factory.cpp +++ b/clickhouse/columns/factory.cpp @@ -8,7 +8,6 @@ #include "ip4.h" #include "ip6.h" #include "lowcardinality.h" -#include "lowcardinalityadaptor.h" #include "map.h" #include "nothing.h" #include "nullable.h" @@ -194,36 +193,20 @@ static ColumnRef CreateColumnFromAst(const TypeAst& ast, CreateColumnByTypeSetti } case TypeAst::LowCardinality: { const auto nested = GetASTChildElement(ast, 0); - if (settings.low_cardinality_as_wrapped_column) { - switch (nested.code) { - // TODO (nemkov): update this to maximize code reuse. - case Type::String: - return std::make_shared>(); - case Type::FixedString: - return std::make_shared>(GetASTChildElement(nested, 0).value); - case Type::Nullable: - throw UnimplementedError("LowCardinality(" + nested.name + ") is not supported with LowCardinalityAsWrappedColumn on"); - default: - throw UnimplementedError("LowCardinality(" + nested.name + ") is not supported"); - } - } - else { - switch (nested.code) { - // TODO (nemkov): update this to maximize code reuse. - case Type::String: - return std::make_shared>(); - case Type::FixedString: - return std::make_shared>(GetASTChildElement(nested, 0).value); - case Type::Nullable: - return std::make_shared( - std::make_shared( - CreateColumnFromAst(GetASTChildElement(nested, 0), settings), - std::make_shared() - ) - ); - default: - throw UnimplementedError("LowCardinality(" + nested.name + ") is not supported"); - } + switch (nested.code) { + case Type::String: + return std::make_shared>(); + case Type::FixedString: + return std::make_shared>(GetASTChildElement(nested, 0).value); + case Type::Nullable: + return std::make_shared( + std::make_shared( + CreateColumnFromAst(GetASTChildElement(nested, 0), settings), + std::make_shared() + ) + ); + default: + throw UnimplementedError("LowCardinality(" + nested.name + ") is not supported"); } } case TypeAst::SimpleAggregateFunction: { diff --git a/clickhouse/columns/factory.h b/clickhouse/columns/factory.h index 9d2bed18..0e3a7323 100644 --- a/clickhouse/columns/factory.h +++ b/clickhouse/columns/factory.h @@ -6,7 +6,6 @@ namespace clickhouse { struct CreateColumnByTypeSettings { - bool low_cardinality_as_wrapped_column = false; }; ColumnRef CreateColumnByType(const std::string& type_name, CreateColumnByTypeSettings settings = {}); diff --git a/clickhouse/columns/lowcardinalityadaptor.h b/clickhouse/columns/lowcardinalityadaptor.h deleted file mode 100644 index bcde1a9b..00000000 --- a/clickhouse/columns/lowcardinalityadaptor.h +++ /dev/null @@ -1,63 +0,0 @@ -#pragma once - -#include "column.h" -#include "lowcardinality.h" - -#include - -namespace clickhouse { - -class OutputStream; -class CodedInputStream; - -/** Adapts any ColumnType to be serialized\deserialized as LowCardinality, - * and to be castable to ColumnType via ColumnPtr->As(). - * - * It helps to ease migration of the old codebases, which can't afford to switch - * to using ColumnLowCardinalityT or ColumnLowCardinality directly, - * but still want to benefit from smaller on-wire LowCardinality bandwidth footprint. - * - * Not intended to be used by users directly. - * - * @see ClientOptions, CreateColumnByType - */ -template -class -[[deprecated("Makes implementation of LC(X) harder and code uglier. Will be removed in next major release (3.0) ")]] -LowCardinalitySerializationAdaptor : public AdaptedColumnType -{ -public: - using AdaptedColumnType::AdaptedColumnType; - - bool LoadPrefix(InputStream* input, size_t rows) override { - auto new_data_column = this->Slice(0, 0)->template As(); - ColumnLowCardinalityT low_cardinality_col(new_data_column); - - return low_cardinality_col.LoadPrefix(input, rows); - } - - /// Loads column data from input stream. - bool LoadBody(InputStream* input, size_t rows) override { - auto new_data_column = this->CloneEmpty()->template As(); - - ColumnLowCardinalityT low_cardinality_col(new_data_column); - if (!low_cardinality_col.LoadBody(input, rows)) - return false; - - // It safe to reuse `flat_data_column` later since ColumnLowCardinalityT makes a deep copy, but still check just in case. - assert(new_data_column->Size() == 0); - - for (size_t i = 0; i < low_cardinality_col.Size(); ++i) - new_data_column->Append(low_cardinality_col[i]); - - this->Swap(*new_data_column); - return true; - } - - /// Saves column data to output stream. - void SaveBody(OutputStream* output) override { - ColumnLowCardinalityT(this->template As()).SaveBody(output); - } -}; - -} diff --git a/clickhouse/types/type_parser.cpp b/clickhouse/types/type_parser.cpp index 6142dec2..4df06407 100644 --- a/clickhouse/types/type_parser.cpp +++ b/clickhouse/types/type_parser.cpp @@ -5,8 +5,10 @@ #include #include +#include #include #include +#include #include #if defined _win_ @@ -89,7 +91,7 @@ static Type::Code GetTypeCode(const std::string& name) { return Type::Void; } -static TypeAst::Meta GetTypeMeta(const StringView& name) { +static TypeAst::Meta GetTypeMeta(const std::string_view& name) { if (name == "Array") { return TypeAst::Array; } @@ -137,7 +139,7 @@ bool ValidateAST(const TypeAst& ast) { } -TypeParser::TypeParser(const StringView& name) +TypeParser::TypeParser(const std::string_view& name) : cur_(name.data()) , end_(name.data() + name.size()) , type_(nullptr) @@ -160,18 +162,18 @@ bool TypeParser::Parse(TypeAst* type) { if (token.value.length() < 1) type_->value_string = {}; else - type_->value_string = token.value.substr(1, token.value.length() - 2).to_string(); + type_->value_string = token.value.substr(1, token.value.length() - 2); type_->code = Type::String; break; } case Token::Name: type_->meta = GetTypeMeta(token.value); - type_->name = token.value.to_string(); + type_->name = token.value; type_->code = GetTypeCode(type_->name); break; case Token::Number: type_->meta = TypeAst::Number; - type_->value = std::stol(token.value.to_string()); + type_->value = std::strtoul(token.value.data(), nullptr, 10); break; case Token::String: type_->meta = TypeAst::String; @@ -214,6 +216,8 @@ bool TypeParser::Parse(TypeAst* type) { } TypeParser::Token TypeParser::NextToken() { + static const auto EMPTY = std::string_view{}; + for (; cur_ < end_; ++cur_) { switch (*cur_) { case ' ': @@ -222,28 +226,28 @@ TypeParser::Token TypeParser::NextToken() { case '\0': continue; case '=': - return Token{Token::Assign, StringView(cur_++, 1)}; + return Token{Token::Assign, std::string_view(cur_++, 1)}; case '(': - return Token{Token::LPar, StringView(cur_++, 1)}; + return Token{Token::LPar, std::string_view(cur_++, 1)}; case ')': - return Token{Token::RPar, StringView(cur_++, 1)}; + return Token{Token::RPar, std::string_view(cur_++, 1)}; case ',': - return Token{Token::Comma, StringView(cur_++, 1)}; + return Token{Token::Comma, std::string_view(cur_++, 1)}; case '\'': { const auto end_quote_length = 1; - const StringView end_quote{cur_, end_quote_length}; + const std::string_view end_quote{cur_, end_quote_length}; // Fast forward to the closing quote. const auto start = cur_++; for (; cur_ < end_ - end_quote_length; ++cur_) { // TODO (nemkov): handle escaping ? - if (end_quote == StringView{cur_, end_quote_length}) { + if (end_quote == std::string_view{cur_, end_quote_length}) { cur_ += end_quote_length; - return Token{Token::QuotedString, StringView{start, cur_}}; + return Token{Token::QuotedString, std::string_view{start, cur_}}; } } - return Token{Token::QuotedString, StringView(cur_++, 1)}; + return Token{Token::QuotedString, std::string_view(cur_++, 1)}; } default: { @@ -252,11 +256,11 @@ TypeParser::Token TypeParser::NextToken() { if (*cur_ == '\'') { for (st = ++cur_; cur_ < end_; ++cur_) { if (*cur_ == '\'') { - return Token{Token::String, StringView(st, cur_++ - st)}; + return Token{Token::String, std::string_view(st, cur_++ - st)}; } } - return Token{Token::Invalid, StringView()}; + return Token{Token::Invalid, EMPTY}; } if (isalpha(*cur_) || *cur_ == '_') { @@ -266,7 +270,7 @@ TypeParser::Token TypeParser::NextToken() { } } - return Token{Token::Name, StringView(st, cur_)}; + return Token{Token::Name, std::string_view(st, cur_)}; } if (isdigit(*cur_) || *cur_ == '-') { @@ -276,15 +280,15 @@ TypeParser::Token TypeParser::NextToken() { } } - return Token{Token::Number, StringView(st, cur_)}; + return Token{Token::Number, std::string_view(st, cur_)}; } - return Token{Token::Invalid, StringView()}; + return Token{Token::Invalid, EMPTY}; } } } - return Token{Token::EOS, StringView()}; + return Token{Token::EOS, EMPTY}; } diff --git a/clickhouse/types/type_parser.h b/clickhouse/types/type_parser.h index 2f8f2f6f..58dde0d2 100644 --- a/clickhouse/types/type_parser.h +++ b/clickhouse/types/type_parser.h @@ -1,11 +1,10 @@ #pragma once -#include "../base/string_view.h" #include "types.h" -#include #include #include +#include namespace clickhouse { @@ -63,11 +62,11 @@ class TypeParser { }; Type type; - StringView value; + std::string_view value; }; public: - explicit TypeParser(const StringView& name); + explicit TypeParser(const std::string_view& name); ~TypeParser(); bool Parse(TypeAst* type); diff --git a/cmake/cpp17.cmake b/cmake/cpp20.cmake similarity index 69% rename from cmake/cpp17.cmake rename to cmake/cpp20.cmake index 37edc551..293c4d17 100644 --- a/cmake/cpp17.cmake +++ b/cmake/cpp20.cmake @@ -1,8 +1,8 @@ -MACRO (USE_CXX17) +MACRO (USE_CXX20) IF (CMAKE_VERSION VERSION_LESS "3.1") SET (CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++17") ELSE () - SET (CMAKE_CXX_STANDARD 17) + SET (CMAKE_CXX_STANDARD 20) SET (CMAKE_CXX_STANDARD_REQUIRED ON) ENDIF () -ENDMACRO (USE_CXX17) +ENDMACRO (USE_CXX20) diff --git a/ut/CreateColumnByType_ut.cpp b/ut/CreateColumnByType_ut.cpp index b6794275..ab612cd4 100644 --- a/ut/CreateColumnByType_ut.cpp +++ b/ut/CreateColumnByType_ut.cpp @@ -31,17 +31,6 @@ TEST(CreateColumnByType, UnmatchedBrackets) { ASSERT_EQ(nullptr, CreateColumnByType("Array(LowCardinality(Nullable(FixedString(10000)))")); } -TEST(CreateColumnByType, LowCardinalityAsWrappedColumn) { - CreateColumnByTypeSettings create_column_settings; - create_column_settings.low_cardinality_as_wrapped_column = true; - - ASSERT_EQ(Type::String, CreateColumnByType("LowCardinality(String)", create_column_settings)->GetType().GetCode()); - ASSERT_EQ(Type::String, CreateColumnByType("LowCardinality(String)", create_column_settings)->As()->GetType().GetCode()); - - ASSERT_EQ(Type::FixedString, CreateColumnByType("LowCardinality(FixedString(10000))", create_column_settings)->GetType().GetCode()); - ASSERT_EQ(Type::FixedString, CreateColumnByType("LowCardinality(FixedString(10000))", create_column_settings)->As()->GetType().GetCode()); -} - TEST(CreateColumnByType, DateTime) { ASSERT_NE(nullptr, CreateColumnByType("DateTime")); ASSERT_NE(nullptr, CreateColumnByType("DateTime('Europe/Moscow')")); diff --git a/ut/client_ut.cpp b/ut/client_ut.cpp index 7c81c519..3056cabc 100644 --- a/ut/client_ut.cpp +++ b/ut/client_ut.cpp @@ -261,53 +261,6 @@ TEST_P(ClientCase, LowCardinality_InsertAfterClear) { ASSERT_EQ(total_rows, data.size()); } -TEST_P(ClientCase, LowCardinalityString_AsString) { - // Validate that LowCardinality(String) column values can be INSERTed from client as ColumnString - // and also read on client (enabled by special option) as ColumnString. - - ClientOptions options = GetParam(); - options.SetBakcwardCompatibilityFeatureLowCardinalityAsWrappedColumn(true); - - client_ = std::make_unique(GetParam()); - // client_->Execute("CREATE DATABASE IF NOT EXISTS test_clickhouse_cpp"); - - Block block; - auto col = std::make_shared(); - - client_->Execute("DROP TEMPORARY TABLE IF EXISTS " + table_name + ";"); - client_->Execute("CREATE TEMPORARY TABLE IF NOT EXISTS " + table_name + "( " + column_name + " LowCardinality(String) )"); - - block.AppendColumn("test_column", col); - - const std::vector data{{"FooBar", "1", "2", "Foo", "4", "Bar", "Foo", "7", "8", "Foo"}}; - for (const auto & v : data) - col->Append(v); - - block.RefreshRowCount(); - client_->Insert(table_name, block); - - // Now that we can access data via ColumnString instead of ColumnLowCardinalityT - 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, data.size()); -} - TEST_P(ClientCase, Generic) { client_->Execute( "CREATE TEMPORARY TABLE IF NOT EXISTS test_clickhouse_cpp_client (id UInt64, name String, f Bool) "); diff --git a/ut/low_cardinality_nullable_tests.cpp b/ut/low_cardinality_nullable_tests.cpp index 357f28a9..c570efed 100644 --- a/ut/low_cardinality_nullable_tests.cpp +++ b/ut/low_cardinality_nullable_tests.cpp @@ -57,7 +57,6 @@ TEST(LowCardinalityOfNullable, InsertAndQuery) { block.AppendColumn("words", column); Client client(ClientOptions(localHostEndpoint) - .SetBakcwardCompatibilityFeatureLowCardinalityAsWrappedColumn(false) .SetPingBeforeQuery(true)); createTable(client); @@ -93,7 +92,6 @@ TEST(LowCardinalityOfNullable, InsertAndQueryOneRow) { block.AppendColumn("words", column); Client client(ClientOptions(localHostEndpoint) - .SetBakcwardCompatibilityFeatureLowCardinalityAsWrappedColumn(false) .SetPingBeforeQuery(true)); createTable(client); @@ -122,7 +120,6 @@ TEST(LowCardinalityOfNullable, InsertAndQueryEmpty) { block.AppendColumn("words", column); Client client(ClientOptions(localHostEndpoint) - .SetBakcwardCompatibilityFeatureLowCardinalityAsWrappedColumn(false) .SetPingBeforeQuery(true)); createTable(client); @@ -141,8 +138,7 @@ TEST(LowCardinalityOfNullable, ThrowOnBackwardsCompatibleLCColumn) { block.AppendColumn("words", column); Client client(ClientOptions(localHostEndpoint) - .SetPingBeforeQuery(true) - .SetBakcwardCompatibilityFeatureLowCardinalityAsWrappedColumn(true)); + .SetPingBeforeQuery(true)); createTable(client); diff --git a/ut/roundtrip_tests.cpp b/ut/roundtrip_tests.cpp index 99e75a1a..32c8aa47 100644 --- a/ut/roundtrip_tests.cpp +++ b/ut/roundtrip_tests.cpp @@ -325,10 +325,7 @@ INSTANTIATE_TEST_SUITE_P( ::testing::Values( ClientOptions(LocalHostEndpoint) .SetPingBeforeQuery(true) - .SetBakcwardCompatibilityFeatureLowCardinalityAsWrappedColumn(false), - - ClientOptions(LocalHostEndpoint) - .SetPingBeforeQuery(false) + , + ClientOptions(LocalHostEndpoint) .SetPingBeforeQuery(false) .SetCompressionMethod(CompressionMethod::LZ4) - .SetBakcwardCompatibilityFeatureLowCardinalityAsWrappedColumn(false) )); From aa9c955cfd8dc5b0ecc95da89cbe47549a47f6a4 Mon Sep 17 00:00:00 2001 From: Vasily Nemkov Date: Sat, 24 Feb 2024 15:19:37 +0100 Subject: [PATCH 2/7] Removed CreateColumnByTypeSettings --- clickhouse/client.cpp | 4 +--- clickhouse/columns/factory.cpp | 18 ++++++++---------- clickhouse/columns/factory.h | 6 +----- 3 files changed, 10 insertions(+), 18 deletions(-) diff --git a/clickhouse/client.cpp b/clickhouse/client.cpp index 0085a2ff..aa590512 100644 --- a/clickhouse/client.cpp +++ b/clickhouse/client.cpp @@ -605,8 +605,6 @@ bool Client::Impl::ReadBlock(InputStream& input, Block* block) { return false; } - CreateColumnByTypeSettings create_column_settings; - for (size_t i = 0; i < num_columns; ++i) { std::string name; std::string type; @@ -617,7 +615,7 @@ bool Client::Impl::ReadBlock(InputStream& input, Block* block) { return false; } - if (ColumnRef col = CreateColumnByType(type, create_column_settings)) { + if (ColumnRef col = CreateColumnByType(type)) { if (num_rows && !col->Load(&input, num_rows)) { throw ProtocolError("can't load column '" + name + "' of type " + type); } diff --git a/clickhouse/columns/factory.cpp b/clickhouse/columns/factory.cpp index c5a987f9..6cbf801d 100644 --- a/clickhouse/columns/factory.cpp +++ b/clickhouse/columns/factory.cpp @@ -131,17 +131,17 @@ static ColumnRef CreateTerminalColumn(const TypeAst& ast) { } } -static ColumnRef CreateColumnFromAst(const TypeAst& ast, CreateColumnByTypeSettings settings) { +static ColumnRef CreateColumnFromAst(const TypeAst& ast) { switch (ast.meta) { case TypeAst::Array: { return std::make_shared( - CreateColumnFromAst(GetASTChildElement(ast, 0), settings) + CreateColumnFromAst(GetASTChildElement(ast, 0)) ); } case TypeAst::Nullable: { return std::make_shared( - CreateColumnFromAst(GetASTChildElement(ast, 0), settings), + CreateColumnFromAst(GetASTChildElement(ast, 0)), std::make_shared() ); } @@ -155,7 +155,7 @@ static ColumnRef CreateColumnFromAst(const TypeAst& ast, CreateColumnByTypeSetti columns.reserve(ast.elements.size()); for (const auto& elem : ast.elements) { - if (auto col = CreateColumnFromAst(elem, settings)) { + if (auto col = CreateColumnFromAst(elem)) { columns.push_back(col); } else { return nullptr; @@ -201,7 +201,7 @@ static ColumnRef CreateColumnFromAst(const TypeAst& ast, CreateColumnByTypeSetti case Type::Nullable: return std::make_shared( std::make_shared( - CreateColumnFromAst(GetASTChildElement(nested, 0), settings), + CreateColumnFromAst(GetASTChildElement(nested, 0)), std::make_shared() ) ); @@ -222,7 +222,7 @@ static ColumnRef CreateColumnFromAst(const TypeAst& ast, CreateColumnByTypeSetti columns.reserve(ast.elements.size()); for (const auto& elem : ast.elements) { - if (auto col = CreateColumnFromAst(elem, settings)) { + if (auto col = CreateColumnFromAst(elem)) { columns.push_back(col); } else { return nullptr; @@ -246,14 +246,12 @@ static ColumnRef CreateColumnFromAst(const TypeAst& ast, CreateColumnByTypeSetti } // namespace - -ColumnRef CreateColumnByType(const std::string& type_name, CreateColumnByTypeSettings settings) { +ColumnRef CreateColumnByType(const std::string& type_name) { auto ast = ParseTypeName(type_name); if (ast != nullptr) { - return CreateColumnFromAst(*ast, settings); + return CreateColumnFromAst(*ast); } return nullptr; } - } diff --git a/clickhouse/columns/factory.h b/clickhouse/columns/factory.h index 0e3a7323..05c7358c 100644 --- a/clickhouse/columns/factory.h +++ b/clickhouse/columns/factory.h @@ -4,10 +4,6 @@ namespace clickhouse { -struct CreateColumnByTypeSettings -{ -}; - -ColumnRef CreateColumnByType(const std::string& type_name, CreateColumnByTypeSettings settings = {}); +ColumnRef CreateColumnByType(const std::string& type_name); } From 8fa835e4c2f6351d0b07fbefac37fe13f75b8dd4 Mon Sep 17 00:00:00 2001 From: Vasily Nemkov Date: Sun, 25 Feb 2024 10:00:02 +0100 Subject: [PATCH 3/7] Removed test that is no longer needed --- ut/low_cardinality_nullable_tests.cpp | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/ut/low_cardinality_nullable_tests.cpp b/ut/low_cardinality_nullable_tests.cpp index c570efed..cd13416f 100644 --- a/ut/low_cardinality_nullable_tests.cpp +++ b/ut/low_cardinality_nullable_tests.cpp @@ -131,20 +131,3 @@ TEST(LowCardinalityOfNullable, InsertAndQueryEmpty) { }); } -TEST(LowCardinalityOfNullable, ThrowOnBackwardsCompatibleLCColumn) { - auto column = buildTestColumn({}, {}); - - Block block; - block.AppendColumn("words", column); - - Client client(ClientOptions(localHostEndpoint) - .SetPingBeforeQuery(true)); - - createTable(client); - - EXPECT_THROW(client.Insert("lc_of_nullable", block), UnimplementedError); - - client.Select("SELECT * FROM lc_of_nullable", [&](const Block& bl) { - ASSERT_EQ(bl.GetRowCount(), 0u); - }); -} From 2df87817e68e97aab78fd75549af2c1da7e72612 Mon Sep 17 00:00:00 2001 From: Vasily Nemkov Date: Thu, 29 Feb 2024 08:30:27 +0100 Subject: [PATCH 4/7] Update cpp20.cmake --- cmake/cpp20.cmake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmake/cpp20.cmake b/cmake/cpp20.cmake index 293c4d17..56575fdf 100644 --- a/cmake/cpp20.cmake +++ b/cmake/cpp20.cmake @@ -1,6 +1,6 @@ MACRO (USE_CXX20) IF (CMAKE_VERSION VERSION_LESS "3.1") - SET (CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++17") + SET (CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++20") ELSE () SET (CMAKE_CXX_STANDARD 20) SET (CMAKE_CXX_STANDARD_REQUIRED ON) From 3a3fedfdcb70fb2e87eb0e33e7e1689f41722a6d Mon Sep 17 00:00:00 2001 From: wangwei <1261385937@qq.com> Date: Sat, 20 Apr 2024 14:04:50 +0800 Subject: [PATCH 5/7] fix compile error --- .github/workflows/linux.yml | 22 +--------------------- clickhouse/base/compressed.cpp | 4 ---- clickhouse/columns/lowcardinality.cpp | 3 ++- 3 files changed, 3 insertions(+), 26 deletions(-) diff --git a/.github/workflows/linux.yml b/.github/workflows/linux.yml index 9994762a..6128f14d 100644 --- a/.github/workflows/linux.yml +++ b/.github/workflows/linux.yml @@ -23,36 +23,16 @@ jobs: fail-fast: false matrix: os: [ubuntu-20.04] - compiler: [clang-6, clang-10-libc++, gcc-7, gcc-8, gcc-9] + compiler: [clang-10-libc++] ssl: [ssl_ON, ssl_OFF] dependencies: [dependencies_BUILT_IN] include: - - compiler: clang-6 - COMPILER_INSTALL: clang-6.0 libc++-dev - C_COMPILER: clang-6.0 - CXX_COMPILER: clang++-6.0 - - compiler: clang-10-libc++ COMPILER_INSTALL: clang-10 libc++-dev C_COMPILER: clang-10 CXX_COMPILER: clang++-10 - - compiler: gcc-7 - COMPILER_INSTALL: gcc-7 g++-7 - C_COMPILER: gcc-7 - CXX_COMPILER: g++-7 - - - compiler: gcc-8 - COMPILER_INSTALL: gcc-8 g++-8 - C_COMPILER: gcc-8 - CXX_COMPILER: g++-8 - - - compiler: gcc-9 - COMPILER_INSTALL: gcc-9 g++-9 - C_COMPILER: gcc-9 - CXX_COMPILER: g++-9 - - ssl: ssl_ON SSL_CMAKE_OPTION: -D WITH_OPENSSL=ON diff --git a/clickhouse/base/compressed.cpp b/clickhouse/base/compressed.cpp index b5cefe4c..00bb58d6 100644 --- a/clickhouse/base/compressed.cpp +++ b/clickhouse/base/compressed.cpp @@ -27,11 +27,7 @@ CompressedInput::CompressedInput(InputStream* input) CompressedInput::~CompressedInput() { if (!mem_.Exhausted()) { -#if __cplusplus < 201703L - if (!std::uncaught_exception()) { -#else if (!std::uncaught_exceptions()) { -#endif throw LZ4Error("some data was not read"); } } diff --git a/clickhouse/columns/lowcardinality.cpp b/clickhouse/columns/lowcardinality.cpp index 19369d33..06ca47df 100644 --- a/clickhouse/columns/lowcardinality.cpp +++ b/clickhouse/columns/lowcardinality.cpp @@ -346,7 +346,8 @@ void ColumnLowCardinality::SavePrefix(OutputStream* output) { } void ColumnLowCardinality::SaveBody(OutputStream* output) { - const uint64_t index_serialization_type = indexTypeFromIndexColumn(*index_column_) | IndexFlag::HasAdditionalKeysBit; + const uint64_t index_serialization_type = static_cast(indexTypeFromIndexColumn(*index_column_)) + | static_cast(IndexFlag::HasAdditionalKeysBit); WireFormat::WriteFixed(*output, index_serialization_type); const uint64_t number_of_keys = dictionary_column_->Size(); From 7d9f8e68627dd9ad891a3b20c7d0deee177818b6 Mon Sep 17 00:00:00 2001 From: wangwei <1261385937@qq.com> Date: Sat, 20 Apr 2024 14:55:50 +0800 Subject: [PATCH 6/7] fix compile error --- clickhouse/CMakeLists.txt | 2 -- clickhouse/columns/lowcardinality.cpp | 4 ++-- ut/utils_meta.h | 7 +------ 3 files changed, 3 insertions(+), 10 deletions(-) diff --git a/clickhouse/CMakeLists.txt b/clickhouse/CMakeLists.txt index 0c013e3b..0ae94238 100644 --- a/clickhouse/CMakeLists.txt +++ b/clickhouse/CMakeLists.txt @@ -86,8 +86,6 @@ SET ( clickhouse-cpp-lib-src if (MSVC) add_definitions(-D_CRT_SECURE_NO_WARNINGS) add_compile_options(/W4) - # remove in 3.0 - add_compile_options(/wd4996) else() set(cxx_extra_wall "-Wempty-body -Wconversion -Wreturn-type -Wparentheses -Wuninitialized -Wunreachable-code -Wunused-function -Wunused-value -Wunused-variable") set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${cxx_extra_wall}") diff --git a/clickhouse/columns/lowcardinality.cpp b/clickhouse/columns/lowcardinality.cpp index 06ca47df..f778d0cc 100644 --- a/clickhouse/columns/lowcardinality.cpp +++ b/clickhouse/columns/lowcardinality.cpp @@ -346,8 +346,8 @@ void ColumnLowCardinality::SavePrefix(OutputStream* output) { } void ColumnLowCardinality::SaveBody(OutputStream* output) { - const uint64_t index_serialization_type = static_cast(indexTypeFromIndexColumn(*index_column_)) - | static_cast(IndexFlag::HasAdditionalKeysBit); + const uint64_t index_serialization_type = static_cast(indexTypeFromIndexColumn(*index_column_)) + | static_cast(IndexFlag::HasAdditionalKeysBit); WireFormat::WriteFixed(*output, index_serialization_type); const uint64_t number_of_keys = dictionary_column_->Size(); diff --git a/ut/utils_meta.h b/ut/utils_meta.h index 5b08c319..f15ea373 100644 --- a/ut/utils_meta.h +++ b/ut/utils_meta.h @@ -34,12 +34,7 @@ inline constexpr bool is_container_v = is_container::value; // Since result_of is deprecated in C++17, and invoke_result_of is unavailable until C++20... template -using my_result_of_t = -#if __cplusplus >= 201703L - std::invoke_result_t; -#else - std::result_of_t; -#endif +using my_result_of_t = std::invoke_result_t; // https://stackoverflow.com/a/11251408 template < template class Template, typename T > From bc9557cf994b881935cd1237151f93a1a632ebde Mon Sep 17 00:00:00 2001 From: wangwei <1261385937@qq.com> Date: Sat, 20 Apr 2024 14:56:46 +0800 Subject: [PATCH 7/7] fix TypeParser::Parse -> case Token::Number --- clickhouse/types/type_parser.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/clickhouse/types/type_parser.cpp b/clickhouse/types/type_parser.cpp index 4df06407..ceb9b530 100644 --- a/clickhouse/types/type_parser.cpp +++ b/clickhouse/types/type_parser.cpp @@ -173,7 +173,7 @@ bool TypeParser::Parse(TypeAst* type) { break; case Token::Number: type_->meta = TypeAst::Number; - type_->value = std::strtoul(token.value.data(), nullptr, 10); + type_->value = std::strtoll(token.value.data(), nullptr, 10); break; case Token::String: type_->meta = TypeAst::String; @@ -244,7 +244,8 @@ TypeParser::Token TypeParser::NextToken() { if (end_quote == std::string_view{cur_, end_quote_length}) { cur_ += end_quote_length; - return Token{Token::QuotedString, std::string_view{start, cur_}}; + return Token{ Token::QuotedString, + std::string_view{start, static_cast(cur_ - start)} }; } } return Token{Token::QuotedString, std::string_view(cur_++, 1)}; @@ -270,7 +271,7 @@ TypeParser::Token TypeParser::NextToken() { } } - return Token{Token::Name, std::string_view(st, cur_)}; + return Token{ Token::Name, std::string_view(st, static_cast(cur_ - st)) }; } if (isdigit(*cur_) || *cur_ == '-') { @@ -280,7 +281,7 @@ TypeParser::Token TypeParser::NextToken() { } } - return Token{Token::Number, std::string_view(st, cur_)}; + return Token{ Token::Number, std::string_view(st, static_cast(cur_ - st)) }; } return Token{Token::Invalid, EMPTY};