diff --git a/src/Functions/DivisionUtils.h b/src/Functions/DivisionUtils.h index 440bb3c62d4..3f05a820354 100644 --- a/src/Functions/DivisionUtils.h +++ b/src/Functions/DivisionUtils.h @@ -27,8 +27,11 @@ inline void throwIfDivisionLeadsToFPE(A a, B b) throw Exception(ErrorCodes::ILLEGAL_DIVISION, "Division by zero"); /// http://avva.livejournal.com/2548306.html - if (unlikely(is_signed_v && is_signed_v && a == std::numeric_limits::min() && b == -1)) - throw Exception(ErrorCodes::ILLEGAL_DIVISION, "Division of minimal signed number by minus one"); + if constexpr (is_signed_v && is_signed_v) + { + if (unlikely(a == std::numeric_limits::min() && b == -1)) + throw Exception(ErrorCodes::ILLEGAL_DIVISION, "Division of minimal signed number by minus one"); + } } template @@ -37,8 +40,11 @@ inline bool divisionLeadsToFPE(A a, B b) if (unlikely(b == 0)) return true; - if (unlikely(is_signed_v && is_signed_v && a == std::numeric_limits::min() && b == -1)) - return true; + if constexpr (is_signed_v && is_signed_v) + { + if (unlikely(a == std::numeric_limits::min() && b == -1)) + return true; + } return false; } diff --git a/src/Functions/FunctionBinaryArithmetic.h b/src/Functions/FunctionBinaryArithmetic.h index f99d477d24d..f67e971d4d6 100644 --- a/src/Functions/FunctionBinaryArithmetic.h +++ b/src/Functions/FunctionBinaryArithmetic.h @@ -284,7 +284,7 @@ struct BinaryOperation static ResultType process(A a, B b) { return Op::template apply(a, b); } - static ResultType process(A a, B b, NullMap::value_type * m [[maybe_unused]] = nullptr) { return Op::template apply(a, b, m); } + static ResultType process(A a, B b, NullMap::value_type * m [[maybe_unused]] = nullptr) { return Op::template apply(a, b); } private: template @@ -553,7 +553,7 @@ private: public: template static void NO_INLINE process(const auto & a, const auto & b, ResultContainerType & c, - NativeResultType scale_a, NativeResultType scale_b, const NullMap * right_nullmap = nullptr) + NativeResultType scale_a, NativeResultType scale_b, const NullMap * right_nullmap = nullptr, NullMap * res_nullmap = nullptr) { if constexpr (op_case == OpCase::LeftConstant) static_assert(!is_decimal); if constexpr (op_case == OpCase::RightConstant) static_assert(!is_decimal); @@ -609,7 +609,7 @@ public: } else if constexpr (is_division && is_decimal_b) { - processWithRightNullmapImpl(a, b, c, size, right_nullmap, [&scale_a](const auto & left, const auto & right) + processWithRightNullmapImpl(a, b, c, size, right_nullmap, res_nullmap, [&scale_a](const auto & left, const auto & right) { return applyScaledDiv( static_cast(left), right, scale_a); @@ -618,7 +618,7 @@ public: } processWithRightNullmapImpl( - a, b, c, size, right_nullmap, + a, b, c, size, right_nullmap, res_nullmap, [](const auto & left, const auto & right) { return apply( @@ -628,25 +628,42 @@ public: } template - static ResultType process(A a, B b, NativeResultType scale_a, NativeResultType scale_b) + static ResultType process(A a, B b, NativeResultType scale_a, NativeResultType scale_b, NullMap * res_nullmap) requires(!is_decimal && !is_decimal) { - if constexpr (is_division && is_decimal_b) - return applyScaledDiv(a, b, scale_a); - else if constexpr (is_plus_minus_compare) + try { - if (scale_a != 1) - return applyScaled(a, b, scale_a); - if (scale_b != 1) - return applyScaled(a, b, scale_b); - } + if constexpr (is_division && is_decimal_b) + return applyScaledDiv(a, b, scale_a); + else if constexpr (is_plus_minus_compare) + { + if (scale_a != 1) + return applyScaled(a, b, scale_a); + if (scale_b != 1) + return applyScaled(a, b, scale_b); + } - return apply(a, b); + ResultType res = apply(a, b); + if constexpr (std::is_floating_point_v) + { + if (!std::isfinite(res) && res_nullmap) + (*res_nullmap)[0] = 1; + } + return res; + } + catch (const std::exception&) + { + if (res_nullmap) + (*res_nullmap)[0] = 1; + else + throw; + return ResultType(); + } } private: template - static void processWithRightNullmapImpl(const auto & a, const auto & b, ResultContainerType & c, size_t size, const NullMap * right_nullmap, ApplyFunc apply_func) + static void processWithRightNullmapImpl(const auto & a, const auto & b, ResultContainerType & c, size_t size, const NullMap * right_nullmap, NullMap * res_nullmap, ApplyFunc apply_func) { if (right_nullmap) { @@ -660,7 +677,24 @@ private: } for (size_t i = 0; i < size; ++i) - c[i] = apply_func(undec(a[i]), undec(b)); + { + try + { + c[i] = apply_func(undec(a[i]), undec(b)); + if constexpr (std::is_floating_point_v) + { + if (!std::isfinite(c[i]) && res_nullmap) + (*res_nullmap)[i] = 1; + } + } + catch (const std::exception&) + { + if (res_nullmap) + (*res_nullmap)[i] = 1; + else + throw; + } + } } else { @@ -669,19 +703,55 @@ private: if ((*right_nullmap)[i]) c[i] = ResultType(); else - c[i] = apply_func(unwrap(a, i), undec(b[i])); + { + try + { + c[i] = apply_func(unwrap(a, i), undec(b[i])); + if constexpr (std::is_floating_point_v) + { + if (!std::isfinite(c[i]) && res_nullmap) + (*res_nullmap)[i] = 1; + } + } + catch (const std::exception&) + { + if (res_nullmap) + (*res_nullmap)[i] = 1; + else + throw; + } + } } } } else + { for (size_t i = 0; i < size; ++i) - c[i] = apply_func(unwrap(a, i), unwrap(b, i)); + { + try + { + c[i] = apply_func(unwrap(a, i), unwrap(b, i)); + if constexpr (std::is_floating_point_v) + { + if (!std::isfinite(c[i]) && res_nullmap) + (*res_nullmap)[i] = 1; + } + } + catch (const std::exception&) + { + if (res_nullmap) + (*res_nullmap)[i] = 1; + else + throw; + } + } + } } static constexpr bool is_plus_minus = IsOperation::plus || IsOperation::minus; static constexpr bool is_multiply = IsOperation::multiply; - static constexpr bool is_float_division = IsOperation::div_floating; + static constexpr bool is_float_division = IsOperation::div_floating || IsOperation::divide_or_null; static constexpr bool is_int_division = IsOperation::int_div || IsOperation::int_div_or_zero; static constexpr bool is_division = is_float_division || is_int_division; @@ -1346,12 +1416,12 @@ class FunctionBinaryArithmetic : public IFunction } template - void helperInvokeEither(const auto& left, const auto& right, auto& vec_res, auto scale_a, auto scale_b, const NullMap * right_nullmap) const + void helperInvokeEither(const auto& left, const auto& right, auto& vec_res, auto scale_a, auto scale_b, const NullMap * right_nullmap, NullMap * res_nullmap) const { if (check_decimal_overflow) - OpImplCheck::template process(left, right, vec_res, scale_a, scale_b, right_nullmap); + OpImplCheck::template process(left, right, vec_res, scale_a, scale_b, right_nullmap, res_nullmap); else - OpImpl::template process(left, right, vec_res, scale_a, scale_b, right_nullmap); + OpImpl::template process(left, right, vec_res, scale_a, scale_b, right_nullmap, res_nullmap); } template @@ -1359,7 +1429,7 @@ class FunctionBinaryArithmetic : public IFunction const auto & left, const auto & right, const ColumnConst * const col_left_const, const ColumnConst * const col_right_const, const auto * const col_left, const auto * const col_right, - size_t col_left_size, const NullMap * right_nullmap) const + size_t col_left_size, const NullMap * right_nullmap, NullMap * res_nullmap) const { using T0 = typename LeftDataType::FieldType; using T1 = typename RightDataType::FieldType; @@ -1414,8 +1484,8 @@ class FunctionBinaryArithmetic : public IFunction ResultType res = {}; if (!right_nullmap || !(*right_nullmap)[0]) res = check_decimal_overflow - ? OpImplCheck::template process(const_a, const_b, scale_a, scale_b) - : OpImpl::template process(const_a, const_b, scale_a, scale_b); + ? OpImplCheck::template process(const_a, const_b, scale_a, scale_b, res_nullmap) + : OpImpl::template process(const_a, const_b, scale_a, scale_b, res_nullmap); return ResultDataType(type.getPrecision(), type.getScale()) .createColumnConst(col_left_const->size(), toField(res, type.getScale())); @@ -1429,7 +1499,7 @@ class FunctionBinaryArithmetic : public IFunction if (col_left && col_right) { helperInvokeEither( - col_left->getData(), col_right->getData(), vec_res, scale_a, scale_b, right_nullmap); + col_left->getData(), col_right->getData(), vec_res, scale_a, scale_b, right_nullmap, res_nullmap); } else if (col_left_const && col_right) { @@ -1437,7 +1507,7 @@ class FunctionBinaryArithmetic : public IFunction helperGetOrConvert(col_left_const, left)); helperInvokeEither( - const_a, col_right->getData(), vec_res, scale_a, scale_b, right_nullmap); + const_a, col_right->getData(), vec_res, scale_a, scale_b, right_nullmap, res_nullmap); } else if (col_left && col_right_const) { @@ -1445,7 +1515,7 @@ class FunctionBinaryArithmetic : public IFunction helperGetOrConvert(col_right_const, right)); helperInvokeEither( - col_left->getData(), const_b, vec_res, scale_a, scale_b, right_nullmap); + col_left->getData(), const_b, vec_res, scale_a, scale_b, right_nullmap, res_nullmap); } else return nullptr; @@ -2098,7 +2168,8 @@ ColumnPtr executeStringInteger(const ColumnsWithTypeAndName & arguments, const A col_left_const, col_right_const, col_left, col_right, col_left_size, - right_nullmap); + right_nullmap, + res_nullmap); } /// Here we check if we have `intDiv` or `intDivOrZero` and at least one of the arguments is decimal, because in this case originally we had result as decimal, so we need to convert result into integer after calculations else if constexpr (!decimal_with_float && (is_int_div || is_int_div_or_zero) && (IsDataTypeDecimal || IsDataTypeDecimal)) @@ -2122,7 +2193,8 @@ ColumnPtr executeStringInteger(const ColumnsWithTypeAndName & arguments, const A col_left_const, col_right_const, col_left, col_right, col_left_size, - right_nullmap); + right_nullmap, + res_nullmap); auto col = ColumnWithTypeAndName(res, type_res, name); return castColumn(col, std::make_shared()); @@ -2253,10 +2325,9 @@ ColumnPtr executeStringInteger(const ColumnsWithTypeAndName & arguments, const A auto res = executeImpl2(createBlockWithNestedColumns(arguments), removeNullable(result_type), input_rows_count, &right_null_map, &res_null_map); return wrapInNullable(res, arguments, result_type, input_rows_count, &res_null_map); } - /// Process special case when operation is divideOrNull and moduloOrNull + /// Process special case when operation is divideOrNull and moduloOrNull which will return NULL divided zero. else if ((is_divide_or_null || is_modulo_or_null) && !res_nullmap) { - std::cerr << "gethere divornull or modornull and resnull map is null" << std::endl; NullMap res_null_map(input_rows_count, 0); auto res = executeImpl2(arguments, result_type, input_rows_count, nullptr, &res_null_map); return wrapInNullable(res, arguments, result_type, input_rows_count, &res_null_map); diff --git a/src/Functions/GCDLCMImpl.h b/src/Functions/GCDLCMImpl.h index fc5f387e02f..094c248497b 100644 --- a/src/Functions/GCDLCMImpl.h +++ b/src/Functions/GCDLCMImpl.h @@ -5,7 +5,6 @@ #include #include #include -#include #include "config.h" @@ -50,20 +49,6 @@ struct GCDLCMImpl return Impl::applyImpl(a, b); } - template - static Result apply(A a, B b, NullMap::value_type *m) - { - try - { - return apply(a, b); - } - catch (const std::exception&) - { - *m = 1; - return Result(); - } - } - #if USE_EMBEDDED_COMPILER static constexpr bool compilable = false; /// exceptions (and a non-trivial algorithm) #endif diff --git a/src/Functions/IsOperation.h b/src/Functions/IsOperation.h index 145de9814c6..2a9944ac78b 100644 --- a/src/Functions/IsOperation.h +++ b/src/Functions/IsOperation.h @@ -64,7 +64,7 @@ struct IsOperation static constexpr bool bit_hamming_distance = IsSameOperation::value; - static constexpr bool division = div_floating || int_div || int_div_or_zero || modulo || modulo_or_null; + static constexpr bool division = div_floating || int_div || int_div_or_zero || modulo || modulo_or_null || divide_or_null; // NOTE: allow_decimal should not fully contain `division` because of divInt static constexpr bool allow_decimal = plus || minus || multiply || division || least || greatest; }; diff --git a/src/Functions/bitAnd.cpp b/src/Functions/bitAnd.cpp index ff8525b1594..c6ab9023142 100644 --- a/src/Functions/bitAnd.cpp +++ b/src/Functions/bitAnd.cpp @@ -20,7 +20,7 @@ struct BitAndImpl static constexpr bool allow_string_integer = false; template - static Result apply(A a, B b, NullMap::value_type * m [[maybe_unused]] = nullptr) + static Result apply(A a, B b) { return static_cast(a) & static_cast(b); } diff --git a/src/Functions/bitBoolMaskAnd.cpp b/src/Functions/bitBoolMaskAnd.cpp index 482e1e808da..bd89b6eb69a 100644 --- a/src/Functions/bitBoolMaskAnd.cpp +++ b/src/Functions/bitBoolMaskAnd.cpp @@ -25,20 +25,12 @@ struct BitBoolMaskAndImpl static const constexpr bool allow_string_integer = false; template - static Result apply([[maybe_unused]] A left, [[maybe_unused]] B right, NullMap::value_type * m [[maybe_unused]] = nullptr) + static Result apply([[maybe_unused]] A left, [[maybe_unused]] B right) { // Should be a logical error, but this function is callable from SQL. // Need to investigate this. if constexpr (!std::is_same_v || !std::is_same_v) - { - if (!m) - throw DB::Exception(ErrorCodes::BAD_ARGUMENTS, "It's a bug! Only UInt8 type is supported by __bitBoolMaskAnd."); - else - { - *m = 1; - return Result(); - } - } + throw DB::Exception(ErrorCodes::BAD_ARGUMENTS, "It's a bug! Only UInt8 type is supported by __bitBoolMaskAnd."); auto left_bits = littleBits(left); auto right_bits = littleBits(right); diff --git a/src/Functions/bitBoolMaskOr.cpp b/src/Functions/bitBoolMaskOr.cpp index 79cdcd61c26..1ddf2d258f8 100644 --- a/src/Functions/bitBoolMaskOr.cpp +++ b/src/Functions/bitBoolMaskOr.cpp @@ -25,20 +25,12 @@ struct BitBoolMaskOrImpl static const constexpr bool allow_string_integer = false; template - static Result apply([[maybe_unused]] A left, [[maybe_unused]] B right, NullMap::value_type * m [[maybe_unused]] = nullptr) + static Result apply([[maybe_unused]] A left, [[maybe_unused]] B right) { if constexpr (!std::is_same_v || !std::is_same_v) - { - if (!m) // Should be a logical error, but this function is callable from SQL. // Need to investigate this. - throw DB::Exception(ErrorCodes::BAD_ARGUMENTS, "It's a bug! Only UInt8 type is supported by __bitBoolMaskOr."); - else - { - *m = 1; - return Result(); - } - } + throw DB::Exception(ErrorCodes::BAD_ARGUMENTS, "It's a bug! Only UInt8 type is supported by __bitBoolMaskOr."); auto left_bits = littleBits(left); auto right_bits = littleBits(right); diff --git a/src/Functions/bitHammingDistance.cpp b/src/Functions/bitHammingDistance.cpp index 5e51de6ec2a..f8a1a95ae14 100644 --- a/src/Functions/bitHammingDistance.cpp +++ b/src/Functions/bitHammingDistance.cpp @@ -1,6 +1,5 @@ #include #include -#include "Columns/ColumnNullable.h" #include @@ -20,7 +19,7 @@ struct BitHammingDistanceImpl static constexpr bool allow_string_integer = false; template - static NO_SANITIZE_UNDEFINED Result apply(A a, B b, NullMap::value_type * m [[maybe_unused]] = nullptr) + static NO_SANITIZE_UNDEFINED Result apply(A a, B b) { /// Note: it's unspecified if signed integers should be promoted with sign-extension or with zero-fill. /// This behavior can change in the future. @@ -40,15 +39,7 @@ struct BitHammingDistanceImpl return res; } else - { - if (!m) - throw Exception(ErrorCodes::ILLEGAL_COLUMN, "Unsupported data type combination in function 'bitHammingDistance'"); - else - { - *m = 1; - return Result(); - } - } + throw Exception(ErrorCodes::ILLEGAL_COLUMN, "Unsupported data type combination in function 'bitHammingDistance'"); } #if USE_EMBEDDED_COMPILER diff --git a/src/Functions/bitOr.cpp b/src/Functions/bitOr.cpp index cc192752ea0..22ce15d892d 100644 --- a/src/Functions/bitOr.cpp +++ b/src/Functions/bitOr.cpp @@ -19,7 +19,7 @@ struct BitOrImpl static constexpr bool allow_string_integer = false; template - static Result apply(A a, B b, NullMap::value_type * m [[maybe_unused]] = nullptr) + static Result apply(A a, B b) { return static_cast(a) | static_cast(b); } diff --git a/src/Functions/bitRotateLeft.cpp b/src/Functions/bitRotateLeft.cpp index 382fe9d9d12..6572d21c663 100644 --- a/src/Functions/bitRotateLeft.cpp +++ b/src/Functions/bitRotateLeft.cpp @@ -1,6 +1,5 @@ #include #include -#include "Columns/ColumnNullable.h" #include "Core/Types.h" namespace DB @@ -22,7 +21,7 @@ struct BitRotateLeftImpl static const constexpr bool allow_string_integer = false; template - static NO_SANITIZE_UNDEFINED Result apply(A a [[maybe_unused]], B b [[maybe_unused]], NullMap::value_type * m [[maybe_unused]] = nullptr) + static NO_SANITIZE_UNDEFINED Result apply(A a [[maybe_unused]], B b [[maybe_unused]]) { if constexpr (is_big_int_v || is_big_int_v) throw Exception(ErrorCodes::NOT_IMPLEMENTED, "Bit rotate is not implemented for big integers"); diff --git a/src/Functions/bitRotateRight.cpp b/src/Functions/bitRotateRight.cpp index c3fb7d85808..a2f0fe12324 100644 --- a/src/Functions/bitRotateRight.cpp +++ b/src/Functions/bitRotateRight.cpp @@ -1,6 +1,5 @@ #include #include -#include "Columns/ColumnNullable.h" namespace DB { @@ -21,7 +20,7 @@ struct BitRotateRightImpl static const constexpr bool allow_string_integer = false; template - static NO_SANITIZE_UNDEFINED Result apply(A a [[maybe_unused]], B b [[maybe_unused]], NullMap::value_type * m [[maybe_unused]] = nullptr) + static NO_SANITIZE_UNDEFINED Result apply(A a [[maybe_unused]], B b [[maybe_unused]]) { if constexpr (is_big_int_v || is_big_int_v) throw Exception(ErrorCodes::NOT_IMPLEMENTED, "Bit rotate is not implemented for big integers"); diff --git a/src/Functions/bitShiftLeft.cpp b/src/Functions/bitShiftLeft.cpp index df489ddf49c..0eb0d82ef0f 100644 --- a/src/Functions/bitShiftLeft.cpp +++ b/src/Functions/bitShiftLeft.cpp @@ -21,20 +21,12 @@ struct BitShiftLeftImpl static const constexpr bool allow_string_integer = true; template - static NO_SANITIZE_UNDEFINED Result apply(A a [[maybe_unused]], B b [[maybe_unused]], NullMap::value_type * m [[maybe_unused]] = nullptr) + static NO_SANITIZE_UNDEFINED Result apply(A a [[maybe_unused]], B b [[maybe_unused]]) { if constexpr (is_big_int_v) throw Exception(ErrorCodes::NOT_IMPLEMENTED, "BitShiftLeft is not implemented for big integers as second argument"); else if (b < 0 || static_cast(b) > 8 * sizeof(A)) - { - if (!m) - throw Exception(ErrorCodes::ARGUMENT_OUT_OF_BOUND, "The number of shift positions needs to be a non-negative value and less or equal to the bit width of the value to shift"); - else - { - *m = 1; - return Result(); - } - } + throw Exception(ErrorCodes::ARGUMENT_OUT_OF_BOUND, "The number of shift positions needs to be a non-negative value and less or equal to the bit width of the value to shift"); else if constexpr (is_big_int_v) return static_cast(a) << static_cast(b); else diff --git a/src/Functions/bitShiftRight.cpp b/src/Functions/bitShiftRight.cpp index 9a29e7510aa..16032b32f68 100644 --- a/src/Functions/bitShiftRight.cpp +++ b/src/Functions/bitShiftRight.cpp @@ -1,7 +1,6 @@ #include #include #include -#include "Columns/ColumnNullable.h" namespace DB { @@ -23,20 +22,12 @@ struct BitShiftRightImpl static const constexpr bool allow_string_integer = true; template - static NO_SANITIZE_UNDEFINED Result apply(A a [[maybe_unused]], B b [[maybe_unused]], NullMap::value_type * m [[maybe_unused]] = nullptr) + static NO_SANITIZE_UNDEFINED Result apply(A a [[maybe_unused]], B b [[maybe_unused]]) { if constexpr (is_big_int_v) throw Exception(ErrorCodes::NOT_IMPLEMENTED, "BitShiftRight is not implemented for big integers as second argument"); else if (b < 0 || static_cast(b) > 8 * sizeof(A)) - { - if (!m) throw Exception(ErrorCodes::ARGUMENT_OUT_OF_BOUND, "The number of shift positions needs to be a non-negative value and less or equal to the bit width of the value to shift"); - else - { - *m = 1; - return Result(); - } - } else if constexpr (is_big_int_v) return static_cast(a) >> static_cast(b); else diff --git a/src/Functions/bitTest.cpp b/src/Functions/bitTest.cpp index 6e9e4d54260..cb6b83c1cf1 100644 --- a/src/Functions/bitTest.cpp +++ b/src/Functions/bitTest.cpp @@ -1,7 +1,6 @@ #include #include #include -#include "Columns/ColumnNullable.h" namespace DB @@ -23,7 +22,7 @@ struct BitTestImpl static const constexpr bool allow_string_integer = false; template - static Result apply(A a [[maybe_unused]], B b [[maybe_unused]], NullMap::value_type * m [[maybe_unused]] = nullptr) + static Result apply(A a [[maybe_unused]], B b [[maybe_unused]]) { if constexpr (is_big_int_v || is_big_int_v) throw Exception(ErrorCodes::NOT_IMPLEMENTED, "bitTest is not implemented for big integers as second argument"); @@ -33,17 +32,9 @@ struct BitTestImpl typename NumberTraits::ToInteger::Type b_int = b; const auto max_position = static_cast((8 * sizeof(a)) - 1); if (b_int > max_position || b_int < 0) - { - if (!m) - throw Exception(ErrorCodes::PARAMETER_OUT_OF_BOUND, + throw Exception(ErrorCodes::PARAMETER_OUT_OF_BOUND, "The bit position argument needs to a positive value and less or equal to {} for integer {}", std::to_string(max_position), std::to_string(a_int)); - else - { - *m = 1; - return Result(); - } - } return (a_int >> b_int) & 1; } } diff --git a/src/Functions/bitXor.cpp b/src/Functions/bitXor.cpp index 9907346a430..43004c6f676 100644 --- a/src/Functions/bitXor.cpp +++ b/src/Functions/bitXor.cpp @@ -1,6 +1,5 @@ #include #include -#include "Columns/ColumnNullable.h" namespace DB { @@ -20,7 +19,7 @@ struct BitXorImpl static const constexpr bool allow_string_integer = false; template - static Result apply(A a, B b, NullMap::value_type * m [[maybe_unused]] = nullptr) + static Result apply(A a, B b) { return static_cast(a) ^ static_cast(b); } diff --git a/src/Functions/gcd.cpp b/src/Functions/gcd.cpp index 82dd7f35353..0cd017bb0b4 100644 --- a/src/Functions/gcd.cpp +++ b/src/Functions/gcd.cpp @@ -3,7 +3,6 @@ #include #include -#include "Columns/ColumnNullable.h" namespace DB @@ -19,7 +18,7 @@ struct GCDImpl : public GCDLCMImpl, NameGCD> { using ResultType = typename GCDLCMImpl::ResultType; - static ResultType applyImpl(A a, B b, NullMap::value_type * m [[maybe_unused]] = nullptr) + static ResultType applyImpl(A a, B b) { using Int = typename NumberTraits::ToInteger::Type; return boost::integer::gcd(Int(a), Int(b)); // NOLINT(clang-analyzer-core.UndefinedBinaryOperatorResult) diff --git a/src/Functions/greatest.cpp b/src/Functions/greatest.cpp index d071cac82dc..88539bda4a5 100644 --- a/src/Functions/greatest.cpp +++ b/src/Functions/greatest.cpp @@ -2,7 +2,6 @@ #include #include #include -#include "Columns/ColumnNullable.h" namespace DB @@ -16,7 +15,7 @@ struct GreatestBaseImpl static const constexpr bool allow_string_integer = false; template - static Result apply(A a, B b, NullMap::value_type * m [[maybe_unused]] = nullptr) + static Result apply(A a, B b) { return static_cast(a) > static_cast(b) ? static_cast(a) : static_cast(b); @@ -47,7 +46,7 @@ struct GreatestSpecialImpl static const constexpr bool allow_string_integer = false; template - static Result apply(A a, B b, NullMap::value_type * m [[maybe_unused]] = nullptr) + static Result apply(A a, B b) { static_assert(std::is_same_v, "ResultType != Result"); return accurate::greaterOp(a, b) ? static_cast(a) : static_cast(b); diff --git a/src/Functions/intDiv.cpp b/src/Functions/intDiv.cpp index d812406a6e8..7d38ed50696 100644 --- a/src/Functions/intDiv.cpp +++ b/src/Functions/intDiv.cpp @@ -1,7 +1,6 @@ #include #include -#include "Columns/ColumnNullable.h" #include "divide/divide.h" @@ -27,7 +26,7 @@ struct DivideIntegralByConstantImpl static const constexpr bool allow_string_integer = false; template - static void NO_INLINE process(const A * __restrict a, const B * __restrict b, ResultType * __restrict c, size_t size, const NullMap * right_nullmap, NullMap * res_nullmap) + static void NO_INLINE process(const A * __restrict a, const B * __restrict b, ResultType * __restrict c, size_t size, const NullMap * right_nullmap, NullMap * res_nullmap[[maybe_unused]]) { if constexpr (op_case == OpCase::RightConstant) { @@ -40,20 +39,19 @@ struct DivideIntegralByConstantImpl { if (right_nullmap) { - assert(res_nullmap); for (size_t i = 0; i < size; ++i) if ((*right_nullmap)[i]) c[i] = ResultType(); else - apply(a, b, c, i, &((*res_nullmap)[i])); + apply(a, b, c, i); } else for (size_t i = 0; i < size; ++i) - apply(a, b, c, i); + apply(a, b, c, i); } } - static ResultType process(A a, B b, NullMap::value_type * m = nullptr) { return Op::template apply(a, b, m); } + static ResultType process(A a, B b, NullMap::value_type * m [[maybe_unused]] = nullptr) { return Op::template apply(a, b); } static void NO_INLINE NO_SANITIZE_UNDEFINED vectorConstant(const A * __restrict a_pos, B b, ResultType * __restrict c_pos, size_t size) { @@ -81,23 +79,13 @@ struct DivideIntegralByConstantImpl } private: - template - static void apply(const A * __restrict a, const B * __restrict b, ResultType * __restrict c, size_t i, NullMap::value_type * m = nullptr) + template + static void apply(const A * __restrict a, const B * __restrict b, ResultType * __restrict c, size_t i) { - if constexpr (nullable) - { - if constexpr (op_case == OpCase::Vector) - c[i] = Op::template apply(a[i], b[i], m); - else - c[i] = Op::template apply(*a, b[i], m); - } + if constexpr (op_case == OpCase::Vector) + c[i] = Op::template apply(a[i], b[i]); else - { - if constexpr (op_case == OpCase::Vector) - c[i] = Op::template apply(a[i], b[i]); - else - c[i] = Op::template apply(*a, b[i]); - } + c[i] = Op::template apply(*a, b[i]); } }; diff --git a/src/Functions/intDivOrZero.cpp b/src/Functions/intDivOrZero.cpp index bee82f33be4..f32eac17127 100644 --- a/src/Functions/intDivOrZero.cpp +++ b/src/Functions/intDivOrZero.cpp @@ -13,7 +13,7 @@ struct DivideIntegralOrZeroImpl static const constexpr bool allow_string_integer = false; template - static Result apply(A a, B b, NullMap::value_type * m [[maybe_unused]] = nullptr) + static Result apply(A a, B b) { if (unlikely(divisionLeadsToFPE(a, b))) return 0; diff --git a/src/Functions/lcm.cpp b/src/Functions/lcm.cpp index 804f31256ad..495199fa964 100644 --- a/src/Functions/lcm.cpp +++ b/src/Functions/lcm.cpp @@ -3,7 +3,6 @@ #include #include -#include "Columns/ColumnNullable.h" namespace @@ -38,7 +37,7 @@ struct LCMImpl : public GCDLCMImpl, NameLCM> { using ResultType = typename GCDLCMImpl, NameLCM>::ResultType; - static ResultType applyImpl(A a, B b, NullMap::value_type * m [[maybe_unused]] = nullptr) + static ResultType applyImpl(A a, B b) { using Int = typename NumberTraits::ToInteger::Type; using Unsigned = make_unsigned_t; diff --git a/src/Functions/least.cpp b/src/Functions/least.cpp index 1a14019adca..091a868e8e2 100644 --- a/src/Functions/least.cpp +++ b/src/Functions/least.cpp @@ -2,7 +2,6 @@ #include #include #include -#include "Columns/ColumnNullable.h" namespace DB @@ -22,13 +21,6 @@ struct LeastBaseImpl return static_cast(a) < static_cast(b) ? static_cast(a) : static_cast(b); } - template - static Result apply(A a, B b, NullMap::value_type *) - { - /** gcc 4.9.2 successfully vectorizes a loop from this function. */ - return static_cast(a) < static_cast(b) ? static_cast(a) : static_cast(b); - } - #if USE_EMBEDDED_COMPILER static constexpr bool compilable = true; @@ -54,7 +46,7 @@ struct LeastSpecialImpl static const constexpr bool allow_string_integer = false; template - static Result apply(A a, B b, NullMap::value_type * m [[maybe_unused]] = nullptr) + static Result apply(A a, B b) { static_assert(std::is_same_v, "ResultType != Result"); return accurate::lessOp(a, b) ? static_cast(a) : static_cast(b); diff --git a/src/Functions/minus.cpp b/src/Functions/minus.cpp index 8db41e9ac4c..f3b9b8a7bcb 100644 --- a/src/Functions/minus.cpp +++ b/src/Functions/minus.cpp @@ -1,7 +1,6 @@ #include #include #include -#include "Columns/ColumnNullable.h" namespace DB { @@ -14,7 +13,7 @@ struct MinusImpl static const constexpr bool allow_string_integer = false; template - static NO_SANITIZE_UNDEFINED Result apply(A a, B b, NullMap::value_type * m [[maybe_unused]] = nullptr) + static NO_SANITIZE_UNDEFINED Result apply(A a, B b) { if constexpr (is_big_int_v || is_big_int_v) { diff --git a/src/Functions/modulo.cpp b/src/Functions/modulo.cpp index fd53b20c156..5b444fc9cbf 100644 --- a/src/Functions/modulo.cpp +++ b/src/Functions/modulo.cpp @@ -2,7 +2,6 @@ #include #include -#include "Columns/ColumnNullable.h" #include @@ -28,7 +27,7 @@ struct ModuloByConstantImpl static const constexpr bool allow_string_integer = false; template - static void NO_INLINE process(const A * __restrict a, const B * __restrict b, ResultType * __restrict c, size_t size, const NullMap * right_nullmap, NullMap * res_nullmap) + static void NO_INLINE process(const A * __restrict a, const B * __restrict b, ResultType * __restrict c, size_t size, const NullMap * right_nullmap, NullMap * res_nullmap [[maybe_unused]] = nullptr) { if constexpr (op_case == OpCase::RightConstant) { @@ -44,20 +43,29 @@ struct ModuloByConstantImpl if ((*right_nullmap)[i]) c[i] = ResultType(); else - apply(a, b, c, i, &((*res_nullmap)[i])); + apply(a, b, c, i); } else for (size_t i = 0; i < size; ++i) - apply(a, b, c, i); + apply(a, b, c, i); } } - static ResultType process(A a, B b, NullMap::value_type * m = nullptr) { return Op::template apply(a, b, m); } + static ResultType process(A a, B b, NullMap::value_type * res_nullmap [[maybe_unused]] = nullptr) { return Op::template apply(a, b); } static void NO_INLINE NO_SANITIZE_UNDEFINED vectorConstant(const A * __restrict src, B b, ResultType * __restrict dst, size_t size) { /// Modulo with too small divisor. - if (unlikely((std::is_signed_v && b == -1) || b == 1)) + if constexpr (std::is_signed_v) + { + if (unlikely((b == -1))) + { + for (size_t i = 0; i < size; ++i) + dst[i] = 0; + return; + } + } + if (b == 1) { for (size_t i = 0; i < size; ++i) dst[i] = 0; @@ -105,24 +113,13 @@ struct ModuloByConstantImpl } private: - template - static void apply(const A * __restrict a, const B * __restrict b, ResultType * __restrict c, size_t i, NullMap::value_type * m = nullptr) + template + static void apply(const A * __restrict a, const B * __restrict b, ResultType * __restrict c, size_t i) { - if constexpr (nullable) - { - if constexpr (op_case == OpCase::Vector) - c[i] = Op::template apply(a[i], b[i], m); - else - c[i] = Op::template apply(*a, b[i], m); - } + if constexpr (op_case == OpCase::Vector) + c[i] = Op::template apply(a[i], b[i]); else - { - if constexpr (op_case == OpCase::Vector) - c[i] = Op::template apply(a[i], b[i]); - else - c[i] = Op::template apply(*a, b[i]); - - } + c[i] = Op::template apply(*a, b[i]); } }; diff --git a/src/Functions/moduloOrNull.cpp b/src/Functions/moduloOrNull.cpp index 1008c1ee98b..46f68e56c5c 100644 --- a/src/Functions/moduloOrNull.cpp +++ b/src/Functions/moduloOrNull.cpp @@ -4,6 +4,7 @@ #include #include "Common/StackTrace.h" #include +#include "Functions/StringHelpers.h" #include "base/extended_types.h" #include @@ -46,11 +47,21 @@ struct ModuloOrNullImpl try { res = Op::template apply(a, b); + if constexpr (std::is_floating_point_v) + { + if (!std::isfinite(res) && m) + *m = 1; + } } catch (const std::exception&) { if (m) *m = 1; + else + { + std::cerr << "gethere 1" << std::endl; + throw; + } } return res; } @@ -58,7 +69,16 @@ struct ModuloOrNullImpl static void NO_INLINE NO_SANITIZE_UNDEFINED vectorConstant(const A * __restrict src, B b, ResultType * __restrict dst, size_t size, NullMap * res_nullmap) { /// Modulo with too small divisor. - if (unlikely((std::is_signed_v && b == -1) || b == 1)) + if constexpr (std::is_signed_v) + { + if (unlikely((b == -1))) + { + for (size_t i = 0; i < size; ++i) + dst[i] = 0; + return; + } + } + if (b == 1) { for (size_t i = 0; i < size; ++i) dst[i] = 0; @@ -120,7 +140,7 @@ struct ModuloOrNullImpl if constexpr (std::is_floating_point_v) { if (!std::isfinite(res) && m) - * m = 1; + *m = 1; } } catch (const std::exception&) @@ -128,6 +148,11 @@ struct ModuloOrNullImpl std::cerr <<"gethere 6 exception, m vallue:" << !!m << std::endl; std::cerr << StackTrace().toString() << std::endl; if (m) *m = 1; + else + { + std::cerr << "gethere 7" << std::endl; + throw; + } } return res; } diff --git a/src/Functions/moduloOrZero.cpp b/src/Functions/moduloOrZero.cpp index 60efaedc222..a3b67f15f8f 100644 --- a/src/Functions/moduloOrZero.cpp +++ b/src/Functions/moduloOrZero.cpp @@ -1,6 +1,5 @@ #include #include -#include "Columns/ColumnNullable.h" namespace DB { @@ -15,7 +14,7 @@ struct ModuloOrZeroImpl static const constexpr bool allow_string_integer = false; template - static Result apply(A a, B b, NullMap::value_type * m [[maybe_unused]] = nullptr) + static Result apply(A a, B b) { if constexpr (std::is_floating_point_v) { diff --git a/src/Functions/multiply.cpp b/src/Functions/multiply.cpp index 4c37c8641c9..67b6fff6b58 100644 --- a/src/Functions/multiply.cpp +++ b/src/Functions/multiply.cpp @@ -2,7 +2,6 @@ #include #include #include -#include "Columns/ColumnNullable.h" namespace DB { @@ -15,7 +14,7 @@ struct MultiplyImpl static const constexpr bool allow_string_integer = false; template - static NO_SANITIZE_UNDEFINED Result apply(A a, B b, NullMap::value_type * m [[maybe_unused]] = nullptr) + static NO_SANITIZE_UNDEFINED Result apply(A a, B b) { if constexpr (is_big_int_v || is_big_int_v) { diff --git a/src/Functions/plus.cpp b/src/Functions/plus.cpp index 3781914eb66..ffb0fe2ade7 100644 --- a/src/Functions/plus.cpp +++ b/src/Functions/plus.cpp @@ -1,7 +1,6 @@ #include #include #include -#include "Columns/ColumnNullable.h" namespace DB { @@ -15,7 +14,7 @@ struct PlusImpl static const constexpr bool is_commutative = true; template - static NO_SANITIZE_UNDEFINED Result apply(A a, B b, NullMap::value_type * m [[maybe_unused]] = nullptr) + static NO_SANITIZE_UNDEFINED Result apply(A a, B b) { /// Next everywhere, static_cast - so that there is no wrong result in expressions of the form Int64 c = UInt32(a) * Int32(-1). if constexpr (is_big_int_v || is_big_int_v) diff --git a/src/Functions/vectorFunctions.cpp b/src/Functions/vectorFunctions.cpp index 5e23493c86d..bd5d443cfd4 100644 --- a/src/Functions/vectorFunctions.cpp +++ b/src/Functions/vectorFunctions.cpp @@ -54,7 +54,9 @@ struct PlusName { static constexpr auto name = "plus"; }; struct MinusName { static constexpr auto name = "minus"; }; struct MultiplyName { static constexpr auto name = "multiply"; }; struct DivideName { static constexpr auto name = "divide"; }; +struct DivideOrNullName { static constexpr auto name = "divideOrNull"; }; struct ModuloName { static constexpr auto name = "modulo"; }; +struct ModuloOrNullName { static constexpr auto name = "moduloOrNull"; }; struct IntDivName { static constexpr auto name = "intDiv"; }; struct IntDivOrZeroName { static constexpr auto name = "intDivOrZero"; }; @@ -152,8 +154,12 @@ using FunctionTupleMultiply = FunctionTupleOperator; using FunctionTupleDivide = FunctionTupleOperator; +using FunctionTupleDivideOrNull = FunctionTupleOperator; + using FunctionTupleModulo = FunctionTupleOperator; +using FunctionTupleModuloOrNull = FunctionTupleOperator; + using FunctionTupleIntDiv = FunctionTupleOperator; using FunctionTupleIntDivOrZero = FunctionTupleOperator; @@ -307,12 +313,16 @@ using FunctionTupleMultiplyByNumber = FunctionTupleOperatorByNumber; +using FunctionTupleDivideOrNullByNumber = FunctionTupleOperatorByNumber; + using FunctionTupleModuloByNumber = FunctionTupleOperatorByNumber; using FunctionTupleIntDivByNumber = FunctionTupleOperatorByNumber; using FunctionTupleIntDivOrZeroByNumber = FunctionTupleOperatorByNumber; +using FunctionTupleModuloOrNullByNumber = FunctionTupleOperatorByNumber; + class FunctionDotProduct : public ITupleFunction { public: @@ -1581,7 +1591,9 @@ REGISTER_FUNCTION(VectorFunctions) factory.registerAlias("vectorDifference", FunctionTupleMinus::name, FunctionFactory::Case::Insensitive); factory.registerFunction(); factory.registerFunction(); + factory.registerFunction(); factory.registerFunction(); + factory.registerFunction(); factory.registerFunction(); factory.registerFunction(); factory.registerFunction(); @@ -1647,7 +1659,9 @@ If the types of the first interval (or the interval in the tuple) and the second factory.registerFunction(); factory.registerFunction(); + factory.registerFunction(); factory.registerFunction(); + factory.registerFunction(); factory.registerFunction(); factory.registerFunction(); diff --git a/tests/queries/0_stateless/03224_modulo_or_null.reference b/tests/queries/0_stateless/03224_modulo_or_null.reference index afce7587508..05d1d4e5939 100644 --- a/tests/queries/0_stateless/03224_modulo_or_null.reference +++ b/tests/queries/0_stateless/03224_modulo_or_null.reference @@ -20,3 +20,13 @@ Nullable(UInt8) \N \N \N +\N +\N +\N +\N +\N +\N +(NULL,1,1) +(0,1,1) +(NULL,NULL,NULL) +(1,0,1) diff --git a/tests/queries/0_stateless/03224_modulo_or_null.sql b/tests/queries/0_stateless/03224_modulo_or_null.sql index fbe511ffd93..1681bb25036 100644 --- a/tests/queries/0_stateless/03224_modulo_or_null.sql +++ b/tests/queries/0_stateless/03224_modulo_or_null.sql @@ -9,14 +9,14 @@ select moduloOrNull(1, materialize(0)); select moduloOrNull(materialize(1), 0); select moduloOrNull(materialize(1), materialize(0)); -select moduloOrNull(1, toNullable(materialize(toUInt64(0)))); +select moduloOrNull(1.1, toNullable(materialize(toUInt64(0)))); select moduloOrNull(materialize(1), toNullable(materialize(toUInt64(0)))); select moduloOrNull(toNullable(materialize(1)), toNullable(materialize(toUInt64(0)))); select moduloOrNull(toNullable(materialize(toFloat32(1))), toNullable(materialize(toInt64(0)))); -select moduloOrNull(1, toNullable(materialize(toInt128(0)))); +select moduloOrNull(1.1, toNullable(materialize(toInt128(0)))); select moduloOrNull(toNullable(materialize(toFloat64(1))), toNullable(materialize(toInt128(0)))); select moduloOrNull(toNullable(materialize(toFloat64(1))), toNullable(materialize(toInt256(0)))); -select moduloOrNull(1, toNullable(materialize(toInt256(0)))); +select moduloOrNull(1.0, toNullable(materialize(toInt256(0)))); SELECT moduloOrNull(toNullable(materialize(1)), toNullable(materialize(0))); SELECT moduloOrNull(toNullable(materialize(toFloat32(1))), toNullable(materialize(0))); @@ -24,3 +24,16 @@ SELECT moduloOrNull(toNullable(materialize(toFloat32(1))), materialize(0)); SELECT moduloOrNull(toNullable(materialize(toFloat32(1))), toNullable(0)); SELECT moduloOrNull(materialize(1), CAST(materialize(NULL), 'Nullable(Float32)')); + +SELECT moduloOrNull(toDecimal32(16.2, 2), 0.0); +SELECT moduloOrNull(toDecimal32(16.2, 2), toDecimal32(0.0, 2)); + +SELECT moduloOrNull((16.2), 0.0); +SELECT moduloOrNull(materialize(16.2), 0.0); +SELECT moduloOrNull(16.2, materialize(0.0)); +SELECT moduloOrNull(materialize(16.2), materialize(0.0)); + +SELECT tupleModuloOrNull((15, 10, 5), (0, 3, 2)); +SELECT tupleModuloOrNull((15, 10, 5), (5, 3, 2)); +SELECT tupleModuloOrNullByNumber((15, 10, 5), 0); +SELECT tupleModuloOrNullByNumber((15, 10, 5), 2); \ No newline at end of file diff --git a/tests/queries/0_stateless/03225_divide_or_null.reference b/tests/queries/0_stateless/03225_divide_or_null.reference index 69dc10080ae..4ac45b506a7 100644 --- a/tests/queries/0_stateless/03225_divide_or_null.reference +++ b/tests/queries/0_stateless/03225_divide_or_null.reference @@ -17,3 +17,14 @@ 1.7777777777777777 1.7777777777777777 1.7777777777777777 +\N +\N +\N +\N +\N +\N +\N +(NULL,NULL,NULL) +(3,NULL,NULL) +(3,2,1) +(NULL,NULL,NULL) diff --git a/tests/queries/0_stateless/03225_divide_or_null.sql b/tests/queries/0_stateless/03225_divide_or_null.sql index 4e320351402..dce50079dd9 100644 --- a/tests/queries/0_stateless/03225_divide_or_null.sql +++ b/tests/queries/0_stateless/03225_divide_or_null.sql @@ -22,3 +22,17 @@ SELECT divideOrNull(CAST(16, 'Int8'), CAST(materialize(9), 'Nullable(Int128)')); SELECT divideOrNull(CAST(16, 'Int128'), CAST(materialize(9), 'Nullable(Int128)')); SELECT divideOrNull(CAST(16, 'UInt256'), CAST(materialize(9), 'Nullable(UInt128)')); SELECT divideOrNull(CAST(16, 'UInt256'), CAST(materialize(9), 'Nullable(UInt128)')); + +SELECT divideOrNull(toDecimal32(16.2, 2), toDecimal32(0.0, 1)); +SELECT divideOrNull(toDecimal32(16.2, 2), materialize(toDecimal32(0.0, 1))); +SELECT divideOrNull(materialize(toDecimal32(16.2, 2)), toDecimal32(0.0, 1)); +SELECT divideOrNull(materialize(toDecimal32(16.2, 2)), materialize(toDecimal32(0.0, 1))); + +SELECT divideOrNull(toDecimal32(16.2, 2), 0.0); +SELECT divideOrNull(toDecimal32(16.2, 2), materialize(0.0)); +SELECT divideOrNull(materialize(toDecimal32(16.2, 2)), materialize(0.0)); + +SELECT tupleDivideOrNull((15, 10, 5), (0, 0, 0)); +SELECT tupleDivideOrNull((15, 10, 5), (5, 0, 0)); +SELECT tupleDivideOrNullByNumber((15, 10, 5), 5); +SELECT tupleDivideOrNullByNumber((15, 10, 5), 0);