From 5d535f8edb900c1a511d38b4f30fe17299ed682a Mon Sep 17 00:00:00 2001 From: wangwei <1261385937@qq.com> Date: Sat, 10 Jun 2023 14:45:19 +0800 Subject: [PATCH 1/6] add more strict syntax and fix compile error --- clickhouse/CMakeLists.txt | 11 +++++++++++ clickhouse/base/compressed.cpp | 2 +- clickhouse/base/socket.cpp | 2 +- clickhouse/client.cpp | 10 ++++------ clickhouse/columns/enum.cpp | 4 ++-- clickhouse/columns/factory.cpp | 2 +- clickhouse/columns/lowcardinality.cpp | 2 +- 7 files changed, 21 insertions(+), 12 deletions(-) diff --git a/clickhouse/CMakeLists.txt b/clickhouse/CMakeLists.txt index 67663ec5..40813e34 100644 --- a/clickhouse/CMakeLists.txt +++ b/clickhouse/CMakeLists.txt @@ -34,6 +34,17 @@ SET ( clickhouse-cpp-lib-src query.cpp ) +if (MSVC) + add_definitions(-D_CRT_SECURE_NO_WARNINGS) + add_compile_options(/W4) + add_compile_options(/wd4996) +endif() + +if (UNIX) + set(cxx_extra_wall "-Wno-deprecated-declarations -Wempty-body -Wconversion -Wreturn-type -Wparentheses -Wno-format -Wuninitialized -Wunreachable-code -Wunused-function -Wunused-value -Wunused-variable") + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${cxx_extra_wall}") +endif () + IF (WITH_OPENSSL) LIST(APPEND clickhouse-cpp-lib-src base/sslsocket.cpp) ENDIF () diff --git a/clickhouse/base/compressed.cpp b/clickhouse/base/compressed.cpp index 4d5cc65d..5730e855 100644 --- a/clickhouse/base/compressed.cpp +++ b/clickhouse/base/compressed.cpp @@ -141,7 +141,7 @@ void CompressedOutput::Compress(const void * data, size_t len) { const auto compressed_size = LZ4_compress_default( (const char*)data, (char*)compressed_buffer_.data() + HEADER_SIZE, - len, + static_cast(len), static_cast(compressed_buffer_.size() - HEADER_SIZE)); if (compressed_size <= 0) throw LZ4Error("Failed to compress chunk of " + std::to_string(len) + " bytes, " diff --git a/clickhouse/base/socket.cpp b/clickhouse/base/socket.cpp index 48e90c73..7d7326a4 100644 --- a/clickhouse/base/socket.cpp +++ b/clickhouse/base/socket.cpp @@ -217,7 +217,7 @@ SOCKET SocketConnect(const NetworkAddress& addr, const SocketTimeoutParams& time fd.fd = *s; fd.events = POLLOUT; fd.revents = 0; - ssize_t rval = Poll(&fd, 1, timeout_params.connect_timeout.count()); + ssize_t rval = Poll(&fd, 1, static_cast(timeout_params.connect_timeout.count())); if (rval == -1) { throw std::system_error(getSocketErrorCode(), getErrorCategory(), "fail to connect"); diff --git a/clickhouse/client.cpp b/clickhouse/client.cpp index e4b0c7ef..2f675da0 100644 --- a/clickhouse/client.cpp +++ b/clickhouse/client.cpp @@ -411,12 +411,12 @@ bool Client::Impl::ReceivePacket(uint64_t* server_packet) { if (!WireFormat::ReadUInt64(*input_, &info.bytes)) { return false; } - if (REVISION >= DBMS_MIN_REVISION_WITH_TOTAL_ROWS_IN_PROGRESS) { + if constexpr(REVISION >= DBMS_MIN_REVISION_WITH_TOTAL_ROWS_IN_PROGRESS) { if (!WireFormat::ReadUInt64(*input_, &info.total_rows)) { return false; } } - if (REVISION >= DBMS_MIN_REVISION_WITH_CLIENT_WRITE_INFO) + if constexpr (REVISION >= DBMS_MIN_REVISION_WITH_CLIENT_WRITE_INFO) { if (!WireFormat::ReadUInt64(*input_, &info.written_rows)) { return false; @@ -499,13 +499,11 @@ bool Client::Impl::ReceivePacket(uint64_t* server_packet) { throw UnimplementedError("unimplemented " + std::to_string((int)packet_type)); break; } - - return false; } bool Client::Impl::ReadBlock(InputStream& input, Block* block) { // Additional information about block. - if (REVISION >= DBMS_MIN_REVISION_WITH_BLOCK_INFO) { + if constexpr (REVISION >= DBMS_MIN_REVISION_WITH_BLOCK_INFO) { uint64_t num; BlockInfo info; @@ -569,7 +567,7 @@ bool Client::Impl::ReadBlock(InputStream& input, Block* block) { bool Client::Impl::ReceiveData() { Block block; - if (REVISION >= DBMS_MIN_REVISION_WITH_TEMPORARY_TABLES) { + if constexpr (REVISION >= DBMS_MIN_REVISION_WITH_TEMPORARY_TABLES) { if (!WireFormat::SkipString(*input_)) { return false; } diff --git a/clickhouse/columns/enum.cpp b/clickhouse/columns/enum.cpp index 1361e817..cd7696ca 100644 --- a/clickhouse/columns/enum.cpp +++ b/clickhouse/columns/enum.cpp @@ -30,7 +30,7 @@ void ColumnEnum::Append(const T& value, bool checkValue) { template void ColumnEnum::Append(const std::string& name) { - data_.push_back(type_->As()->GetEnumValue(name)); + data_.push_back(static_cast(type_->As()->GetEnumValue(name))); } template @@ -63,7 +63,7 @@ void ColumnEnum::SetAt(size_t n, const T& value, bool checkValue) { template void ColumnEnum::SetNameAt(size_t n, const std::string& name) { - data_.at(n) = type_->As()->GetEnumValue(name); + data_.at(n) = static_cast(type_->As()->GetEnumValue(name)); } template diff --git a/clickhouse/columns/factory.cpp b/clickhouse/columns/factory.cpp index e003b7f5..aeacdabc 100644 --- a/clickhouse/columns/factory.cpp +++ b/clickhouse/columns/factory.cpp @@ -38,7 +38,7 @@ const auto& GetASTChildElement(const TypeAst & ast, int position) { throw ValidationError("AST child element index out of bounds: " + std::to_string(position)); if (position < 0) - position = ast.elements.size() + position; + position = static_cast(ast.elements.size() + position); return ast.elements[static_cast(position)]; } diff --git a/clickhouse/columns/lowcardinality.cpp b/clickhouse/columns/lowcardinality.cpp index d3627038..13bf17a1 100644 --- a/clickhouse/columns/lowcardinality.cpp +++ b/clickhouse/columns/lowcardinality.cpp @@ -199,7 +199,7 @@ std::uint64_t ColumnLowCardinality::getDictionaryIndex(std::uint64_t item_index) void ColumnLowCardinality::appendIndex(std::uint64_t item_index) { // TODO (nemkov): handle case when index should go from UInt8 to UInt16, etc. VisitIndexColumn([item_index](auto & arg) { - arg.Append(item_index); + arg.Append(static_cast::DataType>(item_index)); }, *index_column_); } From 82386bb5b20495e649ce36e4e4491793da03303e Mon Sep 17 00:00:00 2001 From: wangwei <1261385937@qq.com> Date: Sat, 10 Jun 2023 14:45:19 +0800 Subject: [PATCH 2/6] add more strict syntax and fix compile error --- clickhouse/CMakeLists.txt | 11 +++++++++++ clickhouse/base/compressed.cpp | 4 ++-- clickhouse/base/socket.cpp | 2 +- clickhouse/client.cpp | 10 ++++------ clickhouse/columns/enum.cpp | 4 ++-- clickhouse/columns/factory.cpp | 2 +- clickhouse/columns/lowcardinality.cpp | 2 +- 7 files changed, 22 insertions(+), 13 deletions(-) diff --git a/clickhouse/CMakeLists.txt b/clickhouse/CMakeLists.txt index 67663ec5..40813e34 100644 --- a/clickhouse/CMakeLists.txt +++ b/clickhouse/CMakeLists.txt @@ -34,6 +34,17 @@ SET ( clickhouse-cpp-lib-src query.cpp ) +if (MSVC) + add_definitions(-D_CRT_SECURE_NO_WARNINGS) + add_compile_options(/W4) + add_compile_options(/wd4996) +endif() + +if (UNIX) + set(cxx_extra_wall "-Wno-deprecated-declarations -Wempty-body -Wconversion -Wreturn-type -Wparentheses -Wno-format -Wuninitialized -Wunreachable-code -Wunused-function -Wunused-value -Wunused-variable") + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${cxx_extra_wall}") +endif () + IF (WITH_OPENSSL) LIST(APPEND clickhouse-cpp-lib-src base/sslsocket.cpp) ENDIF () diff --git a/clickhouse/base/compressed.cpp b/clickhouse/base/compressed.cpp index 4d5cc65d..135f9483 100644 --- a/clickhouse/base/compressed.cpp +++ b/clickhouse/base/compressed.cpp @@ -94,7 +94,7 @@ bool CompressedInput::Decompress() { data_ = Buffer(original); - if (LZ4_decompress_safe((const char*)tmp.data() + HEADER_SIZE, (char*)data_.data(), compressed - HEADER_SIZE, original) < 0) { + if (LZ4_decompress_safe((const char*)tmp.data() + HEADER_SIZE, (char*)data_.data(), static_cast(compressed - HEADER_SIZE), original) < 0) { throw LZ4Error("can't decompress data"); } else { mem_.Reset(data_.data(), original); @@ -141,7 +141,7 @@ void CompressedOutput::Compress(const void * data, size_t len) { const auto compressed_size = LZ4_compress_default( (const char*)data, (char*)compressed_buffer_.data() + HEADER_SIZE, - len, + static_cast(len), static_cast(compressed_buffer_.size() - HEADER_SIZE)); if (compressed_size <= 0) throw LZ4Error("Failed to compress chunk of " + std::to_string(len) + " bytes, " diff --git a/clickhouse/base/socket.cpp b/clickhouse/base/socket.cpp index 48e90c73..7d7326a4 100644 --- a/clickhouse/base/socket.cpp +++ b/clickhouse/base/socket.cpp @@ -217,7 +217,7 @@ SOCKET SocketConnect(const NetworkAddress& addr, const SocketTimeoutParams& time fd.fd = *s; fd.events = POLLOUT; fd.revents = 0; - ssize_t rval = Poll(&fd, 1, timeout_params.connect_timeout.count()); + ssize_t rval = Poll(&fd, 1, static_cast(timeout_params.connect_timeout.count())); if (rval == -1) { throw std::system_error(getSocketErrorCode(), getErrorCategory(), "fail to connect"); diff --git a/clickhouse/client.cpp b/clickhouse/client.cpp index e4b0c7ef..2f675da0 100644 --- a/clickhouse/client.cpp +++ b/clickhouse/client.cpp @@ -411,12 +411,12 @@ bool Client::Impl::ReceivePacket(uint64_t* server_packet) { if (!WireFormat::ReadUInt64(*input_, &info.bytes)) { return false; } - if (REVISION >= DBMS_MIN_REVISION_WITH_TOTAL_ROWS_IN_PROGRESS) { + if constexpr(REVISION >= DBMS_MIN_REVISION_WITH_TOTAL_ROWS_IN_PROGRESS) { if (!WireFormat::ReadUInt64(*input_, &info.total_rows)) { return false; } } - if (REVISION >= DBMS_MIN_REVISION_WITH_CLIENT_WRITE_INFO) + if constexpr (REVISION >= DBMS_MIN_REVISION_WITH_CLIENT_WRITE_INFO) { if (!WireFormat::ReadUInt64(*input_, &info.written_rows)) { return false; @@ -499,13 +499,11 @@ bool Client::Impl::ReceivePacket(uint64_t* server_packet) { throw UnimplementedError("unimplemented " + std::to_string((int)packet_type)); break; } - - return false; } bool Client::Impl::ReadBlock(InputStream& input, Block* block) { // Additional information about block. - if (REVISION >= DBMS_MIN_REVISION_WITH_BLOCK_INFO) { + if constexpr (REVISION >= DBMS_MIN_REVISION_WITH_BLOCK_INFO) { uint64_t num; BlockInfo info; @@ -569,7 +567,7 @@ bool Client::Impl::ReadBlock(InputStream& input, Block* block) { bool Client::Impl::ReceiveData() { Block block; - if (REVISION >= DBMS_MIN_REVISION_WITH_TEMPORARY_TABLES) { + if constexpr (REVISION >= DBMS_MIN_REVISION_WITH_TEMPORARY_TABLES) { if (!WireFormat::SkipString(*input_)) { return false; } diff --git a/clickhouse/columns/enum.cpp b/clickhouse/columns/enum.cpp index 1361e817..cd7696ca 100644 --- a/clickhouse/columns/enum.cpp +++ b/clickhouse/columns/enum.cpp @@ -30,7 +30,7 @@ void ColumnEnum::Append(const T& value, bool checkValue) { template void ColumnEnum::Append(const std::string& name) { - data_.push_back(type_->As()->GetEnumValue(name)); + data_.push_back(static_cast(type_->As()->GetEnumValue(name))); } template @@ -63,7 +63,7 @@ void ColumnEnum::SetAt(size_t n, const T& value, bool checkValue) { template void ColumnEnum::SetNameAt(size_t n, const std::string& name) { - data_.at(n) = type_->As()->GetEnumValue(name); + data_.at(n) = static_cast(type_->As()->GetEnumValue(name)); } template diff --git a/clickhouse/columns/factory.cpp b/clickhouse/columns/factory.cpp index e003b7f5..aeacdabc 100644 --- a/clickhouse/columns/factory.cpp +++ b/clickhouse/columns/factory.cpp @@ -38,7 +38,7 @@ const auto& GetASTChildElement(const TypeAst & ast, int position) { throw ValidationError("AST child element index out of bounds: " + std::to_string(position)); if (position < 0) - position = ast.elements.size() + position; + position = static_cast(ast.elements.size() + position); return ast.elements[static_cast(position)]; } diff --git a/clickhouse/columns/lowcardinality.cpp b/clickhouse/columns/lowcardinality.cpp index d3627038..13bf17a1 100644 --- a/clickhouse/columns/lowcardinality.cpp +++ b/clickhouse/columns/lowcardinality.cpp @@ -199,7 +199,7 @@ std::uint64_t ColumnLowCardinality::getDictionaryIndex(std::uint64_t item_index) void ColumnLowCardinality::appendIndex(std::uint64_t item_index) { // TODO (nemkov): handle case when index should go from UInt8 to UInt16, etc. VisitIndexColumn([item_index](auto & arg) { - arg.Append(item_index); + arg.Append(static_cast::DataType>(item_index)); }, *index_column_); } From 532aa49fdff4c3220dfa5b429cbb03896bc8079f Mon Sep 17 00:00:00 2001 From: wangwei <1261385937@qq.com> Date: Sat, 10 Jun 2023 16:53:02 +0800 Subject: [PATCH 3/6] fix compile error --- clickhouse/CMakeLists.txt | 5 ++++- clickhouse/base/sslsocket.cpp | 8 ++++---- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/clickhouse/CMakeLists.txt b/clickhouse/CMakeLists.txt index 40813e34..3915b26d 100644 --- a/clickhouse/CMakeLists.txt +++ b/clickhouse/CMakeLists.txt @@ -40,8 +40,9 @@ if (MSVC) add_compile_options(/wd4996) endif() -if (UNIX) +if (UNIX) set(cxx_extra_wall "-Wno-deprecated-declarations -Wempty-body -Wconversion -Wreturn-type -Wparentheses -Wno-format -Wuninitialized -Wunreachable-code -Wunused-function -Wunused-value -Wunused-variable") + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${cxx_extra_wall}") endif () @@ -66,6 +67,8 @@ ENDIF() IF (CMAKE_CXX_COMPILER_ID STREQUAL "Clang") + # a little abnormal when clang check conversion + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${cxx_extra_wall} -Wno-conversion") INCLUDE (CheckCXXSourceCompiles) CHECK_CXX_SOURCE_COMPILES("#include \nint main() { return __GLIBCXX__ != 0; }" diff --git a/clickhouse/base/sslsocket.cpp b/clickhouse/base/sslsocket.cpp index 8b1971e0..76ea9d3b 100644 --- a/clickhouse/base/sslsocket.cpp +++ b/clickhouse/base/sslsocket.cpp @@ -109,7 +109,7 @@ SSL_CTX * prepareSSLContext(const clickhouse::SSLParams & context_params) { #define HANDLE_SSL_CTX_ERROR(statement) do { \ if (const auto ret_code = (statement); !ret_code) \ - throwSSLError(nullptr, ERR_peek_error(), LOCATION, #statement); \ + throwSSLError(nullptr, static_cast(ERR_peek_error()), LOCATION, #statement); \ } while(false); if (context_params.use_default_ca_locations) @@ -185,7 +185,7 @@ SSL_CTX * SSLContext::getContext() { // Allows caller to use returned value of `statement` if there was no error, throws exception otherwise. #define HANDLE_SSL_ERROR(SSL_PTR, statement) [&] { \ if (const auto ret_code = (statement); ret_code <= 0) { \ - throwSSLError(SSL_PTR, SSL_get_error(SSL_PTR, ret_code), LOCATION, #statement); \ + throwSSLError(SSL_PTR, SSL_get_error(SSL_PTR, static_cast(ret_code)), LOCATION, #statement); \ return static_cast>(0); \ } \ else \ @@ -209,7 +209,7 @@ SSLSocket::SSLSocket(const NetworkAddress& addr, const SocketTimeoutParams& time std::unique_ptr ip_addr(a2i_IPADDRESS(addr.Host().c_str()), &ASN1_OCTET_STRING_free); - HANDLE_SSL_ERROR(ssl, SSL_set_fd(ssl, handle_)); + HANDLE_SSL_ERROR(ssl, SSL_set_fd(ssl, static_cast(handle_))); if (ssl_params.use_SNI) HANDLE_SSL_ERROR(ssl, SSL_set_tlsext_host_name(ssl, addr.Host().c_str())); @@ -295,7 +295,7 @@ SSLSocketOutput::SSLSocketOutput(SSL *ssl) {} size_t SSLSocketOutput::DoWrite(const void* data, size_t len) { - return static_cast(HANDLE_SSL_ERROR(ssl_, SSL_write(ssl_, data, len))); + return static_cast(HANDLE_SSL_ERROR(ssl_, SSL_write(ssl_, data, static_cast(len)))); } #undef HANDLE_SSL_ERROR From 6f47162a55e4d67613578d8ff3f859c63e0a357c Mon Sep 17 00:00:00 2001 From: wangwei <1261385937@qq.com> Date: Sat, 10 Jun 2023 17:24:49 +0800 Subject: [PATCH 4/6] fix compile error --- clickhouse/CMakeLists.txt | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/clickhouse/CMakeLists.txt b/clickhouse/CMakeLists.txt index 3915b26d..fd16d790 100644 --- a/clickhouse/CMakeLists.txt +++ b/clickhouse/CMakeLists.txt @@ -38,13 +38,15 @@ if (MSVC) add_definitions(-D_CRT_SECURE_NO_WARNINGS) add_compile_options(/W4) add_compile_options(/wd4996) -endif() - -if (UNIX) - set(cxx_extra_wall "-Wno-deprecated-declarations -Wempty-body -Wconversion -Wreturn-type -Wparentheses -Wno-format -Wuninitialized -Wunreachable-code -Wunused-function -Wunused-value -Wunused-variable") - +else() + set(cxx_extra_wall "-Wno-deprecated-declarations -Wempty-body -Wconversion -Wreturn-type -Wparentheses -Wno-format -Wuninitialized -Wunreachable-code -Wunused-function -Wunused-value -Wunused-variable") set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${cxx_extra_wall}") -endif () + + if(CMAKE_CXX_COMPILER_ID MATCHES "Clang" OR CMAKE_CXX_COMPILER_ID STREQUAL "AppleClang") + # a little abnormal when clang check conversion + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${cxx_extra_wall} -Wno-conversion") + endif() +endif() IF (WITH_OPENSSL) LIST(APPEND clickhouse-cpp-lib-src base/sslsocket.cpp) @@ -67,8 +69,6 @@ ENDIF() IF (CMAKE_CXX_COMPILER_ID STREQUAL "Clang") - # a little abnormal when clang check conversion - set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${cxx_extra_wall} -Wno-conversion") INCLUDE (CheckCXXSourceCompiles) CHECK_CXX_SOURCE_COMPILES("#include \nint main() { return __GLIBCXX__ != 0; }" From 08bc16ee32f4c6d421ddbe987e3d9423d89c536b Mon Sep 17 00:00:00 2001 From: wangwei <1261385937@qq.com> Date: Sat, 10 Jun 2023 20:08:20 +0800 Subject: [PATCH 5/6] remove -Wno-* flag --- clickhouse/CMakeLists.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/clickhouse/CMakeLists.txt b/clickhouse/CMakeLists.txt index fd16d790..ec544e1b 100644 --- a/clickhouse/CMakeLists.txt +++ b/clickhouse/CMakeLists.txt @@ -37,9 +37,10 @@ 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 "-Wno-deprecated-declarations -Wempty-body -Wconversion -Wreturn-type -Wparentheses -Wno-format -Wuninitialized -Wunreachable-code -Wunused-function -Wunused-value -Wunused-variable") + 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}") if(CMAKE_CXX_COMPILER_ID MATCHES "Clang" OR CMAKE_CXX_COMPILER_ID STREQUAL "AppleClang") From 6a9f0774d7a831b2215f3c09bee263270aabcce6 Mon Sep 17 00:00:00 2001 From: Vasily Nemkov Date: Wed, 14 Jun 2023 12:20:45 +0200 Subject: [PATCH 6/6] SSLSocketOutput::DoWrite exception in case of too much data This is a temporary solution and should be changed to invoking SSL_write() as many times are required until all data is written --- clickhouse/base/sslsocket.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/clickhouse/base/sslsocket.cpp b/clickhouse/base/sslsocket.cpp index 76ea9d3b..4c5185d8 100644 --- a/clickhouse/base/sslsocket.cpp +++ b/clickhouse/base/sslsocket.cpp @@ -295,6 +295,10 @@ SSLSocketOutput::SSLSocketOutput(SSL *ssl) {} size_t SSLSocketOutput::DoWrite(const void* data, size_t len) { + if (len > std::numeric_limits::max()) + // FIXME(vnemkov): We should do multiple `SSL_write`s in this case. + throw AssertionError("Failed to write too big chunk at once " + + std::to_string(len) + " > " + std::to_string(std::numeric_limits::max())); return static_cast(HANDLE_SSL_ERROR(ssl_, SSL_write(ssl_, data, static_cast(len)))); }