adjust code

clickhouse/columns/string.cpp

@@ -119,6 +119,47 @@ ItemView ColumnFixedString::GetItem(size_t index) const {
     return ItemView{Type::FixedString, this->At(index)};
 }
 
+struct ColumnString::Block
+{
+    using CharT = typename std::string::value_type;
+
+    explicit Block(size_t starting_capacity)
+        : size(0),
+        capacity(starting_capacity),
+        data_(new CharT[capacity])
+    {}
+
+    inline auto GetAvailable() const
+    {
+        return capacity - size;
+    }
+
+    std::string_view AppendUnsafe(std::string_view str)
+    {
+        const auto pos = &data_[size];
+
+        memcpy(pos, str.data(), str.size());
+        size += str.size();
+
+        return std::string_view(pos, str.size());
+    }
+
+    auto GetCurrentWritePos()
+    {
+        return &data_[size];
+    }
+
+    std::string_view ConsumeTailAsStringViewUnsafe(size_t len)
+    {
+        const auto start = &data_[size];
+        size += len;
+        return std::string_view(start, len);
+    }
+
+    size_t size;
+    const size_t capacity;
+    std::unique_ptr<CharT[]> data_;
+};
 
 ColumnString::ColumnString()
     : Column(Type::CreateString())
@@ -152,6 +193,27 @@ ColumnString::ColumnString(std::vector<std::string>&& data)
 ColumnString::~ColumnString()
 {}
 
+void ColumnString::Append(const std::string_view& str) {
+    if (blocks_.size() == 0 || blocks_.back().GetAvailable() < str.length())
+    {
+        blocks_.emplace_back(std::max(DEFAULT_BLOCK_SIZE, str.size()));
+    }
+
+    items_.emplace_back(blocks_.back().AppendUnsafe(str));
+}
+
+void ColumnString::Append(std::string&& steal_value)
+{
+    append_data_.emplace_back(std::move(steal_value));
+    auto& last_data = append_data_.back();
+    items_.emplace_back(std::string_view{ last_data.data(),last_data.length() });
+}
+
+void ColumnString::AppendNoManagedLifetime(std::string_view str)
+{
+    items_.emplace_back(str);
+}
+
 void ColumnString::AppendUnsafe(std::string_view str)
 {
     items_.emplace_back(blocks_.back().AppendUnsafe(str));

clickhouse/columns/string.h

@@ -90,6 +90,16 @@ class ColumnString : public Column {
     ColumnString& operator=(const ColumnString&) = delete;
     ColumnString(const ColumnString&) = delete;
 
+    /// Appends one element to the column.
+    void Append(const std::string_view& str);
+
+    /// Appends one element to the column.
+    void Append(std::string&& steal_value);
+
+    /// Appends one element to the column.
+    /// If str lifetime is managed elsewhere and guaranteed to outlive the Block sent to the server
+    void AppendNoManagedLifetime(std::string_view str);
+
     /// Returns element at given row number.
     std::string_view At(size_t n) const;
 
@@ -122,78 +132,11 @@ class ColumnString : public Column {
     void AppendUnsafe(std::string_view);
 
 private:
-    struct Block {
-        using CharT = typename std::string::value_type;
-
-        explicit Block(size_t starting_capacity)
-            : size(0),
-            capacity(starting_capacity),
-            data_(new CharT[capacity])
-        {}
-
-        inline auto GetAvailable() const
-        {
-            return capacity - size;
-        }
-
-        std::string_view AppendUnsafe(std::string_view str)
-        {
-            const auto pos = &data_[size];
-
-            memcpy(pos, str.data(), str.size());
-            size += str.size();
-
-            return std::string_view(pos, str.size());
-        }
-
-        auto GetCurrentWritePos()
-        {
-            return &data_[size];
-        }
-
-        std::string_view ConsumeTailAsStringViewUnsafe(size_t len)
-        {
-            const auto start = &data_[size];
-            size += len;
-            return std::string_view(start, len);
-        }
-
-        size_t size;
-        const size_t capacity;
-        std::unique_ptr<CharT[]> data_;
-    };
+    struct Block;
 
     std::vector<std::string_view> items_;
     std::vector<Block> blocks_;
     std::deque<std::string> append_data_;
-
-public:
-    /// Appends one element to the column. Copy or move str
-    template<typename StringType>
-    void Append(StringType&& str) {
-        using str_type = decltype(str);
-        if (std::is_same_v<std::string, std::decay_t<str_type>> && std::is_rvalue_reference_v<str_type>) {
-            append_data_.emplace_back(std::move(str));
-            auto& last_data = append_data_.back();
-            items_.emplace_back(std::string_view{ last_data.data(),last_data.length() });
-        }
-        else if constexpr (std::is_convertible_v<std::decay_t<str_type>, std::string_view>) {
-            auto data_view = std::string_view(str);
-            if (blocks_.size() == 0 || blocks_.back().GetAvailable() < data_view.length()) {
-                blocks_.emplace_back(std::max(DEFAULT_BLOCK_SIZE, data_view.size()));
-            }
-            items_.emplace_back(blocks_.back().AppendUnsafe(data_view));
-        }
-        else {
-            static_assert(always_false_v<str_type>, "the StringType is not correct");
-        }
-    }
-
-    /// Appends one element to the column.
-    /// If str lifetime is managed elsewhere and guaranteed to outlive the Block sent to the server
-    void AppendNoManagedLifetime(std::string_view str) {
-        items_.emplace_back(str);
-    }
 };
 
 }

ut/columns_ut.cpp

@@ -113,14 +113,15 @@ TEST(ColumnsCase, StringInit) {
 
 TEST(ColumnsCase, StringAppend) {
     auto col = std::make_shared<ColumnString>();
-    std::string data = "ufiudhf3493fyiudferyer3yrifhdflkdjfeuroe";
+    const char* expected = "ufiudhf3493fyiudferyer3yrifhdflkdjfeuroe";
+    std::string data(expected);
     col->Append(data);
     col->Append(std::move(data));
     col->Append("11");
 
     ASSERT_EQ(col->Size(), 3u);
-    ASSERT_EQ(col->At(0), "ufiudhf3493fyiudferyer3yrifhdflkdjfeuroe");
-    ASSERT_EQ(col->At(1), "ufiudhf3493fyiudferyer3yrifhdflkdjfeuroe");
+    ASSERT_EQ(col->At(0), expected);
+    ASSERT_EQ(col->At(1), expected);
     ASSERT_EQ(col->At(2), "11");
 }