From 820af3a48dda0f9a97d8de857e37f40c74683c44 Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Sat, 2 Jan 2021 23:40:15 +0300 Subject: [PATCH 1/3] Simplify code of function "bar" --- src/Common/UnicodeBar.cpp | 22 ++-- src/Common/UnicodeBar.h | 2 +- src/Functions/bar.cpp | 105 +++++------------- .../0_stateless/01502_bar_overflow.sql | 2 +- 4 files changed, 39 insertions(+), 92 deletions(-) diff --git a/src/Common/UnicodeBar.cpp b/src/Common/UnicodeBar.cpp index 8ff5e2052c1..29a9838cd62 100644 --- a/src/Common/UnicodeBar.cpp +++ b/src/Common/UnicodeBar.cpp @@ -5,33 +5,25 @@ #include #include #include +#include - -namespace DB -{ - namespace ErrorCodes - { - extern const int PARAMETER_OUT_OF_BOUND; - } -} +#include namespace UnicodeBar { - double getWidth(Int64 x, Int64 min, Int64 max, double max_width) + double getWidth(double x, double min, double max, double max_width) { + if (isNaN(x)) + return 0; + if (x <= min) return 0; if (x >= max) return max_width; - /// The case when max - min overflows - Int64 max_difference; - if (common::subOverflow(max, min, max_difference)) - throw DB::Exception(DB::ErrorCodes::PARAMETER_OUT_OF_BOUND, "The arguments to render unicode bar will lead to arithmetic overflow"); - - return (x - min) * max_width / max_difference; + return (x - min) / (max - min) * max_width; } size_t getWidthInBytes(double width) diff --git a/src/Common/UnicodeBar.h b/src/Common/UnicodeBar.h index 0c62bd7e8f7..e6f49dde856 100644 --- a/src/Common/UnicodeBar.h +++ b/src/Common/UnicodeBar.h @@ -10,7 +10,7 @@ */ namespace UnicodeBar { - double getWidth(Int64 x, Int64 min, Int64 max, double max_width); + double getWidth(double x, double min, double max, double max_width); size_t getWidthInBytes(double width); /// In `dst` there must be a space for barWidthInBytes(width) characters and a trailing zero. diff --git a/src/Functions/bar.cpp b/src/Functions/bar.cpp index 2eddf23af66..7364311a1be 100644 --- a/src/Functions/bar.cpp +++ b/src/Functions/bar.cpp @@ -2,7 +2,6 @@ #include #include #include -#include #include #include #include @@ -57,23 +56,30 @@ public: + ".", ErrorCodes::NUMBER_OF_ARGUMENTS_DOESNT_MATCH); - if (!isNativeNumber(arguments[0]) || !isNativeNumber(arguments[1]) || !isNativeNumber(arguments[2]) - || (arguments.size() == 4 && !isNativeNumber(arguments[3]))) + if (!isNumber(arguments[0]) || !isNumber(arguments[1]) || !isNumber(arguments[2]) + || (arguments.size() == 4 && !isNumber(arguments[3]))) throw Exception("All arguments for function " + getName() + " must be numeric.", ErrorCodes::ILLEGAL_TYPE_OF_ARGUMENT); return std::make_shared(); } bool useDefaultImplementationForConstants() const override { return true; } - ColumnNumbers getArgumentsThatAreAlwaysConstant() const override { return {1, 2, 3}; } + ColumnNumbers getArgumentsThatAreAlwaysConstant() const override { return {3}; } - ColumnPtr executeImpl(const ColumnsWithTypeAndName & arguments, const DataTypePtr &, size_t /*input_rows_count*/) const override + ColumnPtr executeImpl(const ColumnsWithTypeAndName & arguments, const DataTypePtr &, size_t input_rows_count) const override { - Int64 min = extractConstant(arguments, 1, "Second"); /// The level at which the line has zero length. - Int64 max = extractConstant(arguments, 2, "Third"); /// The level at which the line has the maximum length. + /// The maximum width of the bar in characters. + Float64 max_width = 80; /// Motivated by old-school terminal size. - /// The maximum width of the bar in characters, by default. - Float64 max_width = arguments.size() == 4 ? extractConstant(arguments, 3, "Fourth") : 80; + if (arguments.size() == 4) + { + const auto & max_width_column = *arguments[3].column; + + if (!isColumnConst(max_width_column)) + throw Exception(ErrorCodes::ILLEGAL_COLUMN, "Fourth argument for function {} must be constant", getName()); + + max_width = max_width_column.getFloat64(0); + } if (isNaN(max_width)) throw Exception("Argument 'max_width' must not be NaN", ErrorCodes::ILLEGAL_TYPE_OF_ARGUMENT); @@ -86,83 +92,32 @@ public: const auto & src = *arguments[0].column; - auto res_column = ColumnString::create(); - - if (executeNumber(src, *res_column, min, max, max_width) - || executeNumber(src, *res_column, min, max, max_width) - || executeNumber(src, *res_column, min, max, max_width) - || executeNumber(src, *res_column, min, max, max_width) - || executeNumber(src, *res_column, min, max, max_width) - || executeNumber(src, *res_column, min, max, max_width) - || executeNumber(src, *res_column, min, max, max_width) - || executeNumber(src, *res_column, min, max, max_width) - || executeNumber(src, *res_column, min, max, max_width) - || executeNumber(src, *res_column, min, max, max_width)) - { - return res_column; - } - else - throw Exception( - "Illegal column " + arguments[0].column->getName() + " of argument of function " + getName(), - ErrorCodes::ILLEGAL_COLUMN); - } - -private: - template - T extractConstant(const ColumnsWithTypeAndName & arguments, size_t argument_pos, const char * which_argument) const - { - const auto & column = *arguments[argument_pos].column; - - if (!isColumnConst(column)) - throw Exception( - which_argument + String(" argument for function ") + getName() + " must be constant.", ErrorCodes::ILLEGAL_COLUMN); - - return applyVisitor(FieldVisitorConvertToNumber(), column[0]); - } - - template - static void fill(const PaddedPODArray & src, - ColumnString::Chars & dst_chars, - ColumnString::Offsets & dst_offsets, - Int64 min, - Int64 max, - Float64 max_width) - { - size_t size = src.size(); size_t current_offset = 0; - dst_offsets.resize(size); - dst_chars.reserve(size * (UnicodeBar::getWidthInBytes(max_width) + 1)); /// lines 0-terminated. + auto res_column = ColumnString::create(); - for (size_t i = 0; i < size; ++i) + ColumnString::Chars & dst_chars = res_column->getChars(); + ColumnString::Offsets & dst_offsets = res_column->getOffsets(); + + dst_offsets.resize(input_rows_count); + dst_chars.reserve(input_rows_count * (UnicodeBar::getWidthInBytes(max_width) + 1)); /// strings are 0-terminated. + + for (size_t i = 0; i < input_rows_count; ++i) { - Float64 width = UnicodeBar::getWidth(src[i], min, max, max_width); + Float64 width = UnicodeBar::getWidth( + src.getFloat64(i), + arguments[1].column->getFloat64(i), + arguments[2].column->getFloat64(i), + max_width); + size_t next_size = current_offset + UnicodeBar::getWidthInBytes(width) + 1; dst_chars.resize(next_size); UnicodeBar::render(width, reinterpret_cast(&dst_chars[current_offset])); current_offset = next_size; dst_offsets[i] = current_offset; } - } - template - static void fill(T src, String & dst_chars, Int64 min, Int64 max, Float64 max_width) - { - Float64 width = UnicodeBar::getWidth(src, min, max, max_width); - dst_chars.resize(UnicodeBar::getWidthInBytes(width)); - UnicodeBar::render(width, dst_chars.data()); - } - - template - static bool executeNumber(const IColumn & src, ColumnString & dst, Int64 min, Int64 max, Float64 max_width) - { - if (const ColumnVector * col = checkAndGetColumn>(&src)) - { - fill(col->getData(), dst.getChars(), dst.getOffsets(), min, max, max_width); - return true; - } - else - return false; + return res_column; } }; diff --git a/tests/queries/0_stateless/01502_bar_overflow.sql b/tests/queries/0_stateless/01502_bar_overflow.sql index cb3de7ac20b..4829b487f52 100644 --- a/tests/queries/0_stateless/01502_bar_overflow.sql +++ b/tests/queries/0_stateless/01502_bar_overflow.sql @@ -1 +1 @@ -SELECT bar((greatCircleAngle(100, -1, number, number) - number) * 2, -9223372036854775808, 1023, 100) FROM numbers(1048575); -- { serverError 12 } +SELECT bar((greatCircleAngle(100, -1, number, number) - number) * 2, -9223372036854775808, 1023, 100) FROM numbers(1048575) FORMAT Null; From 2a6388bdf63373c38463263bbbaeec5b36bf9d01 Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Sun, 3 Jan 2021 00:32:37 +0300 Subject: [PATCH 2/3] Update test --- .../01044_great_circle_angle.reference | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/tests/queries/0_stateless/01044_great_circle_angle.reference b/tests/queries/0_stateless/01044_great_circle_angle.reference index b0e80d4cdae..60a616c7187 100644 --- a/tests/queries/0_stateless/01044_great_circle_angle.reference +++ b/tests/queries/0_stateless/01044_great_circle_angle.reference @@ -11,7 +11,7 @@ 179 -0.06 ██ -████ +████▏ ██████▏ ████████▎ ██████████▎ @@ -19,25 +19,25 @@ ██████████████▍ ████████████████▌ ██████████████████▌ -████████████████████▌ +████████████████████▋ ██████████████████████▋ ████████████████████████▋ ██████████████████████████▌ ████████████████████████████▍ ██████████████████████████████▍ -████████████████████████████████▎ +████████████████████████████████▍ ██████████████████████████████████▎ ████████████████████████████████████▏ ██████████████████████████████████████ ███████████████████████████████████████▊ █████████████████████████████████████████▋ ███████████████████████████████████████████▌ -█████████████████████████████████████████████▎ -███████████████████████████████████████████████ -████████████████████████████████████████████████▋ +█████████████████████████████████████████████▍ +███████████████████████████████████████████████▏ +████████████████████████████████████████████████▊ ██████████████████████████████████████████████████▌ ████████████████████████████████████████████████████▏ -█████████████████████████████████████████████████████▋ +█████████████████████████████████████████████████████▊ ███████████████████████████████████████████████████████▍ █████████████████████████████████████████████████████████ ██████████████████████████████████████████████████████████▌ @@ -50,8 +50,8 @@ ████████████████████████████████████████████████████████████████████ █████████████████████████████████████████████████████████████████████▏ ██████████████████████████████████████████████████████████████████████▎ -███████████████████████████████████████████████████████████████████████▎ -████████████████████████████████████████████████████████████████████████▎ +███████████████████████████████████████████████████████████████████████▍ +████████████████████████████████████████████████████████████████████████▍ █████████████████████████████████████████████████████████████████████████▎ ██████████████████████████████████████████████████████████████████████████▏ ███████████████████████████████████████████████████████████████████████████ @@ -61,11 +61,11 @@ █████████████████████████████████████████████████████████████████████████████▌ █████████████████████████████████████████████████████████████████████████████▊ ██████████████████████████████████████████████████████████████████████████████▎ -██████████████████████████████████████████████████████████████████████████████▌ +██████████████████████████████████████████████████████████████████████████████▋ ██████████████████████████████████████████████████████████████████████████████▋ ██████████████████████████████████████████████████████████████████████████████▊ ██████████████████████████████████████████████████████████████████████████████▊ -██████████████████████████████████████████████████████████████████████████████▋ +██████████████████████████████████████████████████████████████████████████████▊ ██████████████████████████████████████████████████████████████████████████████▋ ██████████████████████████████████████████████████████████████████████████████▍ ██████████████████████████████████████████████████████████████████████████████ @@ -75,7 +75,7 @@ ███████████████████████████████████████████████████████████████████████████▌ ██████████████████████████████████████████████████████████████████████████▌ █████████████████████████████████████████████████████████████████████████▌ -████████████████████████████████████████████████████████████████████████▎ +████████████████████████████████████████████████████████████████████████▍ ███████████████████████████████████████████████████████████████████████ █████████████████████████████████████████████████████████████████████▋ ████████████████████████████████████████████████████████████████████ @@ -97,5 +97,5 @@ ██████████████████████▋ ██████████████████▌ ██████████████▏ -█████████▌ +█████████▋ ████▊ From ab83245b571e1823dabf2814e1f69215a606884e Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Sun, 3 Jan 2021 18:07:18 +0300 Subject: [PATCH 3/3] Addition to #18526 --- src/Common/PODArray.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Common/PODArray.h b/src/Common/PODArray.h index 344f7bfcbe1..f0cc9df11cd 100644 --- a/src/Common/PODArray.h +++ b/src/Common/PODArray.h @@ -300,7 +300,7 @@ public: const char * ptr_end = reinterpret_cast(&*from_end); /// Also it's safe if the range is empty. - assert(!((ptr_begin >= c_start && ptr_begin <= c_end) || (ptr_end >= c_start && ptr_end <= c_end)) || (ptr_begin == ptr_end)); + assert(!((ptr_begin >= c_start && ptr_begin < c_end) || (ptr_end > c_start && ptr_end <= c_end)) || (ptr_begin == ptr_end)); #endif }