decimal improvements [CLICKHOUSE-3765]

dbms/src/Columns/ColumnDecimal.cpp

@@ -27,7 +27,7 @@ int ColumnDecimal<T>::compareAt(size_t n, size_t m, const IColumn & rhs_, int )
 {
     auto other = static_cast<const Self &>(rhs_);
     const T & a = data[n];
-    const T & b = static_cast<const Self &>(rhs_).data[m];
+    const T & b = other.data[m];
 
     return decimalLess<T>(b, a, other.scale, scale) ? 1 : (decimalLess<T>(a, b, scale, other.scale) ? -1 : 0);
 }
@@ -56,28 +56,36 @@ void ColumnDecimal<T>::updateHashWithValue(size_t n, SipHash & hash) const
 template <typename T>
 void ColumnDecimal<T>::getPermutation(bool reverse, size_t limit, int , IColumn::Permutation & res) const
 {
-    size_t s = data.size();
-    res.resize(s);
-    for (size_t i = 0; i < s; ++i)
-        res[i] = i;
-
-    if (limit >= s)
-        limit = 0;
-
-    if (limit)
+#if 1 /// TODO: perf test
+    if (data.size() <= std::numeric_limits<UInt32>::max())
     {
-        if (reverse)
-            std::partial_sort(res.begin(), res.begin() + limit, res.end(), [](T a, T b) { return a > b; });
-        else
-            std::partial_sort(res.begin(), res.begin() + limit, res.end(), [](T a, T b) { return a < b; });
-    }
-    else
-    {
-        if (reverse)
-            std::sort(res.begin(), res.end(), [](T a, T b) { return a > b; });
-        else
-            std::sort(res.begin(), res.end(), [](T a, T b) { return a < b; });
+        PaddedPODArray<UInt32> tmp_res;
+        permutation(reverse, limit, tmp_res);
+
+        res.resize(tmp_res.size());
+        for (size_t i = 0; i < tmp_res.size(); ++i)
+            res[i] = tmp_res[i];
+        return;
     }
+#endif
+
+    permutation(reverse, limit, res);
+}
+
+template <typename T>
+ColumnPtr ColumnDecimal<T>::permute(const IColumn::Permutation & perm, size_t limit) const
+{
+    size_t size = limit ? std::min(data.size(), limit) : data.size();
+    if (perm.size() < size)
+        throw Exception("Size of permutation is less than required.", ErrorCodes::SIZES_OF_COLUMNS_DOESNT_MATCH);
+
+    auto res = this->create(size, scale);
+    typename Self::Container & res_data = res->getData();
+
+    for (size_t i = 0; i < size; ++i)
+        res_data[i] = data[perm[i]];
+
+    return std::move(res);
 }
 
 template <typename T>
@@ -94,7 +102,10 @@ MutableColumnPtr ColumnDecimal<T>::cloneResized(size_t size) const
         memcpy(&new_col.data[0], &data[0], count * sizeof(data[0]));
 
         if (size > count)
-            memset(static_cast<void *>(&new_col.data[count]), static_cast<int>(value_type()), (size - count) * sizeof(value_type));
+        {
+            void * tail = &new_col.data[count];
+            memset(tail, 0, (size - count) * sizeof(T));
+        }
     }
 
     return std::move(res);
@@ -106,10 +117,8 @@ void ColumnDecimal<T>::insertRangeFrom(const IColumn & src, size_t start, size_t
     const ColumnDecimal & src_vec = static_cast<const ColumnDecimal &>(src);
 
     if (start + length > src_vec.data.size())
-        throw Exception("Parameters start = "
-            + toString(start) + ", length = "
-            + toString(length) + " are out of bound in ColumnVector<T>::insertRangeFrom method"
-            " (data.size() = " + toString(src_vec.data.size()) + ").",
+        throw Exception("Parameters start = " + toString(start) + ", length = " + toString(length) +
+            " are out of bound in ColumnDecimal<T>::insertRangeFrom method (data.size() = " + toString(src_vec.data.size()) + ").",
             ErrorCodes::PARAMETER_OUT_OF_BOUND);
 
     size_t old_size = data.size();
@@ -146,27 +155,6 @@ ColumnPtr ColumnDecimal<T>::filter(const IColumn::Filter & filt, ssize_t result_
     return std::move(res);
 }
 
-template <typename T>
-ColumnPtr ColumnDecimal<T>::permute(const IColumn::Permutation & perm, size_t limit) const
-{
-    size_t size = data.size();
-
-    if (limit == 0)
-        limit = size;
-    else
-        limit = std::min(size, limit);
-
-    if (perm.size() < limit)
-        throw Exception("Size of permutation is less than required.", ErrorCodes::SIZES_OF_COLUMNS_DOESNT_MATCH);
-
-    auto res = this->create(limit, scale);
-    typename Self::Container & res_data = res->getData();
-    for (size_t i = 0; i < limit; ++i)
-        res_data[i] = data[perm[i]];
-
-    return std::move(res);
-}
-
 template <typename T>
 ColumnPtr ColumnDecimal<T>::index(const IColumn & indexes, size_t limit) const
 {

dbms/src/Columns/ColumnDecimal.h

@@ -60,8 +60,7 @@ private:
     friend class COWPtrHelper<IColumn, Self>;
 
 public:
-    using value_type = T;
-    using Container = DecimalPaddedPODArray<value_type>;
+    using Container = DecimalPaddedPODArray<T>;
 
 private:
     ColumnDecimal(const size_t n, UInt32 scale_)
@@ -138,6 +137,24 @@ public:
 protected:
     Container data;
     UInt32 scale;
+
+    template <typename U>
+    void permutation(bool reverse, size_t limit, PaddedPODArray<U> & res) const
+    {
+        size_t s = data.size();
+        res.resize(s);
+        for (U i = 0; i < s; ++i)
+            res[i] = i;
+
+        std::function greater = [this](size_t a, size_t b) { return data[a] > data[b]; };
+        std::function less = [this](size_t a, size_t b) { return data[a] < data[b]; };
+
+        auto sort_end = res.end();
+        if (limit && limit < s)
+            sort_end = res.begin() + limit;
+
+        std::partial_sort(res.begin(), sort_end, res.end(), reverse ? greater : less);
+    }
 };
 
 template <typename T>

dbms/tests/queries/0_stateless/00700_decimal_arithm.sql

@@ -30,7 +30,7 @@ SELECT f + f, f - f, f * f, f / f FROM test.decimal WHERE f > 0; -- { serverErro
 SELECT g + g, g - g, g * g, g / g FROM test.decimal WHERE g > 0;
 SELECT h + h, h - h, h * h, h / h FROM test.decimal WHERE h > 0; -- { serverError 407 }
 SELECT h + h, h - h FROM test.decimal WHERE h > 0;
---SELECT i + i, i - i, i * i, i / i FROM test.decimal WHERE i > 0; -- { serverError 407 }
+SELECT i + i, i - i, i * i, i / i FROM test.decimal WHERE i > 0; -- { serverError 407 }
 SELECT i + i, i - i FROM test.decimal WHERE i > 0;
 SELECT j + j, j - j, j * j, j / j FROM test.decimal WHERE j > 0;
 
@@ -38,8 +38,7 @@ SELECT a + 21, a - 21, a - 84, a * 21, a * -21, a / 21, a / 84 FROM test.decimal
 SELECT b + 21, b - 21, b - 84, b * 21, b * -21, b / 21, b / 84 FROM test.decimal WHERE b = 42;
 SELECT c + 21, c - 21, c - 84, c * 21, c * -21, c / 21, c / 84 FROM test.decimal WHERE c = 42;
 SELECT e + 21, e - 21, e - 84, e * 21, e * -21, e / 21, e / 84 FROM test.decimal WHERE e > 0; -- { serverError 407 }
-SELECT 1 LIMIT 0;
---SELECT f + 21, f - 21, f - 84, f * 21, f * -21, f / 21, f / 84 FROM test.decimal WHERE f > 0; -- { serverError 407 }
+SELECT f + 21, f - 21, f - 84, f * 21, f * -21, f / 21, f / 84 FROM test.decimal WHERE f > 0; -- { serverError 407 }
 SELECT g + 21, g - 21, g - 84, g * 21, g * -21, g / 21, g / 84 FROM test.decimal WHERE g > 0;
 SELECT h + 21, h - 21, h - 84, h * 21, h * -21, h / 21, h / 84 FROM test.decimal WHERE h > 0;
 SELECT i + 21, i - 21, i - 84, i * 21, i * -21, i / 21, i / 84 FROM test.decimal WHERE i > 0;
@@ -49,8 +48,7 @@ SELECT 21 + a, 21 - a, 84 - a, 21 * a, -21 * a, 21 / a, 84 / a FROM test.decimal
 SELECT 21 + b, 21 - b, 84 - b, 21 * b, -21 * b, 21 / b, 84 / b FROM test.decimal WHERE b = 42;
 SELECT 21 + c, 21 - c, 84 - c, 21 * c, -21 * c, 21 / c, 84 / c FROM test.decimal WHERE c = 42;
 SELECT 21 + e, 21 - e, 84 - e, 21 * e, -21 * e, 21 / e, 84 / e FROM test.decimal WHERE e > 0; -- { serverError 407 }
-SELECT 1 LIMIT 0;
---SELECT 21 + f, 21 - f, 84 - f, 21 * f, -21 * f, 21 / f, 84 / f FROM test.decimal WHERE f > 0; -- { serverError 407 }
+SELECT 21 + f, 21 - f, 84 - f, 21 * f, -21 * f, 21 / f, 84 / f FROM test.decimal WHERE f > 0; -- { serverError 407 }
 SELECT 21 + g, 21 - g, 84 - g, 21 * g, -21 * g, 21 / g, 84 / g FROM test.decimal WHERE g > 0;
 SELECT 21 + h, 21 - h, 84 - h, 21 * h, -21 * h, 21 / h, 84 / h FROM test.decimal WHERE h > 0; -- { serverError 407 }
 SELECT 21 + h, 21 - h, 84 - h, 21 * h, -21 * h FROM test.decimal WHERE h > 0;

libs/libcommon/include/common/arithmeticOverflow.h

@@ -29,8 +29,10 @@ namespace common
     template <>
     inline bool addOverflow(__int128 x, __int128 y, __int128 & res)
     {
+        static constexpr __int128 min_int128 = __int128(0x8000000000000000ll) << 64;
+        static constexpr __int128 max_int128 = (__int128(0x7fffffffffffffffll) << 64) + 0xffffffffffffffffll;
         res = x + y;
-        return (res - y) != x;
+        return (y > 0 && x > max_int128 - y) || (y < 0 && x < min_int128 - y);
     }
 
     template <typename T>
@@ -60,8 +62,10 @@ namespace common
     template <>
     inline bool subOverflow(__int128 x, __int128 y, __int128 & res)
     {
+        static constexpr __int128 min_int128 = __int128(0x8000000000000000ll) << 64;
+        static constexpr __int128 max_int128 = (__int128(0x7fffffffffffffffll) << 64) + 0xffffffffffffffffll;
         res = x - y;
-        return (res + y) != x;
+        return (y < 0 && x > max_int128 + y) || (y > 0 && x < min_int128 + y);
     }
 
     template <typename T>
@@ -92,6 +96,11 @@ namespace common
     inline bool mulOverflow(__int128 x, __int128 y, __int128 & res)
     {
         res = x * y;
-        return (res / y) != x;
+        if (!x || !y)
+            return false;
+
+        unsigned __int128 a = (x > 0) ? x : -x;
+        unsigned __int128 b = (y > 0) ? y : -y;
+        return (a * b) / b != a;
     }
 }