Merge pull request #81 from Enmk/artpaul_119

traceon · web-flow · commit 63288ebb5089 · 2021-03-24T18:08:57.000+04:00
Fixes #66 to allow using absl::int128 as Int128
diff --git a/clickhouse/columns/date.cpp b/clickhouse/columns/date.cpp
@@ -134,7 +134,8 @@ void ColumnDateTime64::Append(const Int64& value) {
 //}
 
 Int64 ColumnDateTime64::At(size_t n) const {
-    return data_->At(n);
+    // make sure to use Absl's Int128 conversion
+    return static_cast<Int64>(data_->At(n));
 }
 
 void ColumnDateTime64::Append(ColumnRef column) {
diff --git a/clickhouse/columns/decimal.cpp b/clickhouse/columns/decimal.cpp
@@ -1,5 +1,102 @@
 #include "decimal.h"
 
+namespace
+{
+using namespace clickhouse;
+
+#ifdef ABSL_HAVE_INTRINSIC_INT128
+template <typename T>
+inline bool addOverflow(const Int128 & l, const T & r, Int128 * result)
+{
+    __int128 res;
+    const auto ret_value = __builtin_add_overflow(static_cast<__int128>(l), static_cast<__int128>(r), &res);
+
+    *result = res;
+    return ret_value;
+}
+
+template <typename T>
+inline bool mulOverflow(const Int128 & l, const T & r, Int128 * result)
+{
+    __int128 res;
+    const auto ret_value = __builtin_mul_overflow(static_cast<__int128>(l), static_cast<__int128>(r), &res);
+
+    *result = res;
+    return ret_value;
+}
+
+#else
+template <typename T>
+inline bool getSignBit(const T & v)
+{
+    return std::signbit(v);
+}
+
+inline bool getSignBit(const Int128 & v)
+{
+//    static constexpr Int128 zero {};
+//    return v < zero;
+
+    // Sign of the whole absl::int128 value is determined by sign of higher 64 bits.
+    return absl::Int128High64(v) < 0;
+}
+
+inline bool addOverflow(const Int128 & l, const Int128 & r, Int128 * result)
+{
+    //    *result = l + r;
+    //    const auto result_sign = getSignBit(*result);
+    //    return l_sign == r_sign && l_sign != result_sign;
+
+    // Based on code from:
+    // https://wiki.sei.cmu.edu/confluence/display/c/INT32-C.+Ensure+that+operations+on+signed+integers+do+not+result+in+overflow#INT32C.Ensurethatoperationsonsignedintegersdonotresultinoverflow-CompliantSolution
+    const auto r_positive = !getSignBit(r);
+
+    if ((r_positive && (l > (std::numeric_limits<Int128>::max() - r))) ||
+        (!r_positive && (l < (std::numeric_limits<Int128>::min() - r)))) {
+        return true;
+    }
+    *result = l + r;
+
+    return false;
+}
+
+template <typename T>
+inline bool mulOverflow(const Int128 & l, const T & r, Int128 * result)
+{
+    // Based on code from:
+    // https://wiki.sei.cmu.edu/confluence/display/c/INT32-C.+Ensure+that+operations+on+signed+integers+do+not+result+in+overflow#INT32C.Ensurethatoperationsonsignedintegersdonotresultinoverflow-CompliantSolution.3
+    const auto l_positive = !getSignBit(l);
+    const auto r_positive = !getSignBit(r);
+
+    if (l_positive) {
+        if (r_positive) {
+            if (r != 0 && l > (std::numeric_limits<Int128>::max() / r)) {
+                return true;
+            }
+        } else {
+            if (l != 0 && r < (std::numeric_limits<Int128>::min() / l)) {
+                return true;
+            }
+        }
+    } else {
+        if (r_positive) {
+            if (r != 0 && l < (std::numeric_limits<Int128>::min() / r)) {
+                return true;
+            }
+        } else {
+            if (l != 0 && (r < (std::numeric_limits<Int128>::max() / l))) {
+                return true;
+            }
+        }
+    }
+
+    *result = l * r;
+    return false;
+}
+#endif
+
+}
+
 namespace clickhouse {
 
 ColumnDecimal::ColumnDecimal(size_t precision, size_t scale)
@@ -57,8 +154,8 @@ void ColumnDecimal::Append(const std::string& value) {
 
             has_dot = true;
         } else if (*c >= '0' && *c <= '9') {
-            if (__builtin_mul_overflow(int_value, 10, &int_value) ||
-                __builtin_add_overflow(int_value, *c - '0', &int_value)) {
+            if (mulOverflow(int_value, 10, &int_value) ||
+                addOverflow(int_value, *c - '0', &int_value)) {
                 throw std::runtime_error("value is too big for 128-bit integer");
             }
         } else {
@@ -72,7 +169,7 @@ void ColumnDecimal::Append(const std::string& value) {
     }
 
     while (zeros) {
-        if (__builtin_mul_overflow(int_value, 10, &int_value)) {
+        if (mulOverflow(int_value, 10, &int_value)) {
             throw std::runtime_error("value is too big for 128-bit integer");
         }
         --zeros;
diff --git a/clickhouse/columns/itemview.cpp b/clickhouse/columns/itemview.cpp
@@ -3,52 +3,69 @@
 namespace clickhouse {
 
 void ItemView::ValidateData(Type::Code type, DataType data) {
-    static const int ANY = -1; // value can be of any size
-    static const int ERR = -2; // value is not allowed inside ItemView
-    static const int value_size_by_type[] = {
-        0,   /*Void*/
-        1,   /*Int8*/
-        2,   /*Int16*/
-        4,   /*Int32*/
-        8,   /*Int64*/
-        1,   /*UInt8*/
-        2,   /*UInt16*/
-        4,   /*UInt32*/
-        8,   /*UInt64*/
-        4,   /*Float32*/
-        8,   /*Float64*/
-        ANY, /*String*/
-        ANY, /*FixedString*/
-        4,   /*DateTime*/
-        8,   /*DateTime64*/
-        2,   /*Date*/
-        ERR, /*Array*/
-        ERR, /*Nullable*/
-        ERR, /*Tuple*/
-        1,   /*Enum8*/
-        2,   /*Enum16*/
-        16,  /*UUID*/
-        4,   /*IPv4*/
-        8,   /*IPv6*/
-        16,  /*Int128*/
-        16,  /*Decimal*/
-        4,   /*Decimal32*/
-        8,   /*Decimal64*/
-        16,  /*Decimal128*/
-        ERR, /*LowCardinality*/
-    };
-
-    if (type >= sizeof(value_size_by_type)/sizeof(value_size_by_type[0]) || type < 0) {
-        throw std::runtime_error("Unknon type code:" + std::to_string(static_cast<int>(type)));
-    } else {
-        const auto expected_size = value_size_by_type[type];
-        if (expected_size == ERR) {
+    int expected_size = 0;
+    switch (type) {
+        case Type::Code::Void:
+            expected_size = 0;
+            break;
+
+        case Type::Code::Int8:
+        case Type::Code::UInt8:
+        case Type::Code::Enum8:
+            expected_size = 1;
+            break;
+
+        case Type::Code::Int16:
+        case Type::Code::UInt16:
+        case Type::Code::Date:
+        case Type::Code::Enum16:
+            expected_size = 2;
+            break;
+
+        case Type::Code::Int32:
+        case Type::Code::UInt32:
+        case Type::Code::Float32:
+        case Type::Code::DateTime:
+        case Type::Code::IPv4:
+        case Type::Code::Decimal32:
+            expected_size = 4;
+            break;
+
+        case Type::Code::Int64:
+        case Type::Code::UInt64:
+        case Type::Code::Float64:
+        case Type::Code::DateTime64:
+        case Type::Code::IPv6:
+        case Type::Code::Decimal64:
+            expected_size = 8;
+            break;
+
+        case Type::Code::String:
+        case Type::Code::FixedString:
+            // value can be of any size
+            return;
+
+        case Type::Code::Array:
+        case Type::Code::Nullable:
+        case Type::Code::Tuple:
+        case Type::Code::LowCardinality:
             throw std::runtime_error("Unsupported type in ItemView: " + std::to_string(static_cast<int>(type)));
-        } else if (expected_size != ANY && expected_size != static_cast<int>(data.size())) {
-            throw std::runtime_error("Value size mismatch for type "
-                    + std::to_string(static_cast<int>(type)) + " expected: "
-                    + std::to_string(expected_size) + ", got: " + std::to_string(data.size()));
-        }
+
+        case Type::Code::UUID:
+        case Type::Code::Int128:
+        case Type::Code::Decimal:
+        case Type::Code::Decimal128:
+            expected_size = 16;
+            break;
+
+        default:
+            throw std::runtime_error("Unknon type code:" + std::to_string(static_cast<int>(type)));
+    }
+
+    if (expected_size != static_cast<int>(data.size())) {
+        throw std::runtime_error("Value size mismatch for type "
+                + std::to_string(static_cast<int>(type)) + " expected: "
+                + std::to_string(expected_size) + ", got: " + std::to_string(data.size()));
     }
 }
 
diff --git a/clickhouse/columns/itemview.h b/clickhouse/columns/itemview.h
@@ -26,10 +26,10 @@ struct ItemView {
     inline auto ConvertToStorageValue(const T& t) {
         if constexpr (std::is_same_v<std::string_view, T> || std::is_same_v<std::string, T>) {
             return std::string_view{t};
-        } else if constexpr (std::is_fundamental_v<T>) {
+        } else if constexpr (std::is_fundamental_v<T> || std::is_same_v<Int128, std::decay_t<T>>) {
             return std::string_view{reinterpret_cast<const char*>(&t), sizeof(T)};
         } else {
-            // will caue error at compile-time
+            static_assert(!std::is_same_v<T, T>, "Unknown type, which can't be stored in ItemView");
             return;
         }
     }
@@ -55,7 +55,7 @@ struct ItemView {
     T get() const {
         if constexpr (std::is_same_v<std::string_view, T> || std::is_same_v<std::string, T>) {
             return data;
-        } else if constexpr (std::is_fundamental_v<T>) {
+        } else if constexpr (std::is_fundamental_v<T> || std::is_same_v<Int128, T>) {
             if (sizeof(T) == data.size()) {
                 return *reinterpret_cast<const T*>(data.data());
             } else {
diff --git a/clickhouse/types/types.h b/clickhouse/types/types.h
@@ -10,6 +10,9 @@
 
 namespace clickhouse {
 
+using Int128 = absl::int128;
+using Int64 = int64_t;
+
 using TypeRef = std::shared_ptr<class Type>;
 
 class Type {
diff --git a/ut/CMakeLists.txt b/ut/CMakeLists.txt
@@ -3,6 +3,7 @@ ADD_EXECUTABLE (clickhouse-cpp-ut
 
     client_ut.cpp
     columns_ut.cpp
+    itemview_ut.cpp
     socket_ut.cpp
     stream_ut.cpp
     type_parser_ut.cpp
diff --git a/ut/columns_ut.cpp b/ut/columns_ut.cpp
@@ -403,6 +403,62 @@ TEST(ColumnsCase, UUIDSlice) {
     ASSERT_EQ(sub->At(1), UInt128(0x3507213c178649f9llu, 0x9faf035d662f60aellu));
 }
 
+TEST(ColumnsCase, Int128) {
+    auto col = std::make_shared<ColumnInt128>(std::vector<Int128>{
+            absl::MakeInt128(0xffffffffffffffffll, 0xffffffffffffffffll), // -1
+            absl::MakeInt128(0, 0xffffffffffffffffll),  // 2^64
+            absl::MakeInt128(0xffffffffffffffffll, 0),
+            absl::MakeInt128(0x8000000000000000ll, 0),
+            Int128(0)
+    });
+    EXPECT_EQ(-1, col->At(0));
+    EXPECT_EQ(0xffffffffffffffffll, col->At(1));
+    EXPECT_EQ(0, col->At(4));
+}
+
+TEST(ColumnsCase, ColumnDecimal128_from_string) {
+    auto col = std::make_shared<ColumnDecimal>(38, 0);
+
+    const auto values = {
+        Int128(0),
+        Int128(-1),
+        Int128(1),
+        std::numeric_limits<Int128>::min() + 1,
+        std::numeric_limits<Int128>::max(),
+    };
+
+    for (size_t i = 0; i < values.size(); ++i)
+    {
+        const auto value = values.begin()[i];
+        SCOPED_TRACE(::testing::Message() << "# index: " << i << " Int128 value: " << value);
+
+        {
+            std::stringstream sstr;
+            sstr << value;
+            const auto string_value = sstr.str();
+
+            EXPECT_NO_THROW(col->Append(string_value));
+        }
+
+        ASSERT_EQ(i + 1, col->Size());
+        EXPECT_EQ(value, col->At(i));
+    }
+}
+
+TEST(ColumnsCase, ColumnDecimal128_from_string_overflow) {
+    auto col = std::make_shared<ColumnDecimal>(38, 0);
+
+    // 2^128 overflows
+    EXPECT_ANY_THROW(col->Append("340282366920938463463374607431768211456"));
+    // special case for number bigger than 2^128, ending in zeroes.
+    EXPECT_ANY_THROW(col->Append("400000000000000000000000000000000000000"));
+
+#ifndef ABSL_HAVE_INTRINSIC_INT128
+    // unfortunatelly std::numeric_limits<Int128>::min() overflows when there is no __int128 intrinsic type.
+    EXPECT_ANY_THROW(col->Append("-170141183460469231731687303715884105728"));
+#endif
+}
+
 TEST(ColumnsCase, ColumnLowCardinalityString_Append_and_Read) {
     const size_t items_count = 11;
     ColumnLowCardinalityT<ColumnString> col;
diff --git a/ut/itemview_ut.cpp b/ut/itemview_ut.cpp

Original file line number	Diff line number	Diff line change
`@@ -134,7 +134,8 @@ void ColumnDateTime64::Append(const Int64& value) {`
`134`	`134`	`//}`
`135`	`135`
`136`	`136`	`Int64 ColumnDateTime64::At(size_t n) const {`
`137`		`- return data_->At(n);`
	`137`	`+ // make sure to use Absl's Int128 conversion`
	`138`	`+ return static_cast<Int64>(data_->At(n));`
`138`	`139`	`}`
`139`	`140`
`140`	`141`	`void ColumnDateTime64::Append(ColumnRef column) {`