From a1dcd24a768f6a791e586e3d201c7f62cadb4bb5 Mon Sep 17 00:00:00 2001 From: Vitaliy Lyudvichenko Date: Fri, 13 Jan 2017 21:15:12 +0300 Subject: [PATCH 1/2] New behavior for least() and greatest() function with (Int64, UInt64) arguments. [#CLICKHOUSE-29] --- .../DB/Functions/FunctionsArithmetic.h | 64 +++++++++++++++++-- dbms/include/DB/Functions/NumberTraits.h | 22 ++++++- ...0413_least_greatest_new_behavior.reference | 3 + .../00413_least_greatest_new_behavior.sql | 3 + 4 files changed, 85 insertions(+), 7 deletions(-) create mode 100644 dbms/tests/queries/0_stateless/00413_least_greatest_new_behavior.reference create mode 100644 dbms/tests/queries/0_stateless/00413_least_greatest_new_behavior.sql diff --git a/dbms/include/DB/Functions/FunctionsArithmetic.h b/dbms/include/DB/Functions/FunctionsArithmetic.h index 377e7d4c612..941fd555d5b 100644 --- a/dbms/include/DB/Functions/FunctionsArithmetic.h +++ b/dbms/include/DB/Functions/FunctionsArithmetic.h @@ -271,9 +271,9 @@ struct BitShiftRightImpl template -struct LeastImpl +struct LeastBaseImpl { - using ResultType = typename NumberTraits::ResultOfIf::Type; + using ResultType = NumberTraits::ResultOfLeast; template static inline Result apply(A a, B b) @@ -283,10 +283,37 @@ struct LeastImpl } }; -template -struct GreatestImpl +template +struct LeastSpecialImpl { - using ResultType = typename NumberTraits::ResultOfIf::Type; + using ResultType = std::make_signed_t; + using SecondType = std::make_unsigned_t; + + template + static inline Result apply(ResultType a, SecondType b) + { + static_assert(std::is_same::value, "typeof(a) != Result"); + return (a < static_cast(b) || a < 0 || b > static_cast(std::numeric_limits::max())) + ? a : static_cast(b); + } + + template + static inline Result apply(SecondType a, ResultType b) + { + static_assert(std::is_same::value, "typeof(b) != Result"); + return (b < static_cast(a) || b < 0 || a > static_cast(std::numeric_limits::max())) + ? b : static_cast(a); + } +}; + +template +using LeastImpl = std::conditional_t::value, LeastBaseImpl, LeastSpecialImpl>; + + +template +struct GreatestBaseImpl +{ + using ResultType = NumberTraits::ResultOfGreatest; template static inline Result apply(A a, B b) @@ -295,6 +322,33 @@ struct GreatestImpl } }; +template +struct GreatestSpecialImpl +{ + using ResultType = std::make_unsigned_t; + using SecondType = std::make_signed_t; + + template + static inline Result apply(ResultType a, SecondType b) + { + static_assert(std::is_same::value, "typeof(a) != Result"); + return (a > static_cast(b) || b < 0 || a > static_cast((std::numeric_limits::max()))) + ? a : static_cast(b); + } + + template + static inline Result apply(SecondType a, ResultType b) + { + static_assert(std::is_same::value, "typeof(b) != Result"); + return (b > static_cast(a) || a < 0 || b > static_cast((std::numeric_limits::max()))) + ? b : static_cast(a); + } +}; + +template +using GreatestImpl = std::conditional_t::value, GreatestBaseImpl, GreatestSpecialImpl>; + + template struct NegateImpl { diff --git a/dbms/include/DB/Functions/NumberTraits.h b/dbms/include/DB/Functions/NumberTraits.h index fb4f8cb4c0d..81cb4bac974 100644 --- a/dbms/include/DB/Functions/NumberTraits.h +++ b/dbms/include/DB/Functions/NumberTraits.h @@ -13,6 +13,7 @@ #include #include +#include namespace DB { @@ -263,6 +264,7 @@ template struct ResultOfBitNot typename Traits::Nullity>::Type Type; }; + /** Приведение типов для функции if: * 1) void, Type -> Type * 2) UInt, UInt -> UInt @@ -302,7 +304,7 @@ struct ResultOfIf typename Traits::Floatness, typename Traits::Bits, typename ExactNext::Bits>::Type>::type>::type, - Bits32>::type, + Bits32>::type, typename boost::mpl::or_::Nullity, typename Traits::Nullity>::type>::Type, /// 2) and 3) typename boost::mpl::if_< @@ -342,10 +344,26 @@ template struct ToInteger typename Traits::Floatness, Bits64, typename Traits::Bits>::type, - typename Traits::Nullity + typename Traits::Nullity >::Type Type; }; + +// CLICKHOUSE-29. The same depth, different signs +template +using GreatestAndLeastSpecialCase = std::integral_constant::value && std::is_integral::value + && (sizeof(A) == sizeof(B)) && (std::is_signed::value ^ std::is_signed::value)>; + +template +using ResultOfLeast = std::conditional_t::value, + typename Construct::Bits, HasNoNull>::Type, + typename ResultOfIf::Type>; + +template +using ResultOfGreatest = std::conditional_t::value, + typename Construct::Bits, HasNoNull>::Type, + typename ResultOfIf::Type>; + /// Notes on type composition. /// /// I. Problem statement. diff --git a/dbms/tests/queries/0_stateless/00413_least_greatest_new_behavior.reference b/dbms/tests/queries/0_stateless/00413_least_greatest_new_behavior.reference new file mode 100644 index 00000000000..7a6eabd4b09 --- /dev/null +++ b/dbms/tests/queries/0_stateless/00413_least_greatest_new_behavior.reference @@ -0,0 +1,3 @@ +Int8 UInt8 Int64 UInt64 +0 -400 127 -1 -1 -128 -128 -9223372036854775808 -9223372036854775807 9223372036854775807 +1 -200 255 1 1 254 255 18446744073709551615 18446744073709551615 9223372036854775808 diff --git a/dbms/tests/queries/0_stateless/00413_least_greatest_new_behavior.sql b/dbms/tests/queries/0_stateless/00413_least_greatest_new_behavior.sql new file mode 100644 index 00000000000..aa1a9c5c97f --- /dev/null +++ b/dbms/tests/queries/0_stateless/00413_least_greatest_new_behavior.sql @@ -0,0 +1,3 @@ +SELECT toTypeName(least(-1, 1)), toTypeName(greatest(-1, 1)), toTypeName(least(-9223372036854775808, 18446744073709551615)), toTypeName(greatest(-9223372036854775808, 18446744073709551615)); +SELECT least(0, 1), least(-400, -200), least(toInt8(127), 255), least(-1, 1), least(toUInt64(1), toInt64(-1)), least(-128, 254), least(-128, 255), least(-9223372036854775808, 18446744073709551615), least(-9223372036854775807, 18446744073709551615), least(toInt64(9223372036854775807), 9223372036854775808); +SELECT greatest(0, 1), greatest(-400, -200), greatest(toInt8(127), 255), greatest(-1, 1), greatest(toUInt64(1), toInt64(-1)), greatest(-128, 254), greatest(-128, 255), greatest(-9223372036854775808, 18446744073709551615), greatest(-9223372036854775807, 18446744073709551615), greatest(toInt64(9223372036854775807), 9223372036854775808); From c56d3724e4f014d8900e301ea1a724616a0e9fa9 Mon Sep 17 00:00:00 2001 From: Vitaliy Lyudvichenko Date: Fri, 13 Jan 2017 22:45:18 +0300 Subject: [PATCH 2/2] Speedup accurate integer comparisons. Simplified code. [#CLICKHOUSE-29] [#CLICKHOUSE-194] --- .../include/DB/Functions/AccurateComparison.h | 33 +++++++-------- .../DB/Functions/FunctionsArithmetic.h | 41 +++++-------------- dbms/include/DB/Functions/NumberTraits.h | 10 +++-- .../accurate_comparisons.sh | 15 +++++-- ...0413_least_greatest_new_behavior.reference | 2 +- .../00413_least_greatest_new_behavior.sql | 2 +- 6 files changed, 47 insertions(+), 56 deletions(-) diff --git a/dbms/include/DB/Functions/AccurateComparison.h b/dbms/include/DB/Functions/AccurateComparison.h index 4871a667f1d..f841c8a468b 100644 --- a/dbms/include/DB/Functions/AccurateComparison.h +++ b/dbms/include/DB/Functions/AccurateComparison.h @@ -1,3 +1,4 @@ +#pragma once #include /** Preceptually-correct number comparisons. @@ -48,27 +49,27 @@ template using bool_if_le_int_vs_uint_t = std::enable_if_t::value, bool>; template -bool_if_le_int_vs_uint_t greaterOpTmpl(TInt a, TUInt b) +inline bool_if_le_int_vs_uint_t greaterOpTmpl(TInt a, TUInt b) { - return (b > static_cast(std::numeric_limits::max()) || a < 0) ? false : static_cast(a) > b; + return static_cast(a) > b && a >= 0 && b <= static_cast(std::numeric_limits::max()); } template -bool_if_le_int_vs_uint_t greaterOpTmpl(TUInt a, TInt b) +inline bool_if_le_int_vs_uint_t greaterOpTmpl(TUInt a, TInt b) { - return (a > static_cast(std::numeric_limits::max()) || b < 0) ? true : a > static_cast(b); + return a > static_cast(b) || b < 0 || a > static_cast(std::numeric_limits::max()); } template -bool_if_le_int_vs_uint_t equalsOpTmpl(TInt a, TUInt b) +inline bool_if_le_int_vs_uint_t equalsOpTmpl(TInt a, TUInt b) { - return (a < 0 || b > static_cast(std::numeric_limits::max())) ? false : static_cast(a) == b; + return static_cast(a) == b && a >= 0 && b <= static_cast(std::numeric_limits::max()); } template -bool_if_le_int_vs_uint_t equalsOpTmpl(TUInt a, TInt b) +inline bool_if_le_int_vs_uint_t equalsOpTmpl(TUInt a, TInt b) { - return (b < 0 || a > static_cast(std::numeric_limits::max())) ? false : a == static_cast(b); + return a == static_cast(b) && b >= 0 && a <= static_cast(std::numeric_limits::max()); } @@ -80,25 +81,25 @@ template using bool_if_gt_int_vs_uint = std::enable_if_t::value, bool>; template -bool_if_gt_int_vs_uint greaterOpTmpl(TInt a, TUInt b) +inline bool_if_gt_int_vs_uint greaterOpTmpl(TInt a, TUInt b) { return static_cast(a) > static_cast(b); } template -bool_if_gt_int_vs_uint greaterOpTmpl(TUInt a, TInt b) +inline bool_if_gt_int_vs_uint greaterOpTmpl(TUInt a, TInt b) { return static_cast(a) > static_cast(b); } template -bool_if_gt_int_vs_uint equalsOpTmpl(TInt a, TUInt b) +inline bool_if_gt_int_vs_uint equalsOpTmpl(TInt a, TUInt b) { return static_cast(a) == static_cast(b); } template -bool_if_gt_int_vs_uint equalsOpTmpl(TUInt a, TInt b) +inline bool_if_gt_int_vs_uint equalsOpTmpl(TUInt a, TInt b) { return static_cast(a) == static_cast(b); } @@ -111,25 +112,25 @@ using bool_if_double_can_be_used = std::enable_if_t< bool>; template -bool_if_double_can_be_used greaterOpTmpl(TAInt a, TAFloat b) +inline bool_if_double_can_be_used greaterOpTmpl(TAInt a, TAFloat b) { return static_cast(a) > static_cast(b); } template -bool_if_double_can_be_used greaterOpTmpl(TAFloat a, TAInt b) +inline bool_if_double_can_be_used greaterOpTmpl(TAFloat a, TAInt b) { return static_cast(a) > static_cast(b); } template -bool_if_double_can_be_used equalsOpTmpl(TAInt a, TAFloat b) +inline bool_if_double_can_be_used equalsOpTmpl(TAInt a, TAFloat b) { return static_cast(a) == static_cast(b); } template -bool_if_double_can_be_used equalsOpTmpl(TAFloat a, TAInt b) +inline bool_if_double_can_be_used equalsOpTmpl(TAFloat a, TAInt b) { return static_cast(a) == static_cast(b); } diff --git a/dbms/include/DB/Functions/FunctionsArithmetic.h b/dbms/include/DB/Functions/FunctionsArithmetic.h index 941fd555d5b..e613001b7a1 100644 --- a/dbms/include/DB/Functions/FunctionsArithmetic.h +++ b/dbms/include/DB/Functions/FunctionsArithmetic.h @@ -5,6 +5,7 @@ #include #include #include +#include #include @@ -283,31 +284,21 @@ struct LeastBaseImpl } }; -template +template struct LeastSpecialImpl { using ResultType = std::make_signed_t; - using SecondType = std::make_unsigned_t; template - static inline Result apply(ResultType a, SecondType b) + static inline Result apply(A a, B b) { - static_assert(std::is_same::value, "typeof(a) != Result"); - return (a < static_cast(b) || a < 0 || b > static_cast(std::numeric_limits::max())) - ? a : static_cast(b); - } - - template - static inline Result apply(SecondType a, ResultType b) - { - static_assert(std::is_same::value, "typeof(b) != Result"); - return (b < static_cast(a) || b < 0 || a > static_cast(std::numeric_limits::max())) - ? b : static_cast(a); + static_assert(std::is_same::value, "ResultType != Result"); + return accurate::lessOp(a, b) ? static_cast(a) : static_cast(b); } }; template -using LeastImpl = std::conditional_t::value, LeastBaseImpl, LeastSpecialImpl>; +using LeastImpl = std::conditional_t::value, LeastBaseImpl, LeastSpecialImpl>; template @@ -322,31 +313,21 @@ struct GreatestBaseImpl } }; -template +template struct GreatestSpecialImpl { using ResultType = std::make_unsigned_t; - using SecondType = std::make_signed_t; template - static inline Result apply(ResultType a, SecondType b) + static inline Result apply(A a, B b) { - static_assert(std::is_same::value, "typeof(a) != Result"); - return (a > static_cast(b) || b < 0 || a > static_cast((std::numeric_limits::max()))) - ? a : static_cast(b); - } - - template - static inline Result apply(SecondType a, ResultType b) - { - static_assert(std::is_same::value, "typeof(b) != Result"); - return (b > static_cast(a) || a < 0 || b > static_cast((std::numeric_limits::max()))) - ? b : static_cast(a); + static_assert(std::is_same::value, "ResultType != Result"); + return accurate::greaterOp(a, b) ? static_cast(a) : static_cast(b); } }; template -using GreatestImpl = std::conditional_t::value, GreatestBaseImpl, GreatestSpecialImpl>; +using GreatestImpl = std::conditional_t::value, GreatestBaseImpl, GreatestSpecialImpl>; template diff --git a/dbms/include/DB/Functions/NumberTraits.h b/dbms/include/DB/Functions/NumberTraits.h index 81cb4bac974..c6833b3c0f3 100644 --- a/dbms/include/DB/Functions/NumberTraits.h +++ b/dbms/include/DB/Functions/NumberTraits.h @@ -350,17 +350,19 @@ template struct ToInteger // CLICKHOUSE-29. The same depth, different signs +// NOTE: This case is applied for 64-bit integers only (for backward compability), but colud be used for any-bit integers template -using GreatestAndLeastSpecialCase = std::integral_constant::value && std::is_integral::value - && (sizeof(A) == sizeof(B)) && (std::is_signed::value ^ std::is_signed::value)>; +using LeastGreatestSpecialCase = std::integral_constant::value && std::is_integral::value + && (8 == sizeof(A) && sizeof(A) == sizeof(B)) + && (std::is_signed::value ^ std::is_signed::value)>; template -using ResultOfLeast = std::conditional_t::value, +using ResultOfLeast = std::conditional_t::value, typename Construct::Bits, HasNoNull>::Type, typename ResultOfIf::Type>; template -using ResultOfGreatest = std::conditional_t::value, +using ResultOfGreatest = std::conditional_t::value, typename Construct::Bits, HasNoNull>::Type, typename ResultOfIf::Type>; diff --git a/dbms/tests/perf_drafts/accurate_comparisons/accurate_comparisons.sh b/dbms/tests/perf_drafts/accurate_comparisons/accurate_comparisons.sh index b4c1b4930bd..94e01c5236f 100755 --- a/dbms/tests/perf_drafts/accurate_comparisons/accurate_comparisons.sh +++ b/dbms/tests/perf_drafts/accurate_comparisons/accurate_comparisons.sh @@ -8,20 +8,27 @@ clickhouse-client -q "INSERT INTO test.comparisons SELECT toInt64(rand64()) + nu function test_cmp { echo -n "$1 : " echo "SELECT count() FROM test.comparisons WHERE ($1)" | clickhouse-benchmark --max_threads=1 -i 20 -d 0 --json test.json 1>&2 2>/dev/null - python2 -c "import json; print json.load(open('test.json'))['query_time_percentiles']['0']" + python2 -c "import json; print '%.3f' % float(json.load(open('test.json'))['query_time_percentiles']['0'])" rm test.json } -test_cmp "u64 = i64" -test_cmp "u64 >= i64" +test_cmp "u64 > i64 " +test_cmp "u64 > toInt64(1) " +test_cmp "i64 > u64 " +test_cmp "i64 > toUInt64(1) " +test_cmp "u64 = i64 " +test_cmp "u64 = toInt64(1) " +test_cmp "i64 = u64 " +test_cmp "i64 = toUInt64(1) " + +test_cmp "u64 >= i64" test_cmp "i64 > -1 " test_cmp "i64 = 0 " test_cmp "u64 != 0 " test_cmp "i64 = f64" test_cmp "i64 < f64" - test_cmp "f64 >= 0 " clickhouse-client -q "DROP TABLE IF EXISTS test.comparisons" diff --git a/dbms/tests/queries/0_stateless/00413_least_greatest_new_behavior.reference b/dbms/tests/queries/0_stateless/00413_least_greatest_new_behavior.reference index 7a6eabd4b09..e7f8e8ec7ba 100644 --- a/dbms/tests/queries/0_stateless/00413_least_greatest_new_behavior.reference +++ b/dbms/tests/queries/0_stateless/00413_least_greatest_new_behavior.reference @@ -1,3 +1,3 @@ -Int8 UInt8 Int64 UInt64 +Int64 UInt64 0 -400 127 -1 -1 -128 -128 -9223372036854775808 -9223372036854775807 9223372036854775807 1 -200 255 1 1 254 255 18446744073709551615 18446744073709551615 9223372036854775808 diff --git a/dbms/tests/queries/0_stateless/00413_least_greatest_new_behavior.sql b/dbms/tests/queries/0_stateless/00413_least_greatest_new_behavior.sql index aa1a9c5c97f..b458e77498c 100644 --- a/dbms/tests/queries/0_stateless/00413_least_greatest_new_behavior.sql +++ b/dbms/tests/queries/0_stateless/00413_least_greatest_new_behavior.sql @@ -1,3 +1,3 @@ -SELECT toTypeName(least(-1, 1)), toTypeName(greatest(-1, 1)), toTypeName(least(-9223372036854775808, 18446744073709551615)), toTypeName(greatest(-9223372036854775808, 18446744073709551615)); +SELECT toTypeName(least(-9223372036854775808, 18446744073709551615)), toTypeName(greatest(-9223372036854775808, 18446744073709551615)); SELECT least(0, 1), least(-400, -200), least(toInt8(127), 255), least(-1, 1), least(toUInt64(1), toInt64(-1)), least(-128, 254), least(-128, 255), least(-9223372036854775808, 18446744073709551615), least(-9223372036854775807, 18446744073709551615), least(toInt64(9223372036854775807), 9223372036854775808); SELECT greatest(0, 1), greatest(-400, -200), greatest(toInt8(127), 255), greatest(-1, 1), greatest(toUInt64(1), toInt64(-1)), greatest(-128, 254), greatest(-128, 255), greatest(-9223372036854775808, 18446744073709551615), greatest(-9223372036854775807, 18446744073709551615), greatest(toInt64(9223372036854775807), 9223372036854775808);