Minor fixes + more tests

Author: Enmk

clickhouse/columns/decimal.cpp

@@ -1,7 +1,5 @@
 #include "decimal.h"
 
-#include <cassert>
-
 namespace clickhouse {
 
 ColumnDecimal::ColumnDecimal(size_t precision, size_t scale)
@@ -16,8 +14,9 @@ ColumnDecimal::ColumnDecimal(size_t precision, size_t scale)
     }
 }
 
-ColumnDecimal::ColumnDecimal(TypeRef type)
-    : Column(type)
+ColumnDecimal::ColumnDecimal(TypeRef type, ColumnRef data)
+    : Column(type),
+      data_(data)
 {
 }
 
@@ -91,7 +90,7 @@ Int128 ColumnDecimal::At(size_t i) const {
         case Type::Int128:
             return data_->As<ColumnInt128>()->At(i);
         default:
-            assert(false && "Invalid data_ column type in ColumnDecimal");
+            throw std::runtime_error("Invalid data_ column type in ColumnDecimal");
             return 0;
     }
 }
@@ -119,9 +118,8 @@ size_t ColumnDecimal::Size() const {
 }
 
 ColumnRef ColumnDecimal::Slice(size_t begin, size_t len) {
-    std::shared_ptr<ColumnDecimal> slice(new ColumnDecimal(type_));
-    slice->data_ = data_->Slice(begin, len);
-    return slice;
+    // coundn't use std::make_shared since this c-tor is private
+    return ColumnRef{new ColumnDecimal(type_, data_->Slice(begin, len))};
 }
 
 void ColumnDecimal::Swap(Column& other) {

clickhouse/columns/decimal.h

@@ -37,7 +37,7 @@ class ColumnDecimal : public Column {
     ///  - ColumnInt128
     ColumnRef data_;
 
-    explicit ColumnDecimal(TypeRef type); // for `Slice(…)`
+    explicit ColumnDecimal(TypeRef type, ColumnRef data);
 };
 
 }

clickhouse/columns/numeric.cpp

@@ -10,9 +10,16 @@ ColumnVector<T>::ColumnVector()
 }
 
 template <typename T>
-ColumnVector<T>::ColumnVector(std::vector<T> data)
+ColumnVector<T>::ColumnVector(const std::vector<T> & data)
     : Column(Type::CreateSimple<T>())
-    , data_(std::move(data))
+    , data_(data)
+{
+}
+
+template <typename T>
+ColumnVector<T>::ColumnVector(std::vector<T> && data)
+    : Column(Type::CreateSimple<T>())
+    , data_(data)
 {
 }
 

clickhouse/columns/numeric.h

@@ -14,7 +14,8 @@ class ColumnVector : public Column {
 
     ColumnVector();
 
-    explicit ColumnVector(std::vector<T> data);
+    explicit ColumnVector(const std::vector<T>& data);
+    explicit ColumnVector(std::vector<T> && data);
 
     /// Appends one element to the end of column.
     void Append(const T& value);

clickhouse/types/type_parser.cpp

@@ -9,7 +9,6 @@
 namespace clickhouse {
 
 bool TypeAst::operator==(const TypeAst & other) const {
-    // TODO: operator == must be a friend function.
     return meta == other.meta
         && code == other.code
         && name == other.name

ut/client_ut.cpp

@@ -769,7 +769,6 @@ TEST_P(ClientCase, DateTime64) {
 
             ASSERT_EQ(1U, block.GetColumnCount());
             if (auto col = block[0]->As<ColumnDateTime64>()) {
-                ASSERT_EQ(data.size(), col->Size());
                 for (size_t i = 0; i < col->Size(); ++i) {
                     EXPECT_EQ(data[i], col->At(i)) << " at index: " << i;
                 }

ut/columns_ut.cpp

@@ -224,15 +224,15 @@ TEST(ColumnsCase, DateTime64_Swap) {
     }
 
     auto column2 = std::make_shared<ColumnDateTime64>(6ul);
+    const auto single_dt64_value = 1'234'567'890'123'456'789ll;
+    column2->Append(single_dt64_value);
     column->Swap(*column2);
 
     // Validate that all items were transferred to column2.
-    ASSERT_EQ(0u, column->Size());
-    ASSERT_EQ(data.size(), column2->Size());
-
-    // Clearing doesn't affects precision value.
-    ASSERT_EQ(6ul, column->GetPrecision());
+    ASSERT_EQ(1u, column->Size());
+    EXPECT_EQ(single_dt64_value, column->At(0));
 
+    ASSERT_EQ(data.size(), column2->Size());
     for (size_t i = 0; i < data.size(); ++i) {
         ASSERT_EQ(data[i], column2->At(i));
     }
@@ -289,6 +289,23 @@ TEST(ColumnsCase, DateTime64_Slice) {
     }
 }
 
+TEST(ColumnsCase, DateTime64_Slice_OUTOFBAND) {
+    // Slice() shouldn't throw exceptions on invalid parameters, just clamp values to the nearest bounds.
+
+    auto column = std::make_shared<ColumnDateTime64>(6ul);
+
+    // Non-Empty slice on empty column
+    EXPECT_EQ(0u, column->Slice(0, 10)->Size());
+
+    const auto data = MakeDateTime64s();
+    for (const auto & v : data) {
+        column->Append(v);
+    }
+
+    EXPECT_EQ(column->Slice(0, data.size() + 1)->Size(), data.size());
+    EXPECT_EQ(column->Slice(data.size() + 1, 1)->Size(), 0u);
+}
+
 TEST(ColumnsCase, DateTime64_Swap_EXCEPTION) {
     auto column1 = std::make_shared<ColumnDateTime64>(6ul);
     auto column2 = std::make_shared<ColumnDateTime64>(0ul);