Merge pull request #345 from 1261385937/master

ClickHouse · Nov 16, 2023 · 901f609 · 901f609
2 parents 7217e32 + af908f2
commit 901f609
Show file tree

Hide file tree

Showing 3 changed files with 99 additions and 5 deletions.
diff --git a/clickhouse/columns/array.h b/clickhouse/columns/array.h
@@ -260,8 +260,16 @@ class ColumnArrayT : public ColumnArray {
     using ColumnArray::Append;
 
     template <typename Container>
-    inline void Append(const Container& container) {
-        Append(std::begin(container), std::end(container));
+    inline void Append(Container&& container) {
+        using container_type = decltype(container);
+        if constexpr (std::is_lvalue_reference_v<container_type> || 
+            std::is_const_v<std::remove_reference_t<container_type>>) {
+            Append(std::begin(container), std::end(container));
+        }
+        else {
+            Append(std::make_move_iterator(std::begin(container)),
+                std::make_move_iterator(std::end(container)));
+        }
     }
 
     template <typename ValueType>
@@ -270,12 +278,12 @@ class ColumnArrayT : public ColumnArray {
     }
 
     template <typename Begin, typename End>
-    inline void Append(Begin begin, const End & end) {
+    inline void Append(Begin begin, End end) {
         auto & nested_data = *typed_nested_data_;
         size_t counter = 0;
 
         while (begin != end) {
-            nested_data.Append(std::move(*begin));
+            nested_data.Append(*begin);
             ++begin;
             ++counter;
         }

diff --git a/clickhouse/columns/string.cpp b/clickhouse/columns/string.cpp
@@ -232,7 +232,6 @@ void ColumnString::Clear() {
     items_.clear();
     blocks_.clear();
     append_data_.clear();
-    append_data_.shrink_to_fit();
 }
 
 std::string_view ColumnString::At(size_t n) const {

diff --git a/ut/column_array_ut.cpp b/ut/column_array_ut.cpp
@@ -313,3 +313,90 @@ TEST(ColumnArrayT, Wrap_UInt64_2D) {
 
     EXPECT_TRUE(CompareRecursive(values, array));
 }
+
+TEST(ColumnArrayT, left_value_no_move) {
+    std::string value0 = "000000000000000000";
+    std::string value1 = "111111111111111111";
+    std::string value2 = "222222222222222222";
+    std::vector<std::vector<std::string>> all_values{ 
+        { value0, value1, value2},
+        { value0, value1, value2},
+        { value0, value1, value2}
+    };
+    size_t origin_size = 3;
+    auto array = std::make_shared<clickhouse::ColumnArrayT<clickhouse::ColumnArrayT<ColumnString>>>();
+    array->Append(all_values);
+    EXPECT_EQ(3u, (*array)[0][0].size());
+    EXPECT_EQ(3u, (*array)[0][1].size());
+    EXPECT_EQ(3u, (*array)[0][2].size());
+
+    EXPECT_EQ(value0, (*array)[0][0][0]);
+    EXPECT_EQ(value1, (*array)[0][1][1]);
+    EXPECT_EQ(value2, (*array)[0][2][2]);
+
+    for (const auto& values : all_values) {
+        EXPECT_EQ(origin_size, values.size());
+    }
+
+    EXPECT_EQ(origin_size, all_values.size());
+    for (const auto& values : all_values) {
+        EXPECT_EQ(values[0], value0);
+        EXPECT_EQ(values[1], value1);
+        EXPECT_EQ(values[2], value2);
+    }
+}
+
+TEST(ColumnArrayT, right_value_move) {
+    std::string value0 = "000000000000000000";
+    std::string value1 = "111111111111111111";
+    std::string value2 = "222222222222222222";
+    std::vector<std::vector<std::string>> all_values{
+        { value0, value1, value2},
+        { value0, value1, value2},
+        { value0, value1, value2}
+    };
+    auto array = std::make_shared<clickhouse::ColumnArrayT<clickhouse::ColumnArrayT<ColumnString>>>();
+    array->Append(std::move(all_values));
+    EXPECT_EQ(3u, (*array)[0][0].size());
+    EXPECT_EQ(3u, (*array)[0][1].size());
+    EXPECT_EQ(3u, (*array)[0][2].size());
+
+    EXPECT_EQ(value0, (*array)[0][0][0]);
+    EXPECT_EQ(value1, (*array)[0][1][1]);
+    EXPECT_EQ(value2, (*array)[0][2][2]);
+
+    // Here we don't care about the size of the container from which all values were moved-out.
+    //EXPECT_EQ(0u, all_values.size());
+}
+
+TEST(ColumnArrayT, const_right_value_no_move) {
+    std::string value0 = "000000000000000000";
+    std::string value1 = "111111111111111111";
+    std::string value2 = "222222222222222222";
+    const std::vector<std::vector<std::string>> all_values{
+        { value0, value1, value2},
+        { value0, value1, value2},
+        { value0, value1, value2}
+    };
+    size_t origin_size = 3;
+    auto array = std::make_shared<clickhouse::ColumnArrayT<clickhouse::ColumnArrayT<ColumnString>>>();
+    array->Append(std::move(all_values));
+    EXPECT_EQ(3u, (*array)[0][0].size());
+    EXPECT_EQ(3u, (*array)[0][1].size());
+    EXPECT_EQ(3u, (*array)[0][2].size());
+
+    EXPECT_EQ(value0, (*array)[0][0][0]);
+    EXPECT_EQ(value1, (*array)[0][1][1]);
+    EXPECT_EQ(value2, (*array)[0][2][2]);
+
+    for (const auto& values : all_values) {
+        EXPECT_EQ(origin_size, values.size());
+    }
+
+    EXPECT_EQ(origin_size, all_values.size());
+    for (const auto& values : all_values) {
+        EXPECT_EQ(values[0], value0);
+        EXPECT_EQ(values[1], value1);
+        EXPECT_EQ(values[2], value2);
+    }
+}