From 5b9689ce553990517ab90aa23cb460980cb45ce2 Mon Sep 17 00:00:00 2001 From: Roman Nozdrin Date: Wed, 30 Dec 2020 11:35:51 +0000 Subject: [PATCH] MCOL-4478 MCS now rounds the last digits of an avg() result for wide-DECIMAL argument --- datatypes/mcs_decimal.cpp | 21 +-------------------- datatypes/mcs_decimal.h | 17 +++++++++++++++++ datatypes/mcs_int128.h | 5 +++++ utils/rowgroup/rowaggregation.cpp | 20 +++++++++++++++++--- 4 files changed, 40 insertions(+), 23 deletions(-) diff --git a/datatypes/mcs_decimal.cpp b/datatypes/mcs_decimal.cpp index 6df2061fa..29bc9cda2 100644 --- a/datatypes/mcs_decimal.cpp +++ b/datatypes/mcs_decimal.cpp @@ -22,24 +22,6 @@ namespace datatypes { - - struct lldiv_t_128 - { - int128_t quot; - int128_t rem; - lldiv_t_128() : quot(0), rem(0) {} - lldiv_t_128(const int128_t& a_quot, const int128_t& a_rem) - : quot(a_quot), rem(a_rem) {} - }; - - inline lldiv_t_128 lldiv128(const int128_t& dividend, const int128_t& divisor) - { - if (UNLIKELY(divisor == 0) || UNLIKELY(dividend == 0)) - return lldiv_t_128(); - - return lldiv_t_128(dividend / divisor, dividend % divisor); - } - template @@ -115,7 +97,6 @@ namespace datatypes ? r.s128Value : r.value; opOverflowCheck(lValue, rValue); - if (result.scale >= l.scale - r.scale) { int128_t scaleMultiplier; @@ -213,7 +194,7 @@ namespace datatypes // rem carries the value's sign, but needs to be normalized. int64_t s = l.scale - r.scale; int128_t divisor; - getScaleDivisor(divisor, abs(s)); + getScaleDivisor(divisor, std::abs(s)); if (s < 0) { diff --git a/datatypes/mcs_decimal.h b/datatypes/mcs_decimal.h index 712bec55e..2d49e667a 100644 --- a/datatypes/mcs_decimal.h +++ b/datatypes/mcs_decimal.h @@ -161,6 +161,23 @@ inline void getScaleDivisor(T& divisor, const int8_t scale) } } +struct lldiv_t_128 +{ + int128_t quot; + int128_t rem; + lldiv_t_128() : quot(0), rem(0) {} + lldiv_t_128(const int128_t& a_quot, const int128_t& a_rem) + : quot(a_quot), rem(a_rem) {} +}; + +inline lldiv_t_128 lldiv128(const int128_t& dividend, const int128_t& divisor) +{ + if (UNLIKELY(divisor == 0) || UNLIKELY(dividend == 0)) + return lldiv_t_128(); + + return lldiv_t_128(dividend / divisor, dividend % divisor); +} + // @brief The class for Decimal related operations // The class contains Decimal related operations are scale and // precision aware. diff --git a/datatypes/mcs_int128.h b/datatypes/mcs_int128.h index 154342952..d8bacd8a8 100644 --- a/datatypes/mcs_int128.h +++ b/datatypes/mcs_int128.h @@ -111,6 +111,11 @@ template<> struct is_uint128_t { static const bool value = true; }; + +inline int128_t abs(int128_t& x) +{ + return (x >= 0) ? x : -x; +} class TSInt128 { diff --git a/utils/rowgroup/rowaggregation.cpp b/utils/rowgroup/rowaggregation.cpp index 48b2e434f..d624c5cab 100755 --- a/utils/rowgroup/rowaggregation.cpp +++ b/utils/rowgroup/rowaggregation.cpp @@ -2788,7 +2788,7 @@ void RowAggregationUM::calculateAvgColumns() uint32_t offset = fRow.getOffset(colOut); uint32_t scale = fRow.getScale(colOut); // Get multiplied to deliver AVG with the scale closest - // to the expected original scale + 4. + // to the expected original scale + 4. // There is a counterpart in buildAggregateColumn. datatypes::Decimal::setScalePrecision4Avg(precision, scale); int128_t* sumPnt = fRow.getBinaryField_offset(offset); @@ -2800,8 +2800,22 @@ void RowAggregationUM::calculateAvgColumns() multOp(*sumPnt, datatypes::mcs_pow_10[scaleDiff], sum); else sum = *sumPnt; - int128_t avg = sum / cnt; - fRow.setBinaryField_offset(&avg, sizeof(avg), offset); + datatypes::lldiv_t_128 avgAndRem = datatypes::lldiv128(sum, cnt); + // Round the last digit + if (datatypes::abs(avgAndRem.rem) * 2 >= (int128_t)cnt) + { + if (utils::is_negative(avgAndRem.rem)) + { + avgAndRem.quot--; + } + else + { + avgAndRem.quot++; + } + } + fRow.setBinaryField_offset(&avgAndRem.quot, + sizeof(avgAndRem.quot), + offset); } } }