From fb552dd2c054f629ac12e32b9eef49109aba6765 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Mar=C3=ADn?= Date: Mon, 18 Nov 2024 13:49:11 +0100 Subject: [PATCH 01/18] Remove unused trash --- src/Columns/ColumnDecimal.h | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/Columns/ColumnDecimal.h b/src/Columns/ColumnDecimal.h index 6f8360a54dd..924a5bdf0b2 100644 --- a/src/Columns/ColumnDecimal.h +++ b/src/Columns/ColumnDecimal.h @@ -3,7 +3,6 @@ #include #include #include -#include #include #include #include @@ -98,8 +97,6 @@ public: return StringRef(reinterpret_cast(&data[n]), sizeof(data[n])); } - Float64 getFloat64(size_t n) const final { return DecimalUtils::convertTo(data[n], scale); } - const char * deserializeAndInsertFromArena(const char * pos) override; const char * skipSerializedInArena(const char * pos) const override; void updateHashWithValue(size_t n, SipHash & hash) const override; From ec776fe8db847b40e7ae079f6e2a51e954be917d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Mar=C3=ADn?= Date: Mon, 18 Nov 2024 17:08:08 +0100 Subject: [PATCH 02/18] Remove wasteful template instatiations --- src/Core/callOnTypeIndex.h | 71 +++++++++++++++++++++++++++++ src/Functions/FunctionsComparison.h | 40 +++++++++++++++- 2 files changed, 110 insertions(+), 1 deletion(-) diff --git a/src/Core/callOnTypeIndex.h b/src/Core/callOnTypeIndex.h index 0c8f2201b0d..09fbc7f1f10 100644 --- a/src/Core/callOnTypeIndex.h +++ b/src/Core/callOnTypeIndex.h @@ -87,6 +87,77 @@ static bool callOnBasicType(TypeIndex number, F && f) return false; } + +template +static bool callOnBasicTypeSecondArg(TypeIndex number, F && f) +{ + if constexpr (_int) + { + switch (number) + { + case TypeIndex::UInt8: return f(TypePair()); + case TypeIndex::UInt16: return f(TypePair()); + case TypeIndex::UInt32: return f(TypePair()); + case TypeIndex::UInt64: return f(TypePair()); + case TypeIndex::UInt128: return f(TypePair()); + case TypeIndex::UInt256: return f(TypePair()); + + case TypeIndex::Int8: return f(TypePair()); + case TypeIndex::Int16: return f(TypePair()); + case TypeIndex::Int32: return f(TypePair()); + case TypeIndex::Int64: return f(TypePair()); + case TypeIndex::Int128: return f(TypePair()); + case TypeIndex::Int256: return f(TypePair()); + + case TypeIndex::Enum8: return f(TypePair()); + case TypeIndex::Enum16: return f(TypePair()); + + default: + break; + } + } + + if constexpr (_decimal) + { + switch (number) + { + case TypeIndex::Decimal32: return f(TypePair()); + case TypeIndex::Decimal64: return f(TypePair()); + case TypeIndex::Decimal128: return f(TypePair()); + case TypeIndex::Decimal256: return f(TypePair()); + default: + break; + } + } + + if constexpr (_float) + { + switch (number) + { + case TypeIndex::BFloat16: return f(TypePair()); + case TypeIndex::Float32: return f(TypePair()); + case TypeIndex::Float64: return f(TypePair()); + default: + break; + } + } + + if constexpr (_datetime) + { + switch (number) + { + case TypeIndex::Date: return f(TypePair()); + case TypeIndex::Date32: return f(TypePair()); + case TypeIndex::DateTime: return f(TypePair()); + case TypeIndex::DateTime64: return f(TypePair()); + default: + break; + } + } + + return false; +} + /// Unroll template using TypeIndex template static inline bool callOnBasicTypes(TypeIndex type_num1, TypeIndex type_num2, F && f) diff --git a/src/Functions/FunctionsComparison.h b/src/Functions/FunctionsComparison.h index c9d3ccb4445..e6133bcd773 100644 --- a/src/Functions/FunctionsComparison.h +++ b/src/Functions/FunctionsComparison.h @@ -50,6 +50,44 @@ namespace DB { +namespace +{ +template +static inline bool callOnAtLeastOneDecimalType(TypeIndex type_num1, TypeIndex type_num2, F && f) +{ + switch (type_num1) + { + case TypeIndex::Decimal32: + return callOnBasicType(type_num2, std::forward(f)); + case TypeIndex::Decimal64: + return callOnBasicType(type_num2, std::forward(f)); + case TypeIndex::Decimal128: + return callOnBasicType(type_num2, std::forward(f)); + case TypeIndex::Decimal256: + return callOnBasicType(type_num2, std::forward(f)); + default: + break; + } + + switch (type_num2) + { + case TypeIndex::Decimal32: + return callOnBasicTypeSecondArg(type_num1, std::forward(f)); + case TypeIndex::Decimal64: + return callOnBasicTypeSecondArg(type_num1, std::forward(f)); + case TypeIndex::Decimal128: + return callOnBasicTypeSecondArg(type_num1, std::forward(f)); + case TypeIndex::Decimal256: + return callOnBasicTypeSecondArg(type_num1, std::forward(f)); + default: + break; + } + + return false; +} +} + + namespace ErrorCodes { extern const int ILLEGAL_COLUMN; @@ -770,7 +808,7 @@ private: return (res = DecimalComparison::apply(col_left, col_right)) != nullptr; }; - if (!callOnBasicTypes(left_number, right_number, call)) + if (!callOnAtLeastOneDecimalType(left_number, right_number, call)) throw Exception(ErrorCodes::LOGICAL_ERROR, "Wrong call for {} with {} and {}", getName(), col_left.type->getName(), col_right.type->getName()); From 1c308f970b92d9c9c48404364cce713353aca916 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Mar=C3=ADn?= Date: Mon, 18 Nov 2024 17:21:06 +0100 Subject: [PATCH 03/18] Try to remove more decimal instantiations --- src/Core/DecimalComparison.h | 133 +++++++++++++--------------- src/Core/Field.cpp | 9 +- src/Formats/ProtobufSerializer.cpp | 3 +- src/Functions/FunctionsComparison.h | 5 +- 4 files changed, 74 insertions(+), 76 deletions(-) diff --git a/src/Core/DecimalComparison.h b/src/Core/DecimalComparison.h index 77402adf164..7311105f6fe 100644 --- a/src/Core/DecimalComparison.h +++ b/src/Core/DecimalComparison.h @@ -52,8 +52,8 @@ struct DecCompareInt using TypeB = Type; }; -template typename Operation, bool _check_overflow = true, - bool _actual = is_decimal || is_decimal> +template typename Operation> +requires is_decimal || is_decimal class DecimalComparison { public: @@ -65,20 +65,17 @@ public: using ArrayA = typename ColVecA::Container; using ArrayB = typename ColVecB::Container; - static ColumnPtr apply(const ColumnWithTypeAndName & col_left, const ColumnWithTypeAndName & col_right) + static ColumnPtr apply(const ColumnWithTypeAndName & col_left, const ColumnWithTypeAndName & col_right, bool check_overflow) { - if constexpr (_actual) - { - ColumnPtr c_res; - Shift shift = getScales(col_left.type, col_right.type); + ColumnPtr c_res; + Shift shift = getScales(col_left.type, col_right.type); - return applyWithScale(col_left.column, col_right.column, shift); - } - else - return nullptr; + if (check_overflow) + return applyWithScale(col_left.column, col_right.column, shift); + return applyWithScale(col_left.column, col_right.column, shift); } - static bool compare(A a, B b, UInt32 scale_a, UInt32 scale_b) + static bool compare(A a, B b, UInt32 scale_a, UInt32 scale_b, bool check_overflow) { static const UInt32 max_scale = DecimalUtils::max_precision; if (scale_a > max_scale || scale_b > max_scale) @@ -90,7 +87,9 @@ public: if (scale_a > scale_b) shift.b = static_cast(DecimalUtils::scaleMultiplier(scale_a - scale_b)); - return applyWithScale(a, b, shift); + if (check_overflow) + return applyWithScale(a, b, shift); + return applyWithScale(a, b, shift); } private: @@ -104,14 +103,14 @@ private: bool right() const { return b != 1; } }; - template + template static auto applyWithScale(T a, U b, const Shift & shift) { if (shift.left()) - return apply(a, b, shift.a); + return apply(a, b, shift.a); if (shift.right()) - return apply(a, b, shift.b); - return apply(a, b, 1); + return apply(a, b, shift.b); + return apply(a, b, 1); } template @@ -158,66 +157,63 @@ private: return shift; } - template + template static ColumnPtr apply(const ColumnPtr & c0, const ColumnPtr & c1, CompareInt scale) { auto c_res = ColumnUInt8::create(); - if constexpr (_actual) + bool c0_is_const = isColumnConst(*c0); + bool c1_is_const = isColumnConst(*c1); + + if (c0_is_const && c1_is_const) { - bool c0_is_const = isColumnConst(*c0); - bool c1_is_const = isColumnConst(*c1); + const ColumnConst & c0_const = checkAndGetColumnConst(*c0); + const ColumnConst & c1_const = checkAndGetColumnConst(*c1); - if (c0_is_const && c1_is_const) + A a = c0_const.template getValue(); + B b = c1_const.template getValue(); + UInt8 res = apply(a, b, scale); + return DataTypeUInt8().createColumnConst(c0->size(), toField(res)); + } + + ColumnUInt8::Container & vec_res = c_res->getData(); + vec_res.resize(c0->size()); + + if (c0_is_const) + { + const ColumnConst & c0_const = checkAndGetColumnConst(*c0); + A a = c0_const.template getValue(); + if (const ColVecB * c1_vec = checkAndGetColumn(c1.get())) + constantVector(a, c1_vec->getData(), vec_res, scale); + else + throw Exception(ErrorCodes::LOGICAL_ERROR, "Wrong column in Decimal comparison"); + } + else if (c1_is_const) + { + const ColumnConst & c1_const = checkAndGetColumnConst(*c1); + B b = c1_const.template getValue(); + if (const ColVecA * c0_vec = checkAndGetColumn(c0.get())) + vectorConstant(c0_vec->getData(), b, vec_res, scale); + else + throw Exception(ErrorCodes::LOGICAL_ERROR, "Wrong column in Decimal comparison"); + } + else + { + if (const ColVecA * c0_vec = checkAndGetColumn(c0.get())) { - const ColumnConst & c0_const = checkAndGetColumnConst(*c0); - const ColumnConst & c1_const = checkAndGetColumnConst(*c1); - - A a = c0_const.template getValue(); - B b = c1_const.template getValue(); - UInt8 res = apply(a, b, scale); - return DataTypeUInt8().createColumnConst(c0->size(), toField(res)); - } - - ColumnUInt8::Container & vec_res = c_res->getData(); - vec_res.resize(c0->size()); - - if (c0_is_const) - { - const ColumnConst & c0_const = checkAndGetColumnConst(*c0); - A a = c0_const.template getValue(); if (const ColVecB * c1_vec = checkAndGetColumn(c1.get())) - constantVector(a, c1_vec->getData(), vec_res, scale); - else - throw Exception(ErrorCodes::LOGICAL_ERROR, "Wrong column in Decimal comparison"); - } - else if (c1_is_const) - { - const ColumnConst & c1_const = checkAndGetColumnConst(*c1); - B b = c1_const.template getValue(); - if (const ColVecA * c0_vec = checkAndGetColumn(c0.get())) - vectorConstant(c0_vec->getData(), b, vec_res, scale); + vectorVector(c0_vec->getData(), c1_vec->getData(), vec_res, scale); else throw Exception(ErrorCodes::LOGICAL_ERROR, "Wrong column in Decimal comparison"); } else - { - if (const ColVecA * c0_vec = checkAndGetColumn(c0.get())) - { - if (const ColVecB * c1_vec = checkAndGetColumn(c1.get())) - vectorVector(c0_vec->getData(), c1_vec->getData(), vec_res, scale); - else - throw Exception(ErrorCodes::LOGICAL_ERROR, "Wrong column in Decimal comparison"); - } - else - throw Exception(ErrorCodes::LOGICAL_ERROR, "Wrong column in Decimal comparison"); - } + throw Exception(ErrorCodes::LOGICAL_ERROR, "Wrong column in Decimal comparison"); } return c_res; } - template + template static NO_INLINE UInt8 apply(A a, B b, CompareInt scale [[maybe_unused]]) { CompareInt x; @@ -232,7 +228,7 @@ private: else y = static_cast(b); - if constexpr (_check_overflow) + if constexpr (check_overflow) { bool overflow = false; @@ -264,9 +260,8 @@ private: return Op::apply(x, y); } - template - static void NO_INLINE vectorVector(const ArrayA & a, const ArrayB & b, PaddedPODArray & c, - CompareInt scale) + template + static void NO_INLINE vectorVector(const ArrayA & a, const ArrayB & b, PaddedPODArray & c, CompareInt scale) { size_t size = a.size(); const A * a_pos = a.data(); @@ -276,14 +271,14 @@ private: while (a_pos < a_end) { - *c_pos = apply(*a_pos, *b_pos, scale); + *c_pos = apply(*a_pos, *b_pos, scale); ++a_pos; ++b_pos; ++c_pos; } } - template + template static void NO_INLINE vectorConstant(const ArrayA & a, B b, PaddedPODArray & c, CompareInt scale) { size_t size = a.size(); @@ -293,13 +288,13 @@ private: while (a_pos < a_end) { - *c_pos = apply(*a_pos, b, scale); + *c_pos = apply(*a_pos, b, scale); ++a_pos; ++c_pos; } } - template + template static void NO_INLINE constantVector(A a, const ArrayB & b, PaddedPODArray & c, CompareInt scale) { size_t size = b.size(); @@ -309,7 +304,7 @@ private: while (b_pos < b_end) { - *c_pos = apply(a, *b_pos, scale); + *c_pos = apply(a, *b_pos, scale); ++b_pos; ++c_pos; } diff --git a/src/Core/Field.cpp b/src/Core/Field.cpp index e774a95e19f..90f30b52c0c 100644 --- a/src/Core/Field.cpp +++ b/src/Core/Field.cpp @@ -529,22 +529,25 @@ Field Field::restoreFromDump(std::string_view dump_) template bool decimalEqual(T x, T y, UInt32 x_scale, UInt32 y_scale) { + bool check_overflow = true; using Comparator = DecimalComparison; - return Comparator::compare(x, y, x_scale, y_scale); + return Comparator::compare(x, y, x_scale, y_scale, check_overflow); } template bool decimalLess(T x, T y, UInt32 x_scale, UInt32 y_scale) { + bool check_overflow = true; using Comparator = DecimalComparison; - return Comparator::compare(x, y, x_scale, y_scale); + return Comparator::compare(x, y, x_scale, y_scale, check_overflow); } template bool decimalLessOrEqual(T x, T y, UInt32 x_scale, UInt32 y_scale) { + bool check_overflow = true; using Comparator = DecimalComparison; - return Comparator::compare(x, y, x_scale, y_scale); + return Comparator::compare(x, y, x_scale, y_scale, check_overflow); } diff --git a/src/Formats/ProtobufSerializer.cpp b/src/Formats/ProtobufSerializer.cpp index bd8aeff5a5d..5ea645c9c3c 100644 --- a/src/Formats/ProtobufSerializer.cpp +++ b/src/Formats/ProtobufSerializer.cpp @@ -1307,7 +1307,8 @@ namespace { if (decimal.value == 0) writeInt(0); - else if (DecimalComparison::compare(decimal, 1, scale, 0)) + else if (DecimalComparison::compare( + decimal, 1, scale, 0, /* check overflow */ true)) writeInt(1); else { diff --git a/src/Functions/FunctionsComparison.h b/src/Functions/FunctionsComparison.h index e6133bcd773..054d2fe7079 100644 --- a/src/Functions/FunctionsComparison.h +++ b/src/Functions/FunctionsComparison.h @@ -803,9 +803,8 @@ private: using LeftDataType = typename Types::LeftType; using RightDataType = typename Types::RightType; - if (check_decimal_overflow) - return (res = DecimalComparison::apply(col_left, col_right)) != nullptr; - return (res = DecimalComparison::apply(col_left, col_right)) != nullptr; + return (res = DecimalComparison::apply(col_left, col_right, check_decimal_overflow)) + != nullptr; }; if (!callOnAtLeastOneDecimalType(left_number, right_number, call)) From a258b6d0f270fc08ec98cbac8ecce58aaf2081c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Mar=C3=ADn?= Date: Mon, 18 Nov 2024 17:36:45 +0100 Subject: [PATCH 04/18] Prevent magic_enum in Field.h --- src/Core/Field.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/Core/Field.h b/src/Core/Field.h index c08d5c9eb42..5a6ee9cdf29 100644 --- a/src/Core/Field.h +++ b/src/Core/Field.h @@ -863,6 +863,9 @@ template <> struct Field::EnumToType { usi template <> struct Field::EnumToType { using Type = CustomType; }; template <> struct Field::EnumToType { using Type = UInt64; }; +/// Use it to prevent inclusion of magic_enum in headers, which is very expensive for the compiler +std::string_view fieldTypeToString(Field::Types::Which type); + constexpr bool isInt64OrUInt64FieldType(Field::Types::Which t) { return t == Field::Types::Int64 @@ -886,7 +889,7 @@ auto & Field::safeGet() if (target != which && !(which == Field::Types::Bool && (target == Field::Types::UInt64 || target == Field::Types::Int64)) && !(isInt64OrUInt64FieldType(which) && isInt64OrUInt64FieldType(target))) - throw Exception(ErrorCodes::BAD_GET, "Bad get: has {}, requested {}", getTypeName(), target); + throw Exception(ErrorCodes::BAD_GET, "Bad get: has {}, requested {}", getTypeName(), fieldTypeToString(target)); return get(); } @@ -1002,8 +1005,6 @@ void readQuoted(DecimalField & x, ReadBuffer & buf); void writeFieldText(const Field & x, WriteBuffer & buf); String toString(const Field & x); - -std::string_view fieldTypeToString(Field::Types::Which type); } template <> From e33f5bb4e943bda039b442e8999f5c28ba84ebca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Mar=C3=ADn?= Date: Mon, 18 Nov 2024 17:45:41 +0100 Subject: [PATCH 05/18] Remove unused leftovers Usage was removed in 23.6 https://github.com/ClickHouse/ClickHouse/pull/50531 --- src/Functions/FunctionsComparison.h | 62 ----------------------------- 1 file changed, 62 deletions(-) diff --git a/src/Functions/FunctionsComparison.h b/src/Functions/FunctionsComparison.h index 054d2fe7079..0add34939b2 100644 --- a/src/Functions/FunctionsComparison.h +++ b/src/Functions/FunctionsComparison.h @@ -41,12 +41,6 @@ #include #include -#if USE_EMBEDDED_COMPILER -# include -# include -#endif - - namespace DB { @@ -612,62 +606,6 @@ struct GenericComparisonImpl } }; - -#if USE_EMBEDDED_COMPILER - -template