Merge pull request #190 from Enmk/DateTime32_tests_and_fixes

Fixed Date32 range, now it supports pre-epoch values

Author: Enmk

clickhouse/columns/date.cpp

@@ -65,13 +65,13 @@ ItemView ColumnDate::GetItem(size_t index) const {
 
 ColumnDate32::ColumnDate32()
     : Column(Type::CreateDate32())
-    , data_(std::make_shared<ColumnUInt32>())
+    , data_(std::make_shared<ColumnInt32>())
 {
 }
 
 void ColumnDate32::Append(const std::time_t& value) {
     /// TODO: This code is fundamentally wrong.
-    data_->Append(static_cast<uint16_t>(value / std::time_t(86400)));
+    data_->Append(static_cast<int32_t>(value / std::time_t(86400)));
 }
 
 void ColumnDate32::Clear() {
@@ -101,7 +101,7 @@ size_t ColumnDate32::Size() const {
 }
 
 ColumnRef ColumnDate32::Slice(size_t begin, size_t len) const {
-    auto col = data_->Slice(begin, len)->As<ColumnUInt32>();
+    auto col = data_->Slice(begin, len)->As<ColumnInt32>();
     auto result = std::make_shared<ColumnDate32>();
 
     result->data_->Append(col);
@@ -119,7 +119,7 @@ void ColumnDate32::Swap(Column& other) {
 }
 
 ItemView ColumnDate32::GetItem(size_t index) const {
-    return data_->GetItem(index);
+    return ItemView{Type()->GetCode(), data_->GetItem(index)};
 }
 
 

clickhouse/columns/date.h

@@ -86,7 +86,7 @@ class ColumnDate32 : public Column {
     ItemView GetItem(size_t index) const override;
 
 private:
-    std::shared_ptr<ColumnUInt32> data_;
+    std::shared_ptr<ColumnInt32> data_;
 };
 
 

clickhouse/columns/itemview.cpp

@@ -56,6 +56,7 @@ void ItemView::ValidateData(Type::Code type, DataType data) {
         case Type::Code::UInt32:
         case Type::Code::Float32:
         case Type::Code::DateTime:
+        case Type::Code::Date32:
         case Type::Code::IPv4:
         case Type::Code::Decimal32:
             return AssertSize({4});

clickhouse/columns/itemview.h

@@ -69,7 +69,8 @@ struct ItemView {
             if (sizeof(ValueType) == data.size()) {
                 return *reinterpret_cast<const T*>(data.data());
             } else {
-                throw AssertionError("Incompatitable value type and size.");
+                throw AssertionError("Incompatitable value type and size. Requested size: "
+                        + std::to_string(sizeof(ValueType)) + " stored size: " + std::to_string(data.size()));
             }
         }
     }

ut/Column_ut.cpp

@@ -22,6 +22,14 @@ namespace {
 using namespace clickhouse;
 }
 
+namespace clickhouse{
+
+std::ostream& operator<<(std::ostream& ostr, const Type::Code& type_code) {
+    return ostr << Type::TypeName(type_code) << " (" << static_cast<int>(type_code) << ")";
+}
+
+}
+
 
 // Generic tests for a Column subclass against basic API:
 // 1. Constructor: Create, ensure that it is empty
@@ -60,6 +68,8 @@ class GenericColumnTest : public testing::Test {
             return GenerateVector(values_size, FromVectorGenerator{MakeDateTimes()});
         } else if constexpr (std::is_same_v<ColumnType, ColumnDateTime64>) {
             return MakeDateTime64s(3u, values_size);
+        } else if constexpr (std::is_same_v<ColumnType, ColumnDate32>) {
+            return GenerateVector(values_size, FromVectorGenerator{MakeDates32()});
         } else if constexpr (std::is_same_v<ColumnType, ColumnIPv4>) {
             return GenerateVector(values_size, FromVectorGenerator{MakeIPv4s()});
         } else if constexpr (std::is_same_v<ColumnType, ColumnIPv6>) {
@@ -100,7 +110,7 @@ using ValueColumns = ::testing::Types<
     , ColumnInt8, ColumnInt16, ColumnInt32, ColumnInt64
     , ColumnFloat32, ColumnFloat64
     , ColumnString, ColumnFixedString
-    , ColumnDate, ColumnDateTime, ColumnDateTime64
+    , ColumnDate, ColumnDateTime, ColumnDateTime64, ColumnDate32
     , ColumnIPv4, ColumnIPv6
     , ColumnInt128
     , ColumnDecimal
@@ -168,6 +178,8 @@ inline auto convertValueForGetItem(const ColumnType& col, ValueType&& t) {
         return std::string_view(reinterpret_cast<const char*>(t.s6_addr), 16);
     } else if constexpr (std::is_same_v<ColumnType, ColumnDate>) {
         return static_cast<uint16_t>(t / std::time_t(86400));
+    } else if constexpr (std::is_same_v<ColumnType, ColumnDate32>) {
+        return static_cast<uint32_t>(t / std::time_t(86400));
     } else if constexpr (std::is_same_v<ColumnType, ColumnDateTime>) {
         return static_cast<uint32_t>(t);
     } else {

ut/value_generators.cpp

@@ -65,6 +65,23 @@ std::vector<clickhouse::Int64> MakeDates() {
     return result;
 }
 
+std::vector<clickhouse::Int64> MakeDates32() {
+    // in CH Date32 internally a UInt32 and stores a day number
+    // ColumnDate expects values to be seconds, which is then
+    // converted to day number internally, hence the `* 86400`.
+    // 114634 * 86400 is 2282-11-10, last integer that fits into DateTime32 range
+    // (max is 2283-11-11)
+    std::vector<clickhouse::Int64> result  = MakeDates();
+
+    // add corresponding negative values, since pre-epoch date are supported too.
+    const auto size = result.size();
+    for (size_t i = 0; i < size; ++i) {
+        result.push_back(result[i] * -1);
+    }
+
+    return result;
+}
+
 std::vector<clickhouse::Int64> MakeDateTimes() {
     // in CH DateTime internally a UInt32
     return {

ut/value_generators.h

@@ -33,6 +33,7 @@ std::vector<std::string> MakeFixedStrings(size_t string_size);
 std::vector<std::string> MakeStrings();
 std::vector<clickhouse::Int64> MakeDateTime64s(size_t scale, size_t values_size = 200);
 std::vector<clickhouse::Int64> MakeDates();
+std::vector<clickhouse::Int64> MakeDates32();
 std::vector<clickhouse::Int64> MakeDateTimes();
 std::vector<in_addr> MakeIPv4s();
 std::vector<in6_addr> MakeIPv6s();