Skip to content

Commit

Permalink
[fix](round) fix round decimal128 overflow (#37733)
Browse files Browse the repository at this point in the history
## Proposed changes

Issue Number: close ##37143

<!--Describe your changes.-->
  • Loading branch information
cambyzju authored and dataroaring committed Jul 17, 2024
1 parent 842dd51 commit cbeeac6
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 8 deletions.
4 changes: 3 additions & 1 deletion be/src/vec/exec/format/format_common.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ struct DecimalScaleParams {
int64_t scale_factor = 1;

template <typename DecimalPrimitiveType>
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<DecimalPrimitiveType, Decimal32>) {
return common::exp10_i32(n);
} else if constexpr (std::is_same_v<DecimalPrimitiveType, Decimal64>) {
Expand All @@ -42,6 +42,8 @@ struct DecimalScaleParams {
return common::exp10_i128(n);
} else if constexpr (std::is_same_v<DecimalPrimitiveType, Decimal128V3>) {
return common::exp10_i128(n);
} else if constexpr (std::is_same_v<DecimalPrimitiveType, Decimal256>) {
return common::exp10_i256(n);
} else {
static_assert(!sizeof(DecimalPrimitiveType),
"All types must be matched with if constexpr.");
Expand Down
17 changes: 10 additions & 7 deletions be/src/vec/functions/round.h
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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<T>(scale_arg);

const NativeType* __restrict p_in = reinterpret_cast<const NativeType*>(in.data());
const NativeType* end_in = reinterpret_cast<const NativeType*>(in.data()) + in.size();
NativeType* __restrict p_out = reinterpret_cast<NativeType*>(out.data());

if (out_scale < 0) {
auto negative_scale = DecimalScaleParams::get_scale_factor<T>(-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;
}
Expand All @@ -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<T>(scale_arg);
if (out_scale < 0) {
Op::compute(&in, scale, &out, int_exp10(-out_scale));
auto negative_scale = DecimalScaleParams::get_scale_factor<T>(-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));
Expand Down
Original file line number Diff line number Diff line change
@@ -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

Original file line number Diff line number Diff line change
@@ -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);"
}

0 comments on commit cbeeac6

Please sign in to comment.