This is an automated email from the ASF dual-hosted git repository.
dataroaring pushed a commit to branch branch-3.0
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/branch-3.0 by this push:
new b431c33ca05 [fix](function) stddev with DecimalV2 type will result in
an error (#38731)
b431c33ca05 is described below
commit b431c33ca05267dc7c0450a22efaa22959a5b8d8
Author: Mryange <59914473+mrya...@users.noreply.github.com>
AuthorDate: Thu Aug 8 10:44:31 2024 +0800
[fix](function) stddev with DecimalV2 type will result in an error (#38731)
The stddev function has a separate implementation for the DecimalV2
type, but there are issues with the implementation. Given that there is
almost no existing data for DecimalV2, it will be removed here. For be,
upgrading to this situation will result in an error directly.
```
SELECT STDDEV(data) FROM DECIMALV2_10_0_DATA;
ERROR 1105 (HY000): errCode = 2, detailMessage =
(127.0.0.1)[INTERNAL_ERROR]Agg Function stddev(decimal(10,0)) is not implemented
```
After removing DecimalV2, parameters of type DecimalV2 will be converted
to double for calculations.
---
.../aggregate_function_stddev.cpp | 9 --
.../aggregate_function_stddev.h | 105 +--------------------
.../trees/expressions/functions/agg/Stddev.java | 5 +-
.../expressions/functions/agg/StddevSamp.java | 5 +-
.../trees/expressions/functions/agg/Variance.java | 5 +-
.../expressions/functions/agg/VarianceSamp.java | 5 +-
6 files changed, 8 insertions(+), 126 deletions(-)
diff --git a/be/src/vec/aggregate_functions/aggregate_function_stddev.cpp
b/be/src/vec/aggregate_functions/aggregate_function_stddev.cpp
index ed13e0f5ea0..1d977c1c528 100644
--- a/be/src/vec/aggregate_functions/aggregate_function_stddev.cpp
+++ b/be/src/vec/aggregate_functions/aggregate_function_stddev.cpp
@@ -46,15 +46,6 @@ AggregateFunctionPtr create_function_single_value(const
String& name,
FOR_NUMERIC_TYPES(DISPATCH)
#undef DISPATCH
-#define DISPATCH(TYPE)
\
- if (which.idx == TypeIndex::TYPE)
\
- return creator_without_type::create<AggregateFunctionTemplate<
\
- NameData<Data<TYPE, BaseDatadecimal<TYPE, is_stddev>>>,
is_nullable>>( \
- custom_nullable ? remove_nullable(argument_types) :
argument_types, \
- result_is_nullable);
- FOR_DECIMAL_TYPES(DISPATCH)
-#undef DISPATCH
-
LOG(WARNING) << fmt::format("create_function_single_value with unknowed
type {}",
argument_types[0]->get_name());
return nullptr;
diff --git a/be/src/vec/aggregate_functions/aggregate_function_stddev.h
b/be/src/vec/aggregate_functions/aggregate_function_stddev.h
index ee6e98f341f..496212bc35c 100644
--- a/be/src/vec/aggregate_functions/aggregate_function_stddev.h
+++ b/be/src/vec/aggregate_functions/aggregate_function_stddev.h
@@ -20,20 +20,16 @@
#include <stddef.h>
#include <stdint.h>
-#include <algorithm>
#include <boost/iterator/iterator_facade.hpp>
#include <cmath>
#include <memory>
#include <type_traits>
#include "agent/be_exec_version_manager.h"
-#include "olap/olap_common.h"
-#include "runtime/decimalv2_value.h"
#include "vec/aggregate_functions/aggregate_function.h"
#include "vec/columns/column.h"
#include "vec/columns/column_nullable.h"
#include "vec/common/assert_cast.h"
-#include "vec/core/field.h"
#include "vec/core/types.h"
#include "vec/data_types/data_type_decimal.h"
#include "vec/data_types/data_type_number.h"
@@ -126,103 +122,6 @@ struct BaseData {
int64_t count;
};
-template <typename T, bool is_stddev>
-struct BaseDatadecimal {
- BaseDatadecimal() : mean(0), m2(0), count(0) {}
- virtual ~BaseDatadecimal() = default;
-
- void write(BufferWritable& buf) const {
- write_binary(mean, buf);
- write_binary(m2, buf);
- write_binary(count, buf);
- }
-
- void read(BufferReadable& buf) {
- read_binary(mean, buf);
- read_binary(m2, buf);
- read_binary(count, buf);
- }
-
- void reset() {
- mean = DecimalV2Value();
- m2 = DecimalV2Value();
- count = {};
- }
-
- DecimalV2Value get_result(DecimalV2Value res) const {
- if constexpr (is_stddev) {
- return DecimalV2Value::sqrt(res);
- } else {
- return res;
- }
- }
-
- DecimalV2Value get_pop_result() const {
- DecimalV2Value new_count = DecimalV2Value();
- if (count == 1) {
- return new_count;
- }
- DecimalV2Value res = m2 / new_count.assign_from_double(count);
- return get_result(res);
- }
-
- DecimalV2Value get_samp_result() const {
- DecimalV2Value new_count = DecimalV2Value();
- DecimalV2Value res = m2 / new_count.assign_from_double(count - 1);
- return get_result(res);
- }
-
- void merge(const BaseDatadecimal& rhs) {
- if (rhs.count == 0) {
- return;
- }
- DecimalV2Value new_count = DecimalV2Value();
- new_count.assign_from_double(count);
- DecimalV2Value rhs_count = DecimalV2Value();
- rhs_count.assign_from_double(rhs.count);
-
- DecimalV2Value delta = mean - rhs.mean;
- DecimalV2Value sum_count = new_count + rhs_count;
- mean = rhs.mean + delta * (new_count / sum_count);
- m2 = rhs.m2 + m2 + (delta * delta) * (rhs_count * new_count /
sum_count);
- count += rhs.count;
- }
-
- void add(const IColumn* column, size_t row_num) {
- const auto& sources = assert_cast<const ColumnDecimal<T>&>(*column);
- Field field = sources[row_num];
- auto decimal_field = field.template get<DecimalField<T>>();
- int128_t value;
- if (decimal_field.get_scale() > DecimalV2Value::SCALE) {
- value = static_cast<int128_t>(decimal_field.get_value()) /
- (decimal_field.get_scale_multiplier() /
DecimalV2Value::ONE_BILLION);
- } else {
- value = static_cast<int128_t>(decimal_field.get_value()) *
- (DecimalV2Value::ONE_BILLION /
decimal_field.get_scale_multiplier());
- }
- DecimalV2Value source_data = DecimalV2Value(value);
-
- DecimalV2Value new_count = DecimalV2Value();
- new_count.assign_from_double(count);
- DecimalV2Value increase_count = DecimalV2Value();
- increase_count.assign_from_double(1 + count);
-
- DecimalV2Value delta = source_data - mean;
- DecimalV2Value r = delta / increase_count;
- mean += r;
- m2 += new_count * delta * r;
- count += 1;
- }
-
- static DataTypePtr get_return_type() {
- return std::make_shared<DataTypeDecimal<Decimal128V2>>(27, 9);
- }
-
- DecimalV2Value mean;
- DecimalV2Value m2;
- int64_t count;
-};
-
template <typename T, typename Data>
struct PopData : Data {
using ColVecResult =
@@ -237,6 +136,10 @@ struct PopData : Data {
}
};
+// For this series of functions, the Decimal type is not supported
+// because the operations involve squaring,
+// which can easily exceed the range of the Decimal type.
+
template <typename Data>
struct StddevName : Data {
static const char* name() { return "stddev"; }
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/agg/Stddev.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/agg/Stddev.java
index 6cbebbc0ecf..1f732421940 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/agg/Stddev.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/agg/Stddev.java
@@ -24,7 +24,6 @@ import
org.apache.doris.nereids.trees.expressions.functions.ExplicitlyCastableSi
import org.apache.doris.nereids.trees.expressions.shape.UnaryExpression;
import org.apache.doris.nereids.trees.expressions.visitor.ExpressionVisitor;
import org.apache.doris.nereids.types.BigIntType;
-import org.apache.doris.nereids.types.DecimalV2Type;
import org.apache.doris.nereids.types.DoubleType;
import org.apache.doris.nereids.types.FloatType;
import org.apache.doris.nereids.types.IntegerType;
@@ -49,9 +48,7 @@ public class Stddev extends NullableAggregateFunction
FunctionSignature.ret(DoubleType.INSTANCE).args(SmallIntType.INSTANCE),
FunctionSignature.ret(DoubleType.INSTANCE).args(IntegerType.INSTANCE),
FunctionSignature.ret(DoubleType.INSTANCE).args(BigIntType.INSTANCE),
-
FunctionSignature.ret(DoubleType.INSTANCE).args(FloatType.INSTANCE),
-
FunctionSignature.ret(DecimalV2Type.SYSTEM_DEFAULT).args(DecimalV2Type.SYSTEM_DEFAULT)
- );
+
FunctionSignature.ret(DoubleType.INSTANCE).args(FloatType.INSTANCE));
/**
* constructor with 1 argument.
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/agg/StddevSamp.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/agg/StddevSamp.java
index 234715b28dd..42e909ed0ce 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/agg/StddevSamp.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/agg/StddevSamp.java
@@ -24,7 +24,6 @@ import
org.apache.doris.nereids.trees.expressions.functions.ExplicitlyCastableSi
import org.apache.doris.nereids.trees.expressions.shape.UnaryExpression;
import org.apache.doris.nereids.trees.expressions.visitor.ExpressionVisitor;
import org.apache.doris.nereids.types.BigIntType;
-import org.apache.doris.nereids.types.DecimalV2Type;
import org.apache.doris.nereids.types.DoubleType;
import org.apache.doris.nereids.types.FloatType;
import org.apache.doris.nereids.types.IntegerType;
@@ -49,9 +48,7 @@ public class StddevSamp extends NullableAggregateFunction
FunctionSignature.ret(DoubleType.INSTANCE).args(SmallIntType.INSTANCE),
FunctionSignature.ret(DoubleType.INSTANCE).args(IntegerType.INSTANCE),
FunctionSignature.ret(DoubleType.INSTANCE).args(BigIntType.INSTANCE),
-
FunctionSignature.ret(DoubleType.INSTANCE).args(FloatType.INSTANCE),
-
FunctionSignature.ret(DecimalV2Type.SYSTEM_DEFAULT).args(DecimalV2Type.SYSTEM_DEFAULT)
- );
+
FunctionSignature.ret(DoubleType.INSTANCE).args(FloatType.INSTANCE));
/**
* constructor with 1 argument.
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/agg/Variance.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/agg/Variance.java
index 79fbcfb7646..f56bfa6f6b6 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/agg/Variance.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/agg/Variance.java
@@ -24,7 +24,6 @@ import
org.apache.doris.nereids.trees.expressions.functions.ExplicitlyCastableSi
import org.apache.doris.nereids.trees.expressions.shape.UnaryExpression;
import org.apache.doris.nereids.trees.expressions.visitor.ExpressionVisitor;
import org.apache.doris.nereids.types.BigIntType;
-import org.apache.doris.nereids.types.DecimalV2Type;
import org.apache.doris.nereids.types.DoubleType;
import org.apache.doris.nereids.types.FloatType;
import org.apache.doris.nereids.types.IntegerType;
@@ -49,9 +48,7 @@ public class Variance extends NullableAggregateFunction
FunctionSignature.ret(DoubleType.INSTANCE).args(SmallIntType.INSTANCE),
FunctionSignature.ret(DoubleType.INSTANCE).args(IntegerType.INSTANCE),
FunctionSignature.ret(DoubleType.INSTANCE).args(BigIntType.INSTANCE),
-
FunctionSignature.ret(DoubleType.INSTANCE).args(FloatType.INSTANCE),
-
FunctionSignature.ret(DecimalV2Type.SYSTEM_DEFAULT).args(DecimalV2Type.SYSTEM_DEFAULT)
- );
+
FunctionSignature.ret(DoubleType.INSTANCE).args(FloatType.INSTANCE));
/**
* constructor with 1 argument.
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/agg/VarianceSamp.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/agg/VarianceSamp.java
index 65f2018d11e..f7e234c634c 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/agg/VarianceSamp.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/agg/VarianceSamp.java
@@ -23,7 +23,6 @@ import
org.apache.doris.nereids.trees.expressions.functions.ExplicitlyCastableSi
import org.apache.doris.nereids.trees.expressions.shape.UnaryExpression;
import org.apache.doris.nereids.trees.expressions.visitor.ExpressionVisitor;
import org.apache.doris.nereids.types.BigIntType;
-import org.apache.doris.nereids.types.DecimalV2Type;
import org.apache.doris.nereids.types.DoubleType;
import org.apache.doris.nereids.types.FloatType;
import org.apache.doris.nereids.types.IntegerType;
@@ -48,9 +47,7 @@ public class VarianceSamp extends NullableAggregateFunction
FunctionSignature.ret(DoubleType.INSTANCE).args(SmallIntType.INSTANCE),
FunctionSignature.ret(DoubleType.INSTANCE).args(IntegerType.INSTANCE),
FunctionSignature.ret(DoubleType.INSTANCE).args(BigIntType.INSTANCE),
-
FunctionSignature.ret(DoubleType.INSTANCE).args(FloatType.INSTANCE),
-
FunctionSignature.ret(DecimalV2Type.SYSTEM_DEFAULT).args(DecimalV2Type.SYSTEM_DEFAULT)
- );
+
FunctionSignature.ret(DoubleType.INSTANCE).args(FloatType.INSTANCE));
/**
* constructor with 1 argument.
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org
