From 02baaddc13b5cb40f77b44bcc104421030963012 Mon Sep 17 00:00:00 2001 From: myrrc Date: Wed, 16 Dec 2020 18:20:41 +0300 Subject: [PATCH 01/30] removed some casts --- src/Functions/FunctionBinaryArithmetic.h | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/src/Functions/FunctionBinaryArithmetic.h b/src/Functions/FunctionBinaryArithmetic.h index 73fd40db454..ef54c059b47 100644 --- a/src/Functions/FunctionBinaryArithmetic.h +++ b/src/Functions/FunctionBinaryArithmetic.h @@ -992,20 +992,8 @@ public: /// non-vector result if (col_left_const && col_right_const) { - NativeResultType const_a; - NativeResultType const_b; - - if constexpr (IsFloatingPoint && dec_a) - const_a = DecimalUtils::convertTo( - col_left_const->template getValue(), scale_a); - else - const_a = col_left_const->template getValue(); - - if constexpr (IsFloatingPoint && dec_b) - const_b = DecimalUtils::convertTo( - col_right_const->template getValue(), scale_b); - else - const_b = col_right_const->template getValue(); + NativeResultType const_a = col_left_const->template getValue(); + NativeResultType const_b = col_right_const->template getValue(); auto res = check_decimal_overflow ? OpImplCheck::template constantConstant(const_a, const_b, scale_a, scale_b) : From 17d996054ecd77d59246a26b48ff3f2436c17d3a Mon Sep 17 00:00:00 2001 From: myrrc Date: Wed, 16 Dec 2020 18:47:24 +0300 Subject: [PATCH 02/30] style fix --- src/Functions/FunctionBinaryArithmetic.h | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/Functions/FunctionBinaryArithmetic.h b/src/Functions/FunctionBinaryArithmetic.h index ef54c059b47..db68f07603c 100644 --- a/src/Functions/FunctionBinaryArithmetic.h +++ b/src/Functions/FunctionBinaryArithmetic.h @@ -548,7 +548,13 @@ class FunctionBinaryArithmetic : public IFunction template static bool castBothTypes(const IDataType * left, const IDataType * right, F && f) { - return castType(left, [&](const auto & left_) { return castType(right, [&](const auto & right_) { return f(left_, right_); }); }); + return castType(left, [&](const auto & left_) + { + return castType(right, [&](const auto & right_) + { + return f(left_, right_); + }); + }); } static FunctionOverloadResolverPtr @@ -1097,10 +1103,12 @@ public: const auto * left_generic = left_argument.type.get(); const auto * right_generic = right_argument.type.get(); ColumnPtr res; - bool valid = castBothTypes(left_generic, right_generic, [&](const auto & left, const auto & right) + + const bool valid = castBothTypes(left_generic, right_generic, [&](const auto & left, const auto & right) { using LeftDataType = std::decay_t; using RightDataType = std::decay_t; + if constexpr (std::is_same_v || std::is_same_v) { if constexpr (!Op::allow_fixed_string) From 9d5998fd10812ae4cf6b786dbe11fb947b6ec35b Mon Sep 17 00:00:00 2001 From: myrrc Date: Thu, 17 Dec 2020 18:46:01 +0300 Subject: [PATCH 03/30] added debug info --- src/Functions/FunctionBinaryArithmetic.h | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/Functions/FunctionBinaryArithmetic.h b/src/Functions/FunctionBinaryArithmetic.h index db68f07603c..d97b4dff06d 100644 --- a/src/Functions/FunctionBinaryArithmetic.h +++ b/src/Functions/FunctionBinaryArithmetic.h @@ -45,6 +45,8 @@ # pragma GCC diagnostic pop #endif +#include + namespace DB { @@ -548,6 +550,18 @@ class FunctionBinaryArithmetic : public IFunction template static bool castBothTypes(const IDataType * left, const IDataType * right, F && f) { + const bool l = castType(left, [](const auto& c){ + assert(std::is_same_v); + }); + + assert(l); + + const bool r = castType(right, [](const auto& c){ + assert(std::is_same_v); + }); + + assert(r); + return castType(left, [&](const auto & left_) { return castType(right, [&](const auto & right_) From 0d319c8c612285fc249a6aea9382e9aad3b8e7c4 Mon Sep 17 00:00:00 2001 From: myrrc Date: Thu, 17 Dec 2020 18:47:14 +0300 Subject: [PATCH 04/30] fix --- src/Functions/FunctionBinaryArithmetic.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Functions/FunctionBinaryArithmetic.h b/src/Functions/FunctionBinaryArithmetic.h index d97b4dff06d..ae1c344f507 100644 --- a/src/Functions/FunctionBinaryArithmetic.h +++ b/src/Functions/FunctionBinaryArithmetic.h @@ -551,13 +551,13 @@ class FunctionBinaryArithmetic : public IFunction static bool castBothTypes(const IDataType * left, const IDataType * right, F && f) { const bool l = castType(left, [](const auto& c){ - assert(std::is_same_v); + assert((std::is_same_v)); }); assert(l); const bool r = castType(right, [](const auto& c){ - assert(std::is_same_v); + assert((std::is_same_v)); }); assert(r); From 49f3b71b5f1c731c98eb0d90421bef968a675e64 Mon Sep 17 00:00:00 2001 From: myrrc Date: Thu, 17 Dec 2020 18:48:22 +0300 Subject: [PATCH 05/30] another fix --- src/Functions/FunctionBinaryArithmetic.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Functions/FunctionBinaryArithmetic.h b/src/Functions/FunctionBinaryArithmetic.h index ae1c344f507..bd287bb66cc 100644 --- a/src/Functions/FunctionBinaryArithmetic.h +++ b/src/Functions/FunctionBinaryArithmetic.h @@ -552,12 +552,14 @@ class FunctionBinaryArithmetic : public IFunction { const bool l = castType(left, [](const auto& c){ assert((std::is_same_v)); + return true; }); assert(l); const bool r = castType(right, [](const auto& c){ assert((std::is_same_v)); + return true; }); assert(r); From fcacf5c510c1ea34f3980863c92b34a191523f7d Mon Sep 17 00:00:00 2001 From: myrrc Date: Thu, 17 Dec 2020 19:31:54 +0300 Subject: [PATCH 06/30] another try --- src/Functions/FunctionBinaryArithmetic.h | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/src/Functions/FunctionBinaryArithmetic.h b/src/Functions/FunctionBinaryArithmetic.h index bd287bb66cc..34a0ad1618b 100644 --- a/src/Functions/FunctionBinaryArithmetic.h +++ b/src/Functions/FunctionBinaryArithmetic.h @@ -550,18 +550,10 @@ class FunctionBinaryArithmetic : public IFunction template static bool castBothTypes(const IDataType * left, const IDataType * right, F && f) { - const bool l = castType(left, [](const auto& c){ - assert((std::is_same_v)); - return true; - }); + const bool l = castType(left, [](const auto& c){ return std::is_same_v>; }); + const bool r = castType(right, [](const auto& c){ return std::is_same_v; }); assert(l); - - const bool r = castType(right, [](const auto& c){ - assert((std::is_same_v)); - return true; - }); - assert(r); return castType(left, [&](const auto & left_) From 737d4fc22fd2565e5551b15f6cd96aa419a271cd Mon Sep 17 00:00:00 2001 From: myrrc Date: Thu, 17 Dec 2020 19:43:29 +0300 Subject: [PATCH 07/30] debug fix --- src/Functions/FunctionBinaryArithmetic.h | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/Functions/FunctionBinaryArithmetic.h b/src/Functions/FunctionBinaryArithmetic.h index 34a0ad1618b..05989dad0a7 100644 --- a/src/Functions/FunctionBinaryArithmetic.h +++ b/src/Functions/FunctionBinaryArithmetic.h @@ -550,8 +550,10 @@ class FunctionBinaryArithmetic : public IFunction template static bool castBothTypes(const IDataType * left, const IDataType * right, F && f) { - const bool l = castType(left, [](const auto& c){ return std::is_same_v>; }); - const bool r = castType(right, [](const auto& c){ return std::is_same_v; }); + const bool l = castType(left, [](const auto& c){ + return std::is_same_v>; }); + const bool r = castType(right, [](const auto& c){ + return std::is_same_v; }); assert(l); assert(r); From af8a72a45f2162f69c3cc0034af607cc17d288f9 Mon Sep 17 00:00:00 2001 From: myrrc Date: Thu, 17 Dec 2020 20:16:37 +0300 Subject: [PATCH 08/30] fix --- src/Functions/FunctionBinaryArithmetic.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Functions/FunctionBinaryArithmetic.h b/src/Functions/FunctionBinaryArithmetic.h index 05989dad0a7..0147ff1de86 100644 --- a/src/Functions/FunctionBinaryArithmetic.h +++ b/src/Functions/FunctionBinaryArithmetic.h @@ -551,9 +551,9 @@ class FunctionBinaryArithmetic : public IFunction static bool castBothTypes(const IDataType * left, const IDataType * right, F && f) { const bool l = castType(left, [](const auto& c){ - return std::is_same_v>; }); + return std::is_same_v, DataTypeDecimal>; }); const bool r = castType(right, [](const auto& c){ - return std::is_same_v; }); + return std::is_same_v, DataTypeFloat32>; }); assert(l); assert(r); From fa6a1d3b4a70f3217df426934b14de4c6a166e6d Mon Sep 17 00:00:00 2001 From: myrrc Date: Thu, 17 Dec 2020 20:55:35 +0300 Subject: [PATCH 09/30] fix --- src/Functions/FunctionBinaryArithmetic.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Functions/FunctionBinaryArithmetic.h b/src/Functions/FunctionBinaryArithmetic.h index 0147ff1de86..3d1c53f62b0 100644 --- a/src/Functions/FunctionBinaryArithmetic.h +++ b/src/Functions/FunctionBinaryArithmetic.h @@ -553,7 +553,7 @@ class FunctionBinaryArithmetic : public IFunction const bool l = castType(left, [](const auto& c){ return std::is_same_v, DataTypeDecimal>; }); const bool r = castType(right, [](const auto& c){ - return std::is_same_v, DataTypeFloat32>; }); + return std::is_same_v, DataTypeFloat64>; }); assert(l); assert(r); From 9f217c5df57e6fe8d8ca5d823d4889e39830ffc1 Mon Sep 17 00:00:00 2001 From: myrrc Date: Thu, 17 Dec 2020 21:11:39 +0300 Subject: [PATCH 10/30] case fix --- src/Functions/FunctionBinaryArithmetic.h | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/Functions/FunctionBinaryArithmetic.h b/src/Functions/FunctionBinaryArithmetic.h index 3d1c53f62b0..7986b6f0a24 100644 --- a/src/Functions/FunctionBinaryArithmetic.h +++ b/src/Functions/FunctionBinaryArithmetic.h @@ -550,14 +550,6 @@ class FunctionBinaryArithmetic : public IFunction template static bool castBothTypes(const IDataType * left, const IDataType * right, F && f) { - const bool l = castType(left, [](const auto& c){ - return std::is_same_v, DataTypeDecimal>; }); - const bool r = castType(right, [](const auto& c){ - return std::is_same_v, DataTypeFloat64>; }); - - assert(l); - assert(r); - return castType(left, [&](const auto & left_) { return castType(right, [&](const auto & right_) From d24fc58c94e270f48834451f4807aba10fe330fc Mon Sep 17 00:00:00 2001 From: myrrc Date: Thu, 17 Dec 2020 21:12:24 +0300 Subject: [PATCH 11/30] case fix 2 --- src/Functions/FunctionBinaryArithmetic.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Functions/FunctionBinaryArithmetic.h b/src/Functions/FunctionBinaryArithmetic.h index 7986b6f0a24..a5e706c75a8 100644 --- a/src/Functions/FunctionBinaryArithmetic.h +++ b/src/Functions/FunctionBinaryArithmetic.h @@ -472,6 +472,11 @@ public: Case && IsIntegralOrExtended, LeftDataType>, Case && IsIntegralOrExtended, RightDataType>, + /// e.g Decimal * Float64 = Float64 + Case::multiply, Switch< + Case && IsFloatingPoint, RightDataType>, + Case && IsFloatingPoint, LeftDataType>>>, + /// Decimal Real is not supported (traditional DBs convert Decimal Real to Real) Case && !IsIntegralOrExtendedOrDecimal, InvalidType>, Case && !IsIntegralOrExtendedOrDecimal, InvalidType>, @@ -480,11 +485,6 @@ public: Case && !IsDateOrDateTime, DataTypeFromFieldType>, - /// e.g Decimal * Float64 = Float64 - Case::multiply, Switch< - Case && IsFloatingPoint, RightDataType>, - Case && IsFloatingPoint, LeftDataType>>>, - /// Date + Integral -> Date /// Integral + Date -> Date Case::plus, Switch< From 494e9066c33c9949d2063f8a63ce1f31120c8805 Mon Sep 17 00:00:00 2001 From: myrrc Date: Thu, 17 Dec 2020 21:17:38 +0300 Subject: [PATCH 12/30] fixing first compile bug --- src/Functions/FunctionBinaryArithmetic.h | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/Functions/FunctionBinaryArithmetic.h b/src/Functions/FunctionBinaryArithmetic.h index a5e706c75a8..cc030d76537 100644 --- a/src/Functions/FunctionBinaryArithmetic.h +++ b/src/Functions/FunctionBinaryArithmetic.h @@ -1000,8 +1000,20 @@ public: /// non-vector result if (col_left_const && col_right_const) { - NativeResultType const_a = col_left_const->template getValue(); - NativeResultType const_b = col_right_const->template getValue(); + NativeResultType const_a; + NativeResultType const_b; + + if constexpr (IsFloatingPoint && dec_a) + const_a = DecimalUtils::convertTo( + col_left_const->template getValue(), scale_a); + else + const_a = col_left_const->template getValue(); + + if constexpr (IsFloatingPoint && dec_b) + const_b = DecimalUtils::convertTo( + col_right_const->template getValue(), scale_b); + else + const_b = col_right_const->template getValue(); auto res = check_decimal_overflow ? OpImplCheck::template constantConstant(const_a, const_b, scale_a, scale_b) : From 1f67b0ce8d68693262a6e714d5c9f2f40b74c9a1 Mon Sep 17 00:00:00 2001 From: myrrc Date: Thu, 17 Dec 2020 21:28:51 +0300 Subject: [PATCH 13/30] reverting if constexpr --- src/Functions/FunctionBinaryArithmetic.h | 29 ++++++++++++------------ 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/src/Functions/FunctionBinaryArithmetic.h b/src/Functions/FunctionBinaryArithmetic.h index cc030d76537..05cef1ce0be 100644 --- a/src/Functions/FunctionBinaryArithmetic.h +++ b/src/Functions/FunctionBinaryArithmetic.h @@ -987,11 +987,11 @@ public: using OpImpl = DecimalBinaryOperation; using OpImplCheck = DecimalBinaryOperation; - ResultDataType type = decimalResultType(left, right); - static constexpr const bool dec_a = IsDecimalNumber; static constexpr const bool dec_b = IsDecimalNumber; + ResultDataType type = decimalResultType(left, right); + typename ResultDataType::FieldType scale_a = type.scaleFactorFor(left, is_multiply); typename ResultDataType::FieldType scale_b = type.scaleFactorFor(right, is_multiply || is_division); if constexpr (IsDataTypeDecimal && is_division) @@ -1000,20 +1000,21 @@ public: /// non-vector result if (col_left_const && col_right_const) { - NativeResultType const_a; - NativeResultType const_b; + NativeResultType const_a = col_left_const->template getValue(); + NativeResultType const_b = col_right_const->template getValue(); - if constexpr (IsFloatingPoint && dec_a) - const_a = DecimalUtils::convertTo( - col_left_const->template getValue(), scale_a); - else - const_a = col_left_const->template getValue(); + //if constexpr (IsFloatingPoint && dec_a) + // const_a = DecimalUtils::convertTo( + // col_left_const->template getValue(), scale_a); + //else + // const_a = col_left_const->template getValue(); + + //if constexpr (IsFloatingPoint && dec_b) + // const_b = DecimalUtils::convertTo( + // col_right_const->template getValue(), scale_b); + //else + // const_b = col_right_const->template getValue(); - if constexpr (IsFloatingPoint && dec_b) - const_b = DecimalUtils::convertTo( - col_right_const->template getValue(), scale_b); - else - const_b = col_right_const->template getValue(); auto res = check_decimal_overflow ? OpImplCheck::template constantConstant(const_a, const_b, scale_a, scale_b) : From ea2da764f6bd4e1500adaebfc9794879bf414526 Mon Sep 17 00:00:00 2001 From: myrrc Date: Thu, 17 Dec 2020 21:33:51 +0300 Subject: [PATCH 14/30] fixed result data type init --- src/Functions/FunctionBinaryArithmetic.h | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/Functions/FunctionBinaryArithmetic.h b/src/Functions/FunctionBinaryArithmetic.h index 05cef1ce0be..c43b52ec71f 100644 --- a/src/Functions/FunctionBinaryArithmetic.h +++ b/src/Functions/FunctionBinaryArithmetic.h @@ -990,7 +990,14 @@ public: static constexpr const bool dec_a = IsDecimalNumber; static constexpr const bool dec_b = IsDecimalNumber; - ResultDataType type = decimalResultType(left, right); + ResultDataType type; + + if constexpr(dec_a && IsFloatingPoint) + type = RightDataType(); + else if constexpr(dec_b && IsFloatingPoint) + type = LeftDataType(); + else + type = decimalResultType(left, right); typename ResultDataType::FieldType scale_a = type.scaleFactorFor(left, is_multiply); typename ResultDataType::FieldType scale_b = type.scaleFactorFor(right, is_multiply || is_division); From 3e9428515c28aa6b9f73fa1a9ff02f7115f913a8 Mon Sep 17 00:00:00 2001 From: myrrc Date: Thu, 17 Dec 2020 21:36:15 +0300 Subject: [PATCH 15/30] fix deleted constuctor --- src/Functions/FunctionBinaryArithmetic.h | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/Functions/FunctionBinaryArithmetic.h b/src/Functions/FunctionBinaryArithmetic.h index c43b52ec71f..d515872d381 100644 --- a/src/Functions/FunctionBinaryArithmetic.h +++ b/src/Functions/FunctionBinaryArithmetic.h @@ -990,14 +990,14 @@ public: static constexpr const bool dec_a = IsDecimalNumber; static constexpr const bool dec_b = IsDecimalNumber; - ResultDataType type; - - if constexpr(dec_a && IsFloatingPoint) - type = RightDataType(); - else if constexpr(dec_b && IsFloatingPoint) - type = LeftDataType(); - else - type = decimalResultType(left, right); + const ResultDataType type = [&] { + if constexpr(dec_a && IsFloatingPoint) + return RightDataType(); + else if constexpr(dec_b && IsFloatingPoint) + return LeftDataType(); + else + return decimalResultType(left, right); + }(); typename ResultDataType::FieldType scale_a = type.scaleFactorFor(left, is_multiply); typename ResultDataType::FieldType scale_b = type.scaleFactorFor(right, is_multiply || is_division); From 54887dacbf2a1243e1dc70dee259787141592ab2 Mon Sep 17 00:00:00 2001 From: myrrc Date: Thu, 17 Dec 2020 21:38:36 +0300 Subject: [PATCH 16/30] fix scale init --- src/Functions/FunctionBinaryArithmetic.h | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/Functions/FunctionBinaryArithmetic.h b/src/Functions/FunctionBinaryArithmetic.h index d515872d381..2f0f5e41f4f 100644 --- a/src/Functions/FunctionBinaryArithmetic.h +++ b/src/Functions/FunctionBinaryArithmetic.h @@ -999,8 +999,11 @@ public: return decimalResultType(left, right); }(); - typename ResultDataType::FieldType scale_a = type.scaleFactorFor(left, is_multiply); - typename ResultDataType::FieldType scale_b = type.scaleFactorFor(right, is_multiply || is_division); + typename ResultDataType::FieldType scale_a = dec_a ? type.scaleFactorFor(left, is_multiply) : 0; + typename ResultDataType::FieldType scale_b = dec_b + ? type.scaleFactorFor(right, is_multiply || is_division) + : 0; + if constexpr (IsDataTypeDecimal && is_division) scale_a = right.getScaleMultiplier(); From 39c82fe50cb3652ae1f426650634bb89468023f5 Mon Sep 17 00:00:00 2001 From: myrrc Date: Thu, 17 Dec 2020 21:52:19 +0300 Subject: [PATCH 17/30] fixed result type --- src/Functions/FunctionBinaryArithmetic.h | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/Functions/FunctionBinaryArithmetic.h b/src/Functions/FunctionBinaryArithmetic.h index 2f0f5e41f4f..e3876c93817 100644 --- a/src/Functions/FunctionBinaryArithmetic.h +++ b/src/Functions/FunctionBinaryArithmetic.h @@ -999,10 +999,8 @@ public: return decimalResultType(left, right); }(); - typename ResultDataType::FieldType scale_a = dec_a ? type.scaleFactorFor(left, is_multiply) : 0; - typename ResultDataType::FieldType scale_b = dec_b - ? type.scaleFactorFor(right, is_multiply || is_division) - : 0; + typename ResultDataType::FieldType scale_a = type.scaleFactorFor(left, is_multiply); + typename ResultDataType::FieldType scale_b = type.scaleFactorFor(right, is_multiply || is_division); if constexpr (IsDataTypeDecimal && is_division) scale_a = right.getScaleMultiplier(); @@ -1025,13 +1023,15 @@ public: //else // const_b = col_right_const->template getValue(); - auto res = check_decimal_overflow ? OpImplCheck::template constantConstant(const_a, const_b, scale_a, scale_b) : OpImpl::template constantConstant(const_a, const_b, scale_a, scale_b); - return ResultDataType(type.getPrecision(), type.getScale()).createColumnConst( + if constexpr (IsDataTypeDecimal) + return ResultDataType(type.getPrecision(), type.getScale()).createColumnConst( col_left_const->size(), toField(res, type.getScale())); + else + return ResultDataType().createColumnConst(col_left_const->size(), toField(res)); } col_res = ColVecResult::create(0, type.getScale()); From b428f8fcd88c8bcc29fe3aac86361df672d5d0e4 Mon Sep 17 00:00:00 2001 From: myrrc Date: Thu, 17 Dec 2020 22:01:04 +0300 Subject: [PATCH 18/30] fixed scales --- src/Functions/FunctionBinaryArithmetic.h | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/Functions/FunctionBinaryArithmetic.h b/src/Functions/FunctionBinaryArithmetic.h index e3876c93817..9bb2fb3e824 100644 --- a/src/Functions/FunctionBinaryArithmetic.h +++ b/src/Functions/FunctionBinaryArithmetic.h @@ -986,9 +986,11 @@ public: using NativeResultType = typename NativeType::Type; using OpImpl = DecimalBinaryOperation; using OpImplCheck = DecimalBinaryOperation; + using FieldType = typename ResultDataType::FieldType; static constexpr const bool dec_a = IsDecimalNumber; static constexpr const bool dec_b = IsDecimalNumber; + static constexpr const bool result_is_decimal = IsDataTypeDecimal; const ResultDataType type = [&] { if constexpr(dec_a && IsFloatingPoint) @@ -999,11 +1001,16 @@ public: return decimalResultType(left, right); }(); - typename ResultDataType::FieldType scale_a = type.scaleFactorFor(left, is_multiply); - typename ResultDataType::FieldType scale_b = type.scaleFactorFor(right, is_multiply || is_division); + FieldType scale_a; + FieldType scale_b; if constexpr (IsDataTypeDecimal && is_division) scale_a = right.getScaleMultiplier(); + else if constexpr(result_is_decimal) + scale_a = type.scaleFactorFor(left, is_multiply); + + if constexpr(result_is_decimal) + scale_b = type.scaleFactorFor(right, is_multiply || is_division); /// non-vector result if (col_left_const && col_right_const) @@ -1027,7 +1034,7 @@ public: OpImplCheck::template constantConstant(const_a, const_b, scale_a, scale_b) : OpImpl::template constantConstant(const_a, const_b, scale_a, scale_b); - if constexpr (IsDataTypeDecimal) + if constexpr (result_is_decimal) return ResultDataType(type.getPrecision(), type.getScale()).createColumnConst( col_left_const->size(), toField(res, type.getScale())); else From fd1d24c928acdcf8d0009ba975209fc2876ebfd0 Mon Sep 17 00:00:00 2001 From: myrrc Date: Thu, 17 Dec 2020 22:10:33 +0300 Subject: [PATCH 19/30] fixed overflow check --- src/Functions/FunctionBinaryArithmetic.h | 6 +++++- src/Functions/multiply.cpp | 8 +++++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/Functions/FunctionBinaryArithmetic.h b/src/Functions/FunctionBinaryArithmetic.h index 9bb2fb3e824..b58ea9b9e05 100644 --- a/src/Functions/FunctionBinaryArithmetic.h +++ b/src/Functions/FunctionBinaryArithmetic.h @@ -1041,7 +1041,11 @@ public: return ResultDataType().createColumnConst(col_left_const->size(), toField(res)); } - col_res = ColVecResult::create(0, type.getScale()); + if constexpr(IsDecimalNumber) + col_res = ColVecResult::create(0, type.getScale()); + else + col_res = ColVecResult::create(0); + auto & vec_res = col_res->getData(); vec_res.resize(col_left_raw->size()); diff --git a/src/Functions/multiply.cpp b/src/Functions/multiply.cpp index 18c1927b4a5..d3171a48825 100644 --- a/src/Functions/multiply.cpp +++ b/src/Functions/multiply.cpp @@ -1,3 +1,4 @@ +#include #include #include #include @@ -29,7 +30,12 @@ struct MultiplyImpl template static inline bool apply(A a, B b, Result & c) { - return common::mulOverflow(static_cast(a), b, c); + if constexpr(std::is_same_v || std::is_same_v) + { + c = static_cast(a) * b; + return false; + } else + return common::mulOverflow(static_cast(a), b, c); } #if USE_EMBEDDED_COMPILER From c61af32360f95b5cffa8827b77e603fcb7c01740 Mon Sep 17 00:00:00 2001 From: myrrc Date: Thu, 17 Dec 2020 22:12:12 +0300 Subject: [PATCH 20/30] fixed const column branch --- src/Functions/FunctionBinaryArithmetic.h | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/Functions/FunctionBinaryArithmetic.h b/src/Functions/FunctionBinaryArithmetic.h index b58ea9b9e05..b04e07833ba 100644 --- a/src/Functions/FunctionBinaryArithmetic.h +++ b/src/Functions/FunctionBinaryArithmetic.h @@ -1015,20 +1015,20 @@ public: /// non-vector result if (col_left_const && col_right_const) { - NativeResultType const_a = col_left_const->template getValue(); - NativeResultType const_b = col_right_const->template getValue(); + NativeResultType const_a; + NativeResultType const_b; - //if constexpr (IsFloatingPoint && dec_a) - // const_a = DecimalUtils::convertTo( - // col_left_const->template getValue(), scale_a); - //else - // const_a = col_left_const->template getValue(); + if constexpr (IsFloatingPoint && dec_a) + const_a = DecimalUtils::convertTo( + col_left_const->template getValue(), scale_a); + else + const_a = col_left_const->template getValue(); - //if constexpr (IsFloatingPoint && dec_b) - // const_b = DecimalUtils::convertTo( - // col_right_const->template getValue(), scale_b); - //else - // const_b = col_right_const->template getValue(); + if constexpr (IsFloatingPoint && dec_b) + const_b = DecimalUtils::convertTo( + col_right_const->template getValue(), scale_b); + else + const_b = col_right_const->template getValue(); auto res = check_decimal_overflow ? OpImplCheck::template constantConstant(const_a, const_b, scale_a, scale_b) : From 1c52842e835a661a9fb36693a45ff70a1be96633 Mon Sep 17 00:00:00 2001 From: myrrc Date: Thu, 17 Dec 2020 22:21:23 +0300 Subject: [PATCH 21/30] simplified the condition --- src/Functions/FunctionBinaryArithmetic.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Functions/FunctionBinaryArithmetic.h b/src/Functions/FunctionBinaryArithmetic.h index b04e07833ba..ad4be84d337 100644 --- a/src/Functions/FunctionBinaryArithmetic.h +++ b/src/Functions/FunctionBinaryArithmetic.h @@ -1018,13 +1018,13 @@ public: NativeResultType const_a; NativeResultType const_b; - if constexpr (IsFloatingPoint && dec_a) + if constexpr (!result_is_decimal && dec_a) const_a = DecimalUtils::convertTo( col_left_const->template getValue(), scale_a); else const_a = col_left_const->template getValue(); - if constexpr (IsFloatingPoint && dec_b) + if constexpr (!result_is_decimal && dec_b) const_b = DecimalUtils::convertTo( col_right_const->template getValue(), scale_b); else From 613f4e0f04405a9ce5901f36f82935e32c1ca03b Mon Sep 17 00:00:00 2001 From: myrrc Date: Thu, 17 Dec 2020 22:24:09 +0300 Subject: [PATCH 22/30] removed explicit types --- src/Functions/FunctionBinaryArithmetic.h | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/src/Functions/FunctionBinaryArithmetic.h b/src/Functions/FunctionBinaryArithmetic.h index ad4be84d337..c99f12a99b0 100644 --- a/src/Functions/FunctionBinaryArithmetic.h +++ b/src/Functions/FunctionBinaryArithmetic.h @@ -1015,20 +1015,8 @@ public: /// non-vector result if (col_left_const && col_right_const) { - NativeResultType const_a; - NativeResultType const_b; - - if constexpr (!result_is_decimal && dec_a) - const_a = DecimalUtils::convertTo( - col_left_const->template getValue(), scale_a); - else - const_a = col_left_const->template getValue(); - - if constexpr (!result_is_decimal && dec_b) - const_b = DecimalUtils::convertTo( - col_right_const->template getValue(), scale_b); - else - const_b = col_right_const->template getValue(); + auto const_a = col_left_const->template getValue(); + auto const_b = col_right_const->template getValue(); auto res = check_decimal_overflow ? OpImplCheck::template constantConstant(const_a, const_b, scale_a, scale_b) : From 31e3ca16703e06b5ac5cd3bc39631d0dfdaa12cc Mon Sep 17 00:00:00 2001 From: myrrc Date: Thu, 17 Dec 2020 22:26:15 +0300 Subject: [PATCH 23/30] restored type signature --- src/Functions/FunctionBinaryArithmetic.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Functions/FunctionBinaryArithmetic.h b/src/Functions/FunctionBinaryArithmetic.h index c99f12a99b0..0f6fbad3065 100644 --- a/src/Functions/FunctionBinaryArithmetic.h +++ b/src/Functions/FunctionBinaryArithmetic.h @@ -1015,8 +1015,8 @@ public: /// non-vector result if (col_left_const && col_right_const) { - auto const_a = col_left_const->template getValue(); - auto const_b = col_right_const->template getValue(); + const NativeResultType const_a = col_left_const->template getValue(); + const NativeResultType const_b = col_right_const->template getValue(); auto res = check_decimal_overflow ? OpImplCheck::template constantConstant(const_a, const_b, scale_a, scale_b) : From 6eecef35a6b09f1d83f5e1c75db8200d4a438fe5 Mon Sep 17 00:00:00 2001 From: myrrc Date: Thu, 17 Dec 2020 22:35:46 +0300 Subject: [PATCH 24/30] fixed result type trait --- src/Core/DecimalFunctions.h | 6 +++++- src/Functions/FunctionBinaryArithmetic.h | 9 ++++++--- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/src/Core/DecimalFunctions.h b/src/Core/DecimalFunctions.h index b82cfd88e98..a0a51651353 100644 --- a/src/Core/DecimalFunctions.h +++ b/src/Core/DecimalFunctions.h @@ -6,6 +6,7 @@ #include #include +#include namespace DB @@ -205,10 +206,13 @@ inline typename DecimalType::NativeType getFractionalPart(const DecimalType & de return getFractionalPartWithScaleMultiplier(decimal, scaleMultiplier(scale)); } -/// Decimal to integer/float conversion +/// Decimal to integer/float conversion with the ability to convert into itself (returns the original value); template To convertTo(const DecimalType & decimal, size_t scale) { + if constexpr (std::is_same_v) + return decimal; + using NativeT = typename DecimalType::NativeType; if constexpr (std::is_floating_point_v) diff --git a/src/Functions/FunctionBinaryArithmetic.h b/src/Functions/FunctionBinaryArithmetic.h index 0f6fbad3065..c871cfd914d 100644 --- a/src/Functions/FunctionBinaryArithmetic.h +++ b/src/Functions/FunctionBinaryArithmetic.h @@ -22,6 +22,7 @@ #include #include #include +#include "Core/DecimalFunctions.h" #include "IFunctionImpl.h" #include "FunctionHelpers.h" #include "IsOperation.h" @@ -209,7 +210,9 @@ struct DecimalBinaryOperation DivideIntegralImpl, /// substitute divide by intDiv (throw on division by zero) Operation>; - using ArrayC = typename ColumnDecimal::Container; + using ArrayC = typename std::conditional_t, + ColumnDecimal, + ColumnVector>::Container; template static void NO_INLINE vectorVector(const ArrayA & a, const ArrayB & b, ArrayC & c, @@ -1015,8 +1018,8 @@ public: /// non-vector result if (col_left_const && col_right_const) { - const NativeResultType const_a = col_left_const->template getValue(); - const NativeResultType const_b = col_right_const->template getValue(); + NativeResultType const_a = col_left_const->template getValue(); + NativeResultType const_b = col_right_const->template getValue(); auto res = check_decimal_overflow ? OpImplCheck::template constantConstant(const_a, const_b, scale_a, scale_b) : From 5974e784c29b54c59b1d84a1cecd1049cbc5e5a5 Mon Sep 17 00:00:00 2001 From: myrrc Date: Thu, 17 Dec 2020 22:52:52 +0300 Subject: [PATCH 25/30] added the conversion func --- src/Functions/FunctionBinaryArithmetic.h | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/Functions/FunctionBinaryArithmetic.h b/src/Functions/FunctionBinaryArithmetic.h index c871cfd914d..5223dc6cb4a 100644 --- a/src/Functions/FunctionBinaryArithmetic.h +++ b/src/Functions/FunctionBinaryArithmetic.h @@ -1015,11 +1015,18 @@ public: if constexpr(result_is_decimal) scale_b = type.scaleFactorFor(right, is_multiply || is_division); + const auto get_value_or_convert = [](const auto& col, const auto & scale) { + if constexpr(IsFloatingPoint && IsDecimalNumber) + return DecimalUtils::convertTo(col->template getValue(), scale); + else + return col->template getValue(); + }; + /// non-vector result if (col_left_const && col_right_const) { - NativeResultType const_a = col_left_const->template getValue(); - NativeResultType const_b = col_right_const->template getValue(); + const NativeResultType const_a = get_value_or_convert(col_left_const, scale_a); + const NativeResultType const_b = get_value_or_convert(col_right_const, scale_b); auto res = check_decimal_overflow ? OpImplCheck::template constantConstant(const_a, const_b, scale_a, scale_b) : @@ -1040,6 +1047,7 @@ public: auto & vec_res = col_res->getData(); vec_res.resize(col_left_raw->size()); + if (col_left && col_right) { if (check_decimal_overflow) @@ -1049,7 +1057,7 @@ public: } else if (col_left_const && col_right) { - NativeResultType const_a = col_left_const->template getValue(); + const NativeResultType const_a = get_value_or_convert(col_left_const, scale_a); if (check_decimal_overflow) OpImplCheck::template constantVector(const_a, col_right->getData(), vec_res, scale_a, scale_b); @@ -1058,7 +1066,7 @@ public: } else if (col_left && col_right_const) { - NativeResultType const_b = col_right_const->template getValue(); + const NativeResultType const_b = get_value_or_convert(col_right_const, scale_b); if (check_decimal_overflow) OpImplCheck::template vectorConstant(col_left->getData(), const_b, vec_res, scale_a, scale_b); From f93385dcf01697a3cd5e77d8ef7f5b69aa8c90ac Mon Sep 17 00:00:00 2001 From: myrrc Date: Thu, 17 Dec 2020 23:01:29 +0300 Subject: [PATCH 26/30] replaced lambda with helper function --- src/Functions/FunctionBinaryArithmetic.h | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/src/Functions/FunctionBinaryArithmetic.h b/src/Functions/FunctionBinaryArithmetic.h index 5223dc6cb4a..75ce3bcd7bd 100644 --- a/src/Functions/FunctionBinaryArithmetic.h +++ b/src/Functions/FunctionBinaryArithmetic.h @@ -957,6 +957,14 @@ public: return nullptr; } + template + static auto helperGetOrConvert(const auto& col, const auto& scale) { + if constexpr(IsFloatingPoint && IsDecimalNumber) + return DecimalUtils::convertTo(col->template getValue(), scale); + else + return col->template getValue(); + } + template ColumnPtr executeNumeric(const ColumnsWithTypeAndName & arguments, const A & left, const B & right) const { @@ -1015,18 +1023,11 @@ public: if constexpr(result_is_decimal) scale_b = type.scaleFactorFor(right, is_multiply || is_division); - const auto get_value_or_convert = [](const auto& col, const auto & scale) { - if constexpr(IsFloatingPoint && IsDecimalNumber) - return DecimalUtils::convertTo(col->template getValue(), scale); - else - return col->template getValue(); - }; - /// non-vector result if (col_left_const && col_right_const) { - const NativeResultType const_a = get_value_or_convert(col_left_const, scale_a); - const NativeResultType const_b = get_value_or_convert(col_right_const, scale_b); + const NativeResultType const_a = helperGetOrConvert(col_left_const, scale_a); + const NativeResultType const_b = helperGetOrConvert(col_right_const, scale_b); auto res = check_decimal_overflow ? OpImplCheck::template constantConstant(const_a, const_b, scale_a, scale_b) : @@ -1057,7 +1058,7 @@ public: } else if (col_left_const && col_right) { - const NativeResultType const_a = get_value_or_convert(col_left_const, scale_a); + const NativeResultType const_a = helperGetOrConvert(col_left_const, scale_a); if (check_decimal_overflow) OpImplCheck::template constantVector(const_a, col_right->getData(), vec_res, scale_a, scale_b); @@ -1066,7 +1067,7 @@ public: } else if (col_left && col_right_const) { - const NativeResultType const_b = get_value_or_convert(col_right_const, scale_b); + const NativeResultType const_b = helperGetOrConvert(col_right_const, scale_b); if (check_decimal_overflow) OpImplCheck::template vectorConstant(col_left->getData(), const_b, vec_res, scale_a, scale_b); From e50af17545827eb9ef657ff7d17c319afb17428e Mon Sep 17 00:00:00 2001 From: myrrc Date: Thu, 17 Dec 2020 23:05:10 +0300 Subject: [PATCH 27/30] fixed template args1 --- src/Functions/FunctionBinaryArithmetic.h | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/Functions/FunctionBinaryArithmetic.h b/src/Functions/FunctionBinaryArithmetic.h index 75ce3bcd7bd..8dadfe5b398 100644 --- a/src/Functions/FunctionBinaryArithmetic.h +++ b/src/Functions/FunctionBinaryArithmetic.h @@ -1026,8 +1026,10 @@ public: /// non-vector result if (col_left_const && col_right_const) { - const NativeResultType const_a = helperGetOrConvert(col_left_const, scale_a); - const NativeResultType const_b = helperGetOrConvert(col_right_const, scale_b); + const NativeResultType const_a = helperGetOrConvert( + col_left_const, scale_a); + const NativeResultType const_b = helperGetOrConvert( + col_right_const, scale_b); auto res = check_decimal_overflow ? OpImplCheck::template constantConstant(const_a, const_b, scale_a, scale_b) : @@ -1058,7 +1060,8 @@ public: } else if (col_left_const && col_right) { - const NativeResultType const_a = helperGetOrConvert(col_left_const, scale_a); + const NativeResultType const_a = helperGetOrConvert( + col_left_const, scale_a); if (check_decimal_overflow) OpImplCheck::template constantVector(const_a, col_right->getData(), vec_res, scale_a, scale_b); @@ -1067,7 +1070,8 @@ public: } else if (col_left && col_right_const) { - const NativeResultType const_b = helperGetOrConvert(col_right_const, scale_b); + const NativeResultType const_b = helperGetOrConvert( + col_right_const, scale_b); if (check_decimal_overflow) OpImplCheck::template vectorConstant(col_left->getData(), const_b, vec_res, scale_a, scale_b); From 488f4ef42e90988667f77ec5b6d7e25fef9a3a03 Mon Sep 17 00:00:00 2001 From: myrrc Date: Thu, 17 Dec 2020 23:09:33 +0300 Subject: [PATCH 28/30] fixed compile condition --- src/Functions/FunctionBinaryArithmetic.h | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/Functions/FunctionBinaryArithmetic.h b/src/Functions/FunctionBinaryArithmetic.h index 8dadfe5b398..72b5dbdcecb 100644 --- a/src/Functions/FunctionBinaryArithmetic.h +++ b/src/Functions/FunctionBinaryArithmetic.h @@ -957,9 +957,12 @@ public: return nullptr; } - template + template static auto helperGetOrConvert(const auto& col, const auto& scale) { - if constexpr(IsFloatingPoint && IsDecimalNumber) + using ResultType = typename ResultDataType::FieldType; + using NativeResultType = typename NativeType::Type; + + if constexpr(IsFloatingPoint && IsDecimalNumber) return DecimalUtils::convertTo(col->template getValue(), scale); else return col->template getValue(); @@ -1026,9 +1029,9 @@ public: /// non-vector result if (col_left_const && col_right_const) { - const NativeResultType const_a = helperGetOrConvert( + const NativeResultType const_a = helperGetOrConvert( col_left_const, scale_a); - const NativeResultType const_b = helperGetOrConvert( + const NativeResultType const_b = helperGetOrConvert( col_right_const, scale_b); auto res = check_decimal_overflow ? @@ -1060,7 +1063,7 @@ public: } else if (col_left_const && col_right) { - const NativeResultType const_a = helperGetOrConvert( + const NativeResultType const_a = helperGetOrConvert( col_left_const, scale_a); if (check_decimal_overflow) @@ -1070,7 +1073,7 @@ public: } else if (col_left && col_right_const) { - const NativeResultType const_b = helperGetOrConvert( + const NativeResultType const_b = helperGetOrConvert( col_right_const, scale_b); if (check_decimal_overflow) From 73f3195fe17610f814453b27c8f4bbd15a54820d Mon Sep 17 00:00:00 2001 From: myrrc Date: Thu, 17 Dec 2020 23:12:41 +0300 Subject: [PATCH 29/30] fixed variable init --- src/Functions/FunctionBinaryArithmetic.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Functions/FunctionBinaryArithmetic.h b/src/Functions/FunctionBinaryArithmetic.h index 72b5dbdcecb..35fdd8f0230 100644 --- a/src/Functions/FunctionBinaryArithmetic.h +++ b/src/Functions/FunctionBinaryArithmetic.h @@ -1022,6 +1022,8 @@ public: scale_a = right.getScaleMultiplier(); else if constexpr(result_is_decimal) scale_a = type.scaleFactorFor(left, is_multiply); + else + scale_a = 0.0; //won't be used if constexpr(result_is_decimal) scale_b = type.scaleFactorFor(right, is_multiply || is_division); From 61d9d213e91f7fbc0381a2ef81b1e941d7fd2506 Mon Sep 17 00:00:00 2001 From: myrrc Date: Thu, 17 Dec 2020 23:14:02 +0300 Subject: [PATCH 30/30] fixed another variable init --- src/Functions/FunctionBinaryArithmetic.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Functions/FunctionBinaryArithmetic.h b/src/Functions/FunctionBinaryArithmetic.h index 35fdd8f0230..68b761c4904 100644 --- a/src/Functions/FunctionBinaryArithmetic.h +++ b/src/Functions/FunctionBinaryArithmetic.h @@ -1023,10 +1023,12 @@ public: else if constexpr(result_is_decimal) scale_a = type.scaleFactorFor(left, is_multiply); else - scale_a = 0.0; //won't be used + scale_a = 0.0; //won't be used as the target column is not decimal if constexpr(result_is_decimal) scale_b = type.scaleFactorFor(right, is_multiply || is_division); + else + scale_b = 0.0; //won't be used, same /// non-vector result if (col_left_const && col_right_const)