Merge pull request #233 from Enmk/deprecation_attributes

Minor: Deprecated some APIs

Author: Enmk

CMakeLists.txt

@@ -23,7 +23,8 @@ PROJECT (CLICKHOUSE-CLIENT)
         ENDIF ()
         SET (CMAKE_EXE_LINKER_FLAGS, "${CMAKE_EXE_LINKER_FLAGS} -lpthread")
         # -Wpedantic makes int128 support somewhat harder and less performant (by not allowing builtin __int128)
-        SET (CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -Wextra -Werror")
+        # -Wno-deprecated-declarations to produce less cluttered output when building library itself (`deprecated` attributes are for library users)
+        SET (CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -Wextra -Werror -Wno-deprecated-declarations")
     ENDIF ()
 
     INCLUDE_DIRECTORIES (.)

clickhouse/base/sslsocket.cpp

@@ -78,11 +78,11 @@ void configureSSL(const clickhouse::SSLParams::ConfigurationType & configuration
         else if (err == 1 && value_present)
             throw clickhouse::OpenSSLError("Failed to configure OpenSSL: command '" + kv.first + "' needs no value");
         else if (err == -2)
-            throw clickhouse::OpenSSLError("Failed to cofigure OpenSSL: unknown command '" + kv.first + "'");
+            throw clickhouse::OpenSSLError("Failed to configure OpenSSL: unknown command '" + kv.first + "'");
         else if (err == -3)
-            throw clickhouse::OpenSSLError("Failed to cofigure OpenSSL: command '" + kv.first + "' requires a value");
+            throw clickhouse::OpenSSLError("Failed to configure OpenSSL: command '" + kv.first + "' requires a value");
         else
-            throw clickhouse::OpenSSLError("Failed to cofigure OpenSSL: command '" + kv.first + "' unknown error: " + std::to_string(err));
+            throw clickhouse::OpenSSLError("Failed to configure OpenSSL: command '" + kv.first + "' unknown error: " + std::to_string(err));
     }
 }
 

clickhouse/base/string_utils.h

@@ -8,6 +8,7 @@
 namespace clickhouse {
 
 template <typename T>
+[[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;
@@ -16,6 +17,7 @@ inline T FromString(const std::string& s) {
 }
 
 template <typename T>
+[[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;

clickhouse/client.h

@@ -90,22 +90,20 @@ struct ClientOptions {
     DECLARE_FIELD(connection_recv_timeout, std::chrono::seconds, SetConnectionRecvTimeout, std::chrono::seconds(0));
     DECLARE_FIELD(connection_send_timeout, std::chrono::seconds, SetConnectionSendTimeout, std::chrono::seconds(0));
 
-    // TODO deprecate setting
     /** 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, true);
 
     /** Set max size data to compress if compression enabled.
      *
-     *  Allows choosing tradeoff betwen RAM\CPU:
+     *  Allows choosing tradeoff between RAM\CPU:
      *  - Lower value reduces RAM usage, but slightly increases CPU usage.
      *  - Higher value increases RAM usage but slightly decreases CPU usage.
-     *
-     *  Default is 0, use natural implementation-defined chunk size.
      */
     DECLARE_FIELD(max_compression_chunk_size, unsigned int, SetMaxCompressionChunkSize, 65535);
 
@@ -133,7 +131,7 @@ struct ClientOptions {
          *  If no CAs are configured, the server's identity can't be validated, and the Client would err.
          *  See https://www.openssl.org/docs/man1.1.1/man3/SSL_CTX_set_default_verify_paths.html
         */
-        /// Load deafult CA certificates from deafult locations.
+        /// Load default CA certificates from default locations.
         DECLARE_FIELD(use_default_ca_locations, bool, SetUseDefaultCALocations, true);
         /// Path to the CA files to verify server certificate, may be empty.
         DECLARE_FIELD(path_to_ca_files, std::vector<std::string>, SetPathToCAFiles, {});

clickhouse/columns/array.h

@@ -19,7 +19,7 @@ class ColumnArray : public Column {
 
     /** Create an array of given type.
      *
-     *  `data` is used internaly (and modified) by ColumnArray.
+     *  `data` is used internally (and modified) by ColumnArray.
      *  Users are strongly advised against supplying non-empty columns and/or modifying
      *  contents of `data` afterwards.
      */
@@ -35,7 +35,7 @@ class ColumnArray : public Column {
     /// Converts input column to array and appends as one row to the current column.
     void AppendAsColumn(ColumnRef array);
 
-    /// Convets array at pos n to column.
+    /// Converts array at pos n to column.
     /// Type of element of result column same as type of array element.
     ColumnRef GetAsColumn(size_t n) const;
 

clickhouse/columns/date.h

@@ -145,8 +145,8 @@ class ColumnDateTime64 : public Column {
 
     /// Appends one element to the end of column.
     void Append(const Int64& value);
-    // It is a bit controversal: users might expect it to parse string of ISO8601 or some other human-friendly format,
-    // but current implemntation parses it as fractional integer with decimal point, e.g. "123.456".
+    // It is a bit controversial: users might expect it to parse string of ISO8601 or some other human-friendly format,
+    // but current implementation parses it as fractional integer with decimal point, e.g. "123.456".
 //    void Append(const std::string& value);
 
     /// Returns element at given row number.

clickhouse/columns/itemview.cpp

@@ -90,7 +90,7 @@ void ItemView::ValidateData(Type::Code type, DataType data) {
             return AssertSize({4, 8, 16});
 
         default:
-            throw UnimplementedError("Unknon type code:" + std::to_string(static_cast<int>(type)));
+            throw UnimplementedError("Unknown type code:" + std::to_string(static_cast<int>(type)));
     }
 }
 

clickhouse/protocol.h

@@ -7,7 +7,7 @@ namespace clickhouse {
         enum {
             Hello                = 0,    /// Name, version, revision.
             Data                 = 1,    /// `Block` of data, may be compressed.
-            Exception            = 2,    /// Exception that occured on server side during query execution.
+            Exception            = 2,    /// Exception that occurred on server side during query execution.
             Progress             = 3,    /// Query execcution progress: rows and bytes read.
             Pong                 = 4,    /// response to Ping sent by client.
             EndOfStream          = 5,    /// All packets were sent.

clickhouse/query.h

@@ -123,7 +123,7 @@ class Query : public QueryEvents {
         return *this;
     }
 
-    /// Set handler for receiving a progress of query exceution.
+    /// Set handler for receiving a progress of query execution.
     inline Query& OnProgress(ProgressCallback cb) {
         progress_cb_ = std::move(cb);
         return *this;

ut/Column_ut.cpp

@@ -283,14 +283,14 @@ TYPED_TEST(GenericColumnTest, RoundTrip) {
         // Date32 first appeared in v21.9.2.17-stable
         const auto server_info = client.GetServerInfo();
         if (versionNumber(server_info) < versionNumber(21, 9)) {
-            GTEST_SKIP() << "Date32 is availble since v21.9.2.17-stable and can't be tested against server: " << server_info;
+            GTEST_SKIP() << "Date32 is available since v21.9.2.17-stable and can't be tested against server: " << server_info;
         }
     }
 
     if constexpr (std::is_same_v<typename TestFixture::ColumnType, ColumnInt128>) {
         const auto server_info = client.GetServerInfo();
         if (versionNumber(server_info) < versionNumber(21, 7)) {
-            GTEST_SKIP() << "ColumnInt128 is availble since v21.7.2.7-stable and can't be tested against server: " << server_info;
+            GTEST_SKIP() << "ColumnInt128 is available since v21.7.2.7-stable and can't be tested against server: " << server_info;
         }
     }
 

ut/client_ut.cpp

@@ -428,7 +428,7 @@ TEST_P(ClientCase, Cancellable) {
             "CREATE TEMPORARY TABLE IF NOT EXISTS test_clickhouse_cpp_cancel (x UInt64) ");
 
     /// Insert a few blocks. In order to make cancel have effect, we have to
-    /// insert a relative larget amount of data.
+    /// insert a relative larger amount of data.
     const int kBlock = 10;
     const int kRowEachBlock = 1000000;
     for (unsigned j = 0; j < kBlock; j++) {
@@ -856,7 +856,7 @@ TEST_P(ClientCase, Query_ID) {
         // DB::Exception: clickhouse_cpp_cicd: Not enough privileges. To execute this query it's necessary to have grant SYSTEM FLUSH LOGS ON
         if (std::string(e.what()).find("To execute this query it's necessary to have grant SYSTEM FLUSH LOGS ON") != std::string::npos) {
             // Insufficient privileges, the only safe way is to wait long enough for system
-            // to flush the logs automaticaly. Usualy it takes 7.5 seconds, so just in case,
+            // to flush the logs automatically. Usually it takes 7.5 seconds, so just in case,
             // wait 3 times that to ensure that all previously executed queries are in the logs now.
             const auto wait_duration = std::chrono::seconds(23);
             std::cerr << "Got error while flushing logs, now we wait " << wait_duration << "..." << std::endl;

ut/columns_ut.cpp

@@ -626,7 +626,7 @@ TEST(ColumnsCase, ColumnDecimal128_from_string_overflow) {
     EXPECT_ANY_THROW(col->Append("400000000000000000000000000000000000000"));
 
 #ifndef ABSL_HAVE_INTRINSIC_INT128
-    // unfortunatelly std::numeric_limits<Int128>::min() overflows when there is no __int128 intrinsic type.
+    // unfortunately std::numeric_limits<Int128>::min() overflows when there is no __int128 intrinsic type.
     EXPECT_ANY_THROW(col->Append("-170141183460469231731687303715884105728"));
 #endif
 }
@@ -682,7 +682,7 @@ TEST(ColumnsCase, ColumnLowCardinalityString_Load) {
     }
 }
 
-// This is temporary diabled since we are not 100% compatitable with ClickHouse
+// This is temporary disabled since we are not 100% compatitable with ClickHouse
 // on how we serailize LC columns, but we check interoperability in other tests (see client_ut.cpp)
 TEST(ColumnsCase, DISABLED_ColumnLowCardinalityString_Save) {
     const size_t items_count = 10;

ut/ssl_ut.cpp

@@ -75,7 +75,7 @@ INSTANTIATE_TEST_SUITE_P(
     }
 ));
 
-// For some reasons doen't work on MacOS.
+// For some reasons doesn't work on MacOS.
 // Looks like `VerifyCAPath` has no effect, while parsing and setting value works.
 // Also for some reason SetPathToCADirectory() + SSL_CTX_load_verify_locations() works.
 #if !defined(__APPLE__)
@@ -141,7 +141,7 @@ INSTANTIATE_TEST_SUITE_P(
         ClientOptions(ClickHouseExplorerConfig)
             .SetSSLOptions(ClientOptions::SSLOptions()
                      .SetUseDefaultCALocations(false)
-                     .SetSkipVerification(true)), // No CA loaded, but verfication is skipped
+                     .SetSkipVerification(true)), // No CA loaded, but verification is skipped
         {"SELECT 1;"}
     }
 ));