From f7995465cca875d812cfb3bb960caa750b4ce76d Mon Sep 17 00:00:00 2001 From: cambyzhu Date: Fri, 12 Jul 2024 15:39:09 +0800 Subject: [PATCH 1/2] fix round decimal128 overflow --- be/src/vec/exec/format/format_common.h | 4 +++- be/src/vec/functions/round.h | 17 ++++++++++------- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/be/src/vec/exec/format/format_common.h b/be/src/vec/exec/format/format_common.h index 4227a2128d24e7..3edf021ad2797a 100644 --- a/be/src/vec/exec/format/format_common.h +++ b/be/src/vec/exec/format/format_common.h @@ -33,7 +33,7 @@ struct DecimalScaleParams { int64_t scale_factor = 1; template - static inline constexpr DecimalPrimitiveType get_scale_factor(int32_t n) { + static inline constexpr DecimalPrimitiveType::NativeType get_scale_factor(int32_t n) { if constexpr (std::is_same_v) { return common::exp10_i32(n); } else if constexpr (std::is_same_v) { @@ -42,6 +42,8 @@ struct DecimalScaleParams { return common::exp10_i128(n); } else if constexpr (std::is_same_v) { return common::exp10_i128(n); + } else if constexpr (std::is_same_v) { + return common::exp10_i256(n); } else { static_assert(!sizeof(DecimalPrimitiveType), "All types must be matched with if constexpr."); diff --git a/be/src/vec/functions/round.h b/be/src/vec/functions/round.h index a17865914c4fa6..c8a2cd5d4e9664 100644 --- a/be/src/vec/functions/round.h +++ b/be/src/vec/functions/round.h @@ -32,6 +32,7 @@ #include "vec/core/types.h" #include "vec/data_types/data_type.h" #include "vec/data_types/data_type_nullable.h" +#include "vec/exec/format/format_common.h" #include "vec/functions/function.h" #if defined(__SSE4_1__) || defined(__aarch64__) #include "util/sse_util.hpp" @@ -126,7 +127,7 @@ struct IntegerRoundingComputation { __builtin_unreachable(); } - static ALWAYS_INLINE T compute(T x, T scale, size_t target_scale) { + static ALWAYS_INLINE T compute(T x, T scale, T target_scale) { switch (scale_mode) { case ScaleMode::Zero: case ScaleMode::Positive: @@ -163,21 +164,22 @@ class DecimalRoundingImpl { Int16 out_scale) { Int16 scale_arg = in_scale - out_scale; if (scale_arg > 0) { - size_t scale = int_exp10(scale_arg); + auto scale = DecimalScaleParams::get_scale_factor(scale_arg); const NativeType* __restrict p_in = reinterpret_cast(in.data()); const NativeType* end_in = reinterpret_cast(in.data()) + in.size(); NativeType* __restrict p_out = reinterpret_cast(out.data()); if (out_scale < 0) { + auto negative_scale = DecimalScaleParams::get_scale_factor(-out_scale); while (p_in < end_in) { - Op::compute(p_in, scale, p_out, int_exp10(-out_scale)); + *p_out = Op::compute(*p_in, scale, negative_scale); ++p_in; ++p_out; } } else { while (p_in < end_in) { - Op::compute(p_in, scale, p_out, 1); + *p_out = Op::compute(*p_in, scale, 1); ++p_in; ++p_out; } @@ -191,11 +193,12 @@ class DecimalRoundingImpl { Int16 out_scale) { Int16 scale_arg = in_scale - out_scale; if (scale_arg > 0) { - size_t scale = int_exp10(scale_arg); + auto scale = DecimalScaleParams::get_scale_factor(scale_arg); if (out_scale < 0) { - Op::compute(&in, scale, &out, int_exp10(-out_scale)); + auto negative_scale = DecimalScaleParams::get_scale_factor(-out_scale); + out = Op::compute(in, scale, negative_scale); } else { - Op::compute(&in, scale, &out, 1); + out = Op::compute(in, scale, 1); } } else { memcpy(&out, &in, sizeof(NativeType)); From 2b6991bd8d17d8c042325b53b4da43d960d98f5b Mon Sep 17 00:00:00 2001 From: cambyzhu Date: Fri, 12 Jul 2024 20:42:08 +0800 Subject: [PATCH 2/2] fix round overflow problem --- .../math_functions/test_round_overflow.out | 10 ++++++++ .../math_functions/test_round_overflow.groovy | 25 +++++++++++++++++++ 2 files changed, 35 insertions(+) create mode 100644 regression-test/data/nereids_p0/sql_functions/math_functions/test_round_overflow.out create mode 100644 regression-test/suites/nereids_p0/sql_functions/math_functions/test_round_overflow.groovy diff --git a/regression-test/data/nereids_p0/sql_functions/math_functions/test_round_overflow.out b/regression-test/data/nereids_p0/sql_functions/math_functions/test_round_overflow.out new file mode 100644 index 00000000000000..a18cc094872f0e --- /dev/null +++ b/regression-test/data/nereids_p0/sql_functions/math_functions/test_round_overflow.out @@ -0,0 +1,10 @@ +-- This file is automatically generated. You should know what you did if you want to edit this +-- !select1 -- +186 + +-- !select2 -- +0 + +-- !select3 -- +20000000000000000000000 + diff --git a/regression-test/suites/nereids_p0/sql_functions/math_functions/test_round_overflow.groovy b/regression-test/suites/nereids_p0/sql_functions/math_functions/test_round_overflow.groovy new file mode 100644 index 00000000000000..12311537c2749a --- /dev/null +++ b/regression-test/suites/nereids_p0/sql_functions/math_functions/test_round_overflow.groovy @@ -0,0 +1,25 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +suite("test_round_overflow") { + sql "SET enable_nereids_planner=true" + sql "SET enable_fallback_to_original_planner=false" + + qt_select1 "select round(coalesce(186,-33280029.8473323000000000000000000));" + qt_select2 "select round(coalesce(186,-33280029.8473323000000000000000000), -20);" + qt_select3 "select round(coalesce(18618500001234567890123.0,-33280029.847332300000000),-22);" +}