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;
         }
     }