This is an automated email from the ASF dual-hosted git repository.
yiguolei pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new 5129ab5738c [fix](decimalv2) fix decimalv2 agg errors (#29246)
5129ab5738c is described below
commit 5129ab5738c3da0eb7c9c165c022051b771a3cf4
Author: TengJianPing <[email protected]>
AuthorDate: Thu Dec 28 21:17:16 2023 +0800
[fix](decimalv2) fix decimalv2 agg errors (#29246)
---
be/src/common/consts.h | 4 +-
be/src/runtime/type_limit.h | 5 +-
be/src/util/string_parser.hpp | 31 +++++------
be/src/vec/data_types/data_type_decimal.cpp | 7 ++-
be/src/vec/data_types/data_type_decimal.h | 2 +-
.../datatype_p0/decimalv2/test_decimalv2_agg.out | 19 +++++++
.../datatype_p0/decimalv2/test_decimalv2_load.out | 8 +++
.../data/datatype_p0/decimalv3/test_load.out | 6 ++
.../decimalv2/test_decimalv2_agg.groovy | 64 ++++++++++++++++++++++
.../decimalv2/test_decimalv2_load.groovy | 36 ++++++++++++
.../suites/datatype_p0/decimalv3/test_load.groovy | 43 +++++++++++++++
11 files changed, 199 insertions(+), 26 deletions(-)
diff --git a/be/src/common/consts.h b/be/src/common/consts.h
index 7548f9a2026..dfee235e37b 100644
--- a/be/src/common/consts.h
+++ b/be/src/common/consts.h
@@ -40,8 +40,8 @@ constexpr int MAX_DECIMAL128_PRECISION = 38;
constexpr int MAX_DECIMAL256_PRECISION = 76;
/// Must be kept in sync with FE's max precision/scale.
-static constexpr int MAX_DECIMALV2_PRECISION = MAX_DECIMAL128_PRECISION;
-static constexpr int MAX_DECIMALV2_SCALE = MAX_DECIMALV2_PRECISION;
+static constexpr int MAX_DECIMALV2_PRECISION = 27;
+static constexpr int MAX_DECIMALV2_SCALE = 9;
static constexpr int MAX_DECIMALV3_PRECISION = MAX_DECIMAL256_PRECISION;
static constexpr int MAX_DECIMALV3_SCALE = MAX_DECIMALV3_PRECISION;
diff --git a/be/src/runtime/type_limit.h b/be/src/runtime/type_limit.h
index ebfc1d92ec7..be3b2186d12 100644
--- a/be/src/runtime/type_limit.h
+++ b/be/src/runtime/type_limit.h
@@ -64,10 +64,7 @@ struct type_limit<vectorized::Decimal128I> {
template <>
struct type_limit<vectorized::Decimal128> {
- static vectorized::Decimal128 max() {
- return (static_cast<int128_t>(999999999999999999ll) *
100000000000000000ll * 1000ll +
- static_cast<int128_t>(99999999999999999ll) * 1000ll + 999ll);
- }
+ static vectorized::Decimal128 max() { return
DecimalV2Value::get_max_decimal().value(); }
static vectorized::Decimal128 min() { return -max(); }
};
static wide::Int256 MAX_DECIMAL256_INT({18446744073709551615ul,
8607968719199866879ul,
diff --git a/be/src/util/string_parser.hpp b/be/src/util/string_parser.hpp
index 83dcc4fd958..3a72250d2fa 100644
--- a/be/src/util/string_parser.hpp
+++ b/be/src/util/string_parser.hpp
@@ -154,7 +154,8 @@ public:
return string_to_bool_internal(s + i, len - i, result);
}
- template <PrimitiveType P, typename T =
PrimitiveTypeTraits<P>::CppType::NativeType>
+ template <PrimitiveType P, typename T =
PrimitiveTypeTraits<P>::CppType::NativeType,
+ typename DecimalType =
PrimitiveTypeTraits<P>::ColumnType::value_type>
static inline T string_to_decimal(const char* s, int len, int
type_precision, int type_scale,
ParseResult* result);
@@ -519,7 +520,7 @@ inline bool StringParser::string_to_bool_internal(const
char* s, int len, ParseR
return false;
}
-template <PrimitiveType P, typename T>
+template <PrimitiveType P, typename T, typename DecimalType>
T StringParser::string_to_decimal(const char* s, int len, int type_precision,
int type_scale,
ParseResult* result) {
static_assert(std::is_same_v<T, int32_t> || std::is_same_v<T, int64_t> ||
@@ -598,8 +599,9 @@ T StringParser::string_to_decimal(const char* s, int len,
int type_precision, in
value = (value * 10) + (c - '0'); // Benchmarks are faster
with parenthesis...
} else {
*result = StringParser::PARSE_OVERFLOW;
- value = is_negative ? type_limit<DecimalV2Value>::min()
- : type_limit<DecimalV2Value>::max();
+ value = is_negative
+ ?
vectorized::min_decimal_value<DecimalType>(type_precision)
+ :
vectorized::max_decimal_value<DecimalType>(type_precision);
return value;
}
DCHECK(value >= 0); // For some reason //DCHECK_GE doesn't
work with __int128.
@@ -646,10 +648,9 @@ T StringParser::string_to_decimal(const char* s, int len,
int type_precision, in
cur_digit = precision - scale;
} else if (!found_dot && max_digit < (precision - scale)) {
*result = StringParser::PARSE_OVERFLOW;
- value = is_negative ?
vectorized::min_decimal_value<vectorized::Decimal<T>>(
- type_precision)
- :
vectorized::max_decimal_value<vectorized::Decimal<T>>(
- type_precision);
+ value = is_negative
+ ?
vectorized::min_decimal_value<DecimalType>(type_precision)
+ :
vectorized::max_decimal_value<DecimalType>(type_precision);
return value;
} else if (found_dot && scale >= type_scale && !has_round) {
// make rounding cases
@@ -689,11 +690,10 @@ T StringParser::string_to_decimal(const char* s, int len,
int type_precision, in
if (!is_numeric_ascii(c)) {
if (cur_digit > type_precision) {
*result = StringParser::PARSE_OVERFLOW;
- value = is_negative
- ?
vectorized::min_decimal_value<vectorized::Decimal<T>>(
- type_precision)
- :
vectorized::max_decimal_value<vectorized::Decimal<T>>(
- type_precision);
+ value = is_negative ?
vectorized::min_decimal_value<DecimalType>(
+ type_precision)
+ :
vectorized::max_decimal_value<DecimalType>(
+ type_precision);
return value;
}
return is_negative ? T(-value) : T(value);
@@ -732,9 +732,8 @@ T StringParser::string_to_decimal(const char* s, int len,
int type_precision, in
*result = StringParser::PARSE_OVERFLOW;
if constexpr (TYPE_DECIMALV2 != P) {
// decimalv3 overflow will return max min value for type precision
- value = is_negative
- ?
vectorized::min_decimal_value<vectorized::Decimal<T>>(type_precision)
- :
vectorized::max_decimal_value<vectorized::Decimal<T>>(type_precision);
+ value = is_negative ?
vectorized::min_decimal_value<DecimalType>(type_precision)
+ :
vectorized::max_decimal_value<DecimalType>(type_precision);
return value;
}
} else if (UNLIKELY(scale > type_scale)) {
diff --git a/be/src/vec/data_types/data_type_decimal.cpp
b/be/src/vec/data_types/data_type_decimal.cpp
index 38c3468c5df..20e444fb0e2 100644
--- a/be/src/vec/data_types/data_type_decimal.cpp
+++ b/be/src/vec/data_types/data_type_decimal.cpp
@@ -166,11 +166,12 @@ bool DataTypeDecimal<T>::parse_from_string(const
std::string& str, T* res) const
}
DataTypePtr create_decimal(UInt64 precision_value, UInt64 scale_value, bool
use_v2) {
- if (precision_value < min_decimal_precision() ||
- precision_value > max_decimal_precision<Decimal256>()) {
+ auto max_precision =
+ use_v2 ? max_decimal_precision<Decimal128>() :
max_decimal_precision<Decimal256>();
+ if (precision_value < min_decimal_precision() || precision_value >
max_precision) {
throw doris::Exception(doris::ErrorCode::NOT_IMPLEMENTED_ERROR,
"Wrong precision {}, min: {}, max: {}",
precision_value,
- min_decimal_precision(),
max_decimal_precision<Decimal256>());
+ min_decimal_precision(), max_precision);
}
if (static_cast<UInt64>(scale_value) > precision_value) {
diff --git a/be/src/vec/data_types/data_type_decimal.h
b/be/src/vec/data_types/data_type_decimal.h
index 7bb78265738..68d48e393b0 100644
--- a/be/src/vec/data_types/data_type_decimal.h
+++ b/be/src/vec/data_types/data_type_decimal.h
@@ -87,7 +87,7 @@ constexpr size_t max_decimal_precision<Decimal64>() {
}
template <>
constexpr size_t max_decimal_precision<Decimal128>() {
- return BeConsts::MAX_DECIMAL128_PRECISION;
+ return BeConsts::MAX_DECIMALV2_PRECISION;
}
template <>
constexpr size_t max_decimal_precision<Decimal128I>() {
diff --git a/regression-test/data/datatype_p0/decimalv2/test_decimalv2_agg.out
b/regression-test/data/datatype_p0/decimalv2/test_decimalv2_agg.out
new file mode 100644
index 00000000000..19e5ec004b6
--- /dev/null
+++ b/regression-test/data/datatype_p0/decimalv2/test_decimalv2_agg.out
@@ -0,0 +1,19 @@
+-- This file is automatically generated. You should know what you did if you
want to edit this
+-- !decimalv2_min --
+-123456789012345678.901234567
+
+-- !decimalv2_max --
+123456789012345678.901234567
+
+-- !decimalv2_count --
+3
+
+-- !decimalv2_sum --
+0.012345679
+
+-- !decimalv2_avg --
+0.004115226
+
+-- !decimalv2_sum2 --
+1000000000000000000.000000000
+
diff --git a/regression-test/data/datatype_p0/decimalv2/test_decimalv2_load.out
b/regression-test/data/datatype_p0/decimalv2/test_decimalv2_load.out
index 8156a9144aa..ae9921a0f37 100644
--- a/regression-test/data/datatype_p0/decimalv2/test_decimalv2_load.out
+++ b/regression-test/data/datatype_p0/decimalv2/test_decimalv2_load.out
@@ -15,3 +15,11 @@
11.99990
837.43444
+-- !decimalv2_insert --
+999999999999999999.999999999 1.000000000
+-999999999999999999.999999999 2.000000000
+999999999999999999.999999999 3.000000000
+-999999999999999999.999999999 4.000000000
+999999999999999999.999999999 5.000000000
+-999999999999999999.999999999 6.000000000
+
diff --git a/regression-test/data/datatype_p0/decimalv3/test_load.out
b/regression-test/data/datatype_p0/decimalv3/test_load.out
index 35c355781e6..204304a2a30 100644
--- a/regression-test/data/datatype_p0/decimalv3/test_load.out
+++ b/regression-test/data/datatype_p0/decimalv3/test_load.out
@@ -4,3 +4,9 @@
0.000135039891190100
0.000390160098269360
+-- !decimalv3_insert --
+-99999999999999999999999999999999.999999 2.000000
+-99999999999999999999999999999999.999999 4.000000
+99999999999999999999999999999999.999999 1.000000
+99999999999999999999999999999999.999999 3.000000
+
diff --git
a/regression-test/suites/datatype_p0/decimalv2/test_decimalv2_agg.groovy
b/regression-test/suites/datatype_p0/decimalv2/test_decimalv2_agg.groovy
new file mode 100644
index 00000000000..36fc347c0c4
--- /dev/null
+++ b/regression-test/suites/datatype_p0/decimalv2/test_decimalv2_agg.groovy
@@ -0,0 +1,64 @@
+// 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_decimalv2_agg", "nonConcurrent") {
+
+ sql """
+ admin set frontend config("enable_decimal_conversion" = "false");
+ """
+
+ sql """
+ drop table if exists test_decimalv2_agg;
+ """
+ sql """
+ create table test_decimalv2_agg (
+ k1 decimalv2(27,9)
+ )distributed by hash(k1)
+ properties("replication_num"="1");
+ """
+ sql """
+ insert into test_decimalv2_agg values
+ (123456789012345678.901234567),
+ (-123456789012345678.901234567),
+ (0.012345679),
+ (NULL);
+ """
+ qt_decimalv2_min " select min(k1) from test_decimalv2_agg; "
+ qt_decimalv2_max " select max(k1) from test_decimalv2_agg; "
+ qt_decimalv2_count " select count(k1) from test_decimalv2_agg; "
+ qt_decimalv2_sum " select sum(k1) from test_decimalv2_agg; "
+ qt_decimalv2_avg " select avg(k1) from test_decimalv2_agg; "
+
+ // test overflow
+ sql """
+ drop table if exists test_decimalv2_agg;
+ """
+ sql """
+ create table test_decimalv2_agg (
+ k1 decimalv2(27,9)
+ )distributed by hash(k1)
+ properties("replication_num"="1");
+ """
+ sql """
+ insert into test_decimalv2_agg values
+ (999999999999999999.999999999),
+ (0.000000001),
+ (NULL);
+ """
+ qt_decimalv2_sum2 " select sum(k1) from test_decimalv2_agg; "
+
+}
\ No newline at end of file
diff --git
a/regression-test/suites/datatype_p0/decimalv2/test_decimalv2_load.groovy
b/regression-test/suites/datatype_p0/decimalv2/test_decimalv2_load.groovy
index 5c065a921a0..08027c96d1d 100644
--- a/regression-test/suites/datatype_p0/decimalv2/test_decimalv2_load.groovy
+++ b/regression-test/suites/datatype_p0/decimalv2/test_decimalv2_load.groovy
@@ -84,6 +84,42 @@ suite("test_decimalv2_load", "nonConcurrent") {
select * from ${tableName2} order by 1;
"""
+ sql """
+ drop table if exists test_decimalv2_insert;
+ """
+ sql """
+ CREATE TABLE `test_decimalv2_insert` (
+ `k1` decimalv2(27, 9) null,
+ `k2` decimalv2(27, 9) null
+ )
+ DISTRIBUTED BY HASH(`k1`) BUCKETS 10
+ PROPERTIES (
+ "replication_num" = "1"
+ );
+ """
+ sql "set enable_insert_strict=true;"
+ // overflow, max is inserted
+ sql """
+ insert into test_decimalv2_insert
values("999999999999999999999999999999",1);
+ """
+ // underflow, min is inserted
+ sql """
+ insert into test_decimalv2_insert
values("-999999999999999999999999999999",2);
+ """
+ sql """
+ insert into test_decimalv2_insert
values("999999999999999999.9999999991",3);
+ """
+ sql """
+ insert into test_decimalv2_insert
values("-999999999999999999.9999999991",4);
+ """
+ sql """
+ insert into test_decimalv2_insert
values("999999999999999999.9999999995",5);
+ """
+ sql """
+ insert into test_decimalv2_insert
values("-999999999999999999.9999999995",6);
+ """
+ qt_decimalv2_insert "select * from test_decimalv2_insert order by 2; "
+
sql """
admin set frontend config("enable_decimal_conversion" = "true");
"""
diff --git a/regression-test/suites/datatype_p0/decimalv3/test_load.groovy
b/regression-test/suites/datatype_p0/decimalv3/test_load.groovy
index c2651b3c908..726b1ef0485 100644
--- a/regression-test/suites/datatype_p0/decimalv3/test_load.groovy
+++ b/regression-test/suites/datatype_p0/decimalv3/test_load.groovy
@@ -57,4 +57,47 @@ suite("test_load") {
} finally {
try_sql("DROP TABLE IF EXISTS ${tableName}")
}
+
+ sql """
+ drop table if exists test_decimalv3_insert;
+ """
+ sql """
+ CREATE TABLE `test_decimalv3_insert` (
+ `k1` decimalv3(38, 6) null,
+ `k2` decimalv3(38, 6) null
+ )
+ DISTRIBUTED BY HASH(`k1`) BUCKETS 10
+ PROPERTIES (
+ "replication_num" = "1"
+ );
+ """
+ sql "set enable_insert_strict=true;"
+ // overflow, max is inserted
+ sql """
+ insert into test_decimalv3_insert
values("9999999999999999999999999999999999999999",1);
+ """
+ // underflow, min is inserted
+ sql """
+ insert into test_decimalv3_insert
values("-9999999999999999999999999999999999999999",2);
+ """
+ sql """
+ insert into test_decimalv3_insert
values("99999999999999999999999999999999.9999991",3);
+ """
+ sql """
+ insert into test_decimalv3_insert
values("-99999999999999999999999999999999.9999991",4);
+ """
+
+ test {
+ sql """
+ insert into test_decimalv3_insert
values("99999999999999999999999999999999.9999999",5);
+ """
+ exception "error"
+ }
+ test {
+ sql """
+ insert into test_decimalv3_insert
values("-99999999999999999999999999999999.9999999",6);
+ """
+ exception "error"
+ }
+ qt_decimalv3_insert "select * from test_decimalv3_insert order by 1;"
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
