You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by yi...@apache.org on 2024/09/05 11:51:08 UTC
(doris) branch branch-2.1 updated: [fix](unary function) Fix wrong result of asin, acos and sqrt when processing invalid input #40267 (#40358)
This is an automated email from the ASF dual-hosted git repository.
yiguolei pushed a commit to branch branch-2.1
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/branch-2.1 by this push:
new 0928c9c6ede [fix](unary function) Fix wrong result of asin, acos and sqrt when processing invalid input #40267 (#40358)
0928c9c6ede is described below
commit 0928c9c6ede11863dc1f7e47d00c2df51c500aef
Author: zhiqiang <se...@163.com>
AuthorDate: Thu Sep 5 19:51:01 2024 +0800
[fix](unary function) Fix wrong result of asin, acos and sqrt when processing invalid input #40267 (#40358)
cherry pick from #40267
---
.../functions/function_math_unary_alway_nullable.h | 94 +++++++++++++++++++++
be/src/vec/functions/math.cpp | 16 +++-
.../trees/expressions/functions/scalar/Acos.java | 4 +-
.../trees/expressions/functions/scalar/Asin.java | 4 +-
.../trees/expressions/functions/scalar/Dsqrt.java | 4 +-
.../trees/expressions/functions/scalar/Sqrt.java | 4 +-
.../test_math_unary_always_nullable.out | 95 ++++++++++++++++++++++
.../test_math_unary_always_nullable.groovy | 81 ++++++++++++++++++
8 files changed, 291 insertions(+), 11 deletions(-)
diff --git a/be/src/vec/functions/function_math_unary_alway_nullable.h b/be/src/vec/functions/function_math_unary_alway_nullable.h
new file mode 100644
index 00000000000..8d2cea1bc0d
--- /dev/null
+++ b/be/src/vec/functions/function_math_unary_alway_nullable.h
@@ -0,0 +1,94 @@
+// 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.
+
+#pragma once
+
+#include "vec/columns/column.h"
+#include "vec/columns/column_decimal.h"
+#include "vec/columns/column_nullable.h"
+#include "vec/columns/columns_number.h"
+#include "vec/core/call_on_type_index.h"
+#include "vec/core/types.h"
+#include "vec/data_types/data_type_decimal.h"
+#include "vec/data_types/data_type_nullable.h"
+#include "vec/data_types/data_type_number.h"
+#include "vec/functions/function.h"
+#include "vec/functions/function_helpers.h"
+#include "vec/utils/util.hpp"
+
+namespace doris::vectorized {
+
+template <typename Impl>
+class FunctionMathUnaryAlwayNullable : public IFunction {
+public:
+ using IFunction::execute;
+
+ static constexpr auto name = Impl::name;
+ static FunctionPtr create() { return std::make_shared<FunctionMathUnaryAlwayNullable>(); }
+
+private:
+ String get_name() const override { return name; }
+ size_t get_number_of_arguments() const override { return 1; }
+
+ DataTypePtr get_return_type_impl(const DataTypes& arguments) const override {
+ return make_nullable(std::make_shared<typename Impl::Type>());
+ }
+
+ static void execute_in_iterations(const double* src_data, double* dst_data, size_t size) {
+ for (size_t i = 0; i < size; i++) {
+ Impl::execute(&src_data[i], &dst_data[i]);
+ }
+ }
+
+ Status execute_impl(FunctionContext* context, Block& block, const ColumnNumbers& arguments,
+ size_t result, size_t input_rows_count) const override {
+ const ColumnFloat64* col =
+ assert_cast<const ColumnFloat64*>(block.get_by_position(arguments[0]).column.get());
+ auto dst = ColumnFloat64::create();
+ auto& dst_data = dst->get_data();
+ dst_data.resize(input_rows_count);
+
+ execute_in_iterations(col->get_data().data(), dst_data.data(), input_rows_count);
+
+ auto result_null_map = ColumnUInt8::create(input_rows_count, 0);
+
+ for (size_t i = 0; i < input_rows_count; i++) {
+ if (Impl::is_invalid_input(col->get_data()[i])) [[unlikely]] {
+ result_null_map->get_data().data()[i] = 1;
+ }
+ }
+
+ block.replace_by_position(
+ result, ColumnNullable::create(std::move(dst), std::move(result_null_map)));
+ return Status::OK();
+ }
+};
+
+template <typename Name, Float64(Function)(Float64)>
+struct UnaryFunctionPlainAlwayNullable {
+ using Type = DataTypeFloat64;
+ static constexpr auto name = Name::name;
+
+ static constexpr bool is_invalid_input(Float64 x) { return Name::is_invalid_input(x); }
+
+ template <typename T, typename U>
+ static void execute(const T* src, U* dst) {
+ *dst = static_cast<Float64>(Function(*src));
+ }
+};
+
+} // namespace doris::vectorized
diff --git a/be/src/vec/functions/math.cpp b/be/src/vec/functions/math.cpp
index bc086ce4447..7c55866e2c4 100644
--- a/be/src/vec/functions/math.cpp
+++ b/be/src/vec/functions/math.cpp
@@ -35,6 +35,7 @@
#include "vec/functions/function_const.h"
#include "vec/functions/function_math_log.h"
#include "vec/functions/function_math_unary.h"
+#include "vec/functions/function_math_unary_alway_nullable.h"
#include "vec/functions/function_string.h"
#include "vec/functions/function_totype.h"
#include "vec/functions/function_unary_arithmetic.h"
@@ -51,13 +52,19 @@ struct Log2Impl;
namespace doris::vectorized {
struct AcosName {
static constexpr auto name = "acos";
+ // https://dev.mysql.com/doc/refman/8.4/en/mathematical-functions.html#function_acos
+ static constexpr bool is_invalid_input(Float64 x) { return x < -1 || x > 1; }
};
-using FunctionAcos = FunctionMathUnary<UnaryFunctionPlain<AcosName, std::acos>>;
+using FunctionAcos =
+ FunctionMathUnaryAlwayNullable<UnaryFunctionPlainAlwayNullable<AcosName, std::acos>>;
struct AsinName {
static constexpr auto name = "asin";
+ // https://dev.mysql.com/doc/refman/8.4/en/mathematical-functions.html#function_asin
+ static constexpr bool is_invalid_input(Float64 x) { return x < -1 || x > 1; }
};
-using FunctionAsin = FunctionMathUnary<UnaryFunctionPlain<AsinName, std::asin>>;
+using FunctionAsin =
+ FunctionMathUnaryAlwayNullable<UnaryFunctionPlainAlwayNullable<AsinName, std::asin>>;
struct AtanName {
static constexpr auto name = "atan";
@@ -221,8 +228,11 @@ using FunctionSin = FunctionMathUnary<UnaryFunctionPlain<SinName, std::sin>>;
struct SqrtName {
static constexpr auto name = "sqrt";
+ // https://dev.mysql.com/doc/refman/8.4/en/mathematical-functions.html#function_sqrt
+ static constexpr bool is_invalid_input(Float64 x) { return x < 0; }
};
-using FunctionSqrt = FunctionMathUnary<UnaryFunctionPlain<SqrtName, std::sqrt>>;
+using FunctionSqrt =
+ FunctionMathUnaryAlwayNullable<UnaryFunctionPlainAlwayNullable<SqrtName, std::sqrt>>;
struct CbrtName {
static constexpr auto name = "cbrt";
diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/Acos.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/Acos.java
index c99af81123f..2193221c326 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/Acos.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/Acos.java
@@ -19,8 +19,8 @@ package org.apache.doris.nereids.trees.expressions.functions.scalar;
import org.apache.doris.catalog.FunctionSignature;
import org.apache.doris.nereids.trees.expressions.Expression;
+import org.apache.doris.nereids.trees.expressions.functions.AlwaysNullable;
import org.apache.doris.nereids.trees.expressions.functions.ExplicitlyCastableSignature;
-import org.apache.doris.nereids.trees.expressions.functions.PropagateNullable;
import org.apache.doris.nereids.trees.expressions.shape.UnaryExpression;
import org.apache.doris.nereids.trees.expressions.visitor.ExpressionVisitor;
import org.apache.doris.nereids.types.DoubleType;
@@ -34,7 +34,7 @@ import java.util.List;
* ScalarFunction 'acos'. This class is generated by GenerateFunction.
*/
public class Acos extends ScalarFunction
- implements UnaryExpression, ExplicitlyCastableSignature, PropagateNullable {
+ implements UnaryExpression, ExplicitlyCastableSignature, AlwaysNullable {
public static final List<FunctionSignature> SIGNATURES = ImmutableList.of(
FunctionSignature.ret(DoubleType.INSTANCE).args(DoubleType.INSTANCE)
diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/Asin.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/Asin.java
index 0e06d8d77ed..22e1ff59b7d 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/Asin.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/Asin.java
@@ -19,8 +19,8 @@ package org.apache.doris.nereids.trees.expressions.functions.scalar;
import org.apache.doris.catalog.FunctionSignature;
import org.apache.doris.nereids.trees.expressions.Expression;
+import org.apache.doris.nereids.trees.expressions.functions.AlwaysNullable;
import org.apache.doris.nereids.trees.expressions.functions.ExplicitlyCastableSignature;
-import org.apache.doris.nereids.trees.expressions.functions.PropagateNullable;
import org.apache.doris.nereids.trees.expressions.shape.UnaryExpression;
import org.apache.doris.nereids.trees.expressions.visitor.ExpressionVisitor;
import org.apache.doris.nereids.types.DoubleType;
@@ -34,7 +34,7 @@ import java.util.List;
* ScalarFunction 'asin'. This class is generated by GenerateFunction.
*/
public class Asin extends ScalarFunction
- implements UnaryExpression, ExplicitlyCastableSignature, PropagateNullable {
+ implements UnaryExpression, ExplicitlyCastableSignature, AlwaysNullable {
public static final List<FunctionSignature> SIGNATURES = ImmutableList.of(
FunctionSignature.ret(DoubleType.INSTANCE).args(DoubleType.INSTANCE)
diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/Dsqrt.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/Dsqrt.java
index 874befd09db..3caef79776b 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/Dsqrt.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/Dsqrt.java
@@ -19,8 +19,8 @@ package org.apache.doris.nereids.trees.expressions.functions.scalar;
import org.apache.doris.catalog.FunctionSignature;
import org.apache.doris.nereids.trees.expressions.Expression;
+import org.apache.doris.nereids.trees.expressions.functions.AlwaysNullable;
import org.apache.doris.nereids.trees.expressions.functions.ExplicitlyCastableSignature;
-import org.apache.doris.nereids.trees.expressions.functions.PropagateNullable;
import org.apache.doris.nereids.trees.expressions.shape.UnaryExpression;
import org.apache.doris.nereids.trees.expressions.visitor.ExpressionVisitor;
import org.apache.doris.nereids.types.DoubleType;
@@ -34,7 +34,7 @@ import java.util.List;
* ScalarFunction 'dsqrt'. This class is generated by GenerateFunction.
*/
public class Dsqrt extends ScalarFunction
- implements UnaryExpression, ExplicitlyCastableSignature, PropagateNullable {
+ implements UnaryExpression, ExplicitlyCastableSignature, AlwaysNullable {
public static final List<FunctionSignature> SIGNATURES = ImmutableList.of(
FunctionSignature.ret(DoubleType.INSTANCE).args(DoubleType.INSTANCE)
diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/Sqrt.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/Sqrt.java
index 495321c6dfa..f954eb07a54 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/Sqrt.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/Sqrt.java
@@ -19,8 +19,8 @@ package org.apache.doris.nereids.trees.expressions.functions.scalar;
import org.apache.doris.catalog.FunctionSignature;
import org.apache.doris.nereids.trees.expressions.Expression;
+import org.apache.doris.nereids.trees.expressions.functions.AlwaysNullable;
import org.apache.doris.nereids.trees.expressions.functions.ExplicitlyCastableSignature;
-import org.apache.doris.nereids.trees.expressions.functions.PropagateNullable;
import org.apache.doris.nereids.trees.expressions.shape.UnaryExpression;
import org.apache.doris.nereids.trees.expressions.visitor.ExpressionVisitor;
import org.apache.doris.nereids.types.DoubleType;
@@ -34,7 +34,7 @@ import java.util.List;
* ScalarFunction 'sqrt'. This class is generated by GenerateFunction.
*/
public class Sqrt extends ScalarFunction
- implements UnaryExpression, ExplicitlyCastableSignature, PropagateNullable {
+ implements UnaryExpression, ExplicitlyCastableSignature, AlwaysNullable {
public static final List<FunctionSignature> SIGNATURES = ImmutableList.of(
FunctionSignature.ret(DoubleType.INSTANCE).args(DoubleType.INSTANCE)
diff --git a/regression-test/data/query_p0/sql_functions/math_functions/test_math_unary_always_nullable.out b/regression-test/data/query_p0/sql_functions/math_functions/test_math_unary_always_nullable.out
new file mode 100644
index 00000000000..0a190f0bd6b
--- /dev/null
+++ b/regression-test/data/query_p0/sql_functions/math_functions/test_math_unary_always_nullable.out
@@ -0,0 +1,95 @@
+-- This file is automatically generated. You should know what you did if you want to edit this
+-- !acos_1 --
+\N true
+
+-- !acos_2 --
+\N true
+
+-- !acos_3 --
+\N true 0
+\N true 1
+\N true 2
+\N true 3
+\N true 4
+\N true 5
+\N true 6
+\N true 7
+\N true 8
+\N true 9
+
+-- !asin_1 --
+\N true
+
+-- !asin_2 --
+\N true
+
+-- !asin_3 --
+\N true 0
+\N true 1
+\N true 2
+\N true 3
+\N true 4
+\N true 5
+\N true 6
+\N true 7
+\N true 8
+\N true 9
+
+-- !sqrt_1 --
+\N true
+
+-- !sqrt_2 --
+\N true
+
+-- !sqrt_3 --
+\N true 0
+\N true 1
+\N true 2
+\N true 3
+\N true 4
+\N true 5
+\N true 6
+\N true 7
+\N true 8
+\N true 9
+
+-- !acos_tbl_1 --
+1 \N true
+2 \N true
+3 1.5707963267948966 false
+4 \N true
+5 \N true
+6 \N true
+7 \N true
+8 \N true
+
+-- !asin_tbl_1 --
+1 \N true
+2 \N true
+3 0.0 false
+4 \N true
+5 \N true
+6 \N true
+7 \N true
+8 \N true
+
+-- !sqrt_tbl_1 --
+1 1.0488088481701516 false
+2 \N true
+3 0.0 false
+4 \N true
+5 \N true
+6 \N true
+7 \N true
+8 \N true
+
+-- !dsqrt_tbl_1 --
+1 1.0488088481701516 false
+2 \N true
+3 0.0 false
+4 \N true
+5 \N true
+6 \N true
+7 \N true
+8 \N true
+
diff --git a/regression-test/suites/query_p0/sql_functions/math_functions/test_math_unary_always_nullable.groovy b/regression-test/suites/query_p0/sql_functions/math_functions/test_math_unary_always_nullable.groovy
new file mode 100644
index 00000000000..f8907f4c21e
--- /dev/null
+++ b/regression-test/suites/query_p0/sql_functions/math_functions/test_math_unary_always_nullable.groovy
@@ -0,0 +1,81 @@
+// 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_math_unary_alway_nullable") {
+ qt_acos_1 """
+ select acos(1.1), acos(1.1) is null;
+ """
+ qt_acos_2 """
+ select acos(-1.1), acos(-1.1) is null;
+ """
+ qt_acos_3 """
+ select acos(-1.1), acos(-1.1) is NULL, number from numbers("number"="10")
+ """
+
+ qt_asin_1 """
+ select asin(1.1), asin(1.1) is null;
+ """
+ qt_asin_2 """
+ select asin(-1.1), asin(-1.1) is null;
+ """
+ qt_asin_3 """
+ select asin(-1.1), asin(-1.1) is NULL, number from numbers("number"="10")
+ """
+
+ qt_sqrt_1 """
+ select sqrt(-1), sqrt(-1) is null;
+ """
+ qt_sqrt_2 """
+ select sqrt(-1.1), sqrt(-1.1) is null;
+ """
+ qt_sqrt_3 """
+ select sqrt(-1.1), sqrt(-1.1) is NULL, number from numbers("number"="10")
+ """
+
+ sql "drop table if exists test_math_unary_alway_nullable"
+
+ sql """
+ create table if not exists test_math_unary_alway_nullable (rowid int, val double NULL)
+ distributed by hash(rowid) properties ("replication_num"="1");
+ """
+
+ sql """
+ insert into test_math_unary_alway_nullable values
+ (1, 1.1), (2, -1.1), (3, 0), (4, NULL)
+ """
+ sql """
+ insert into test_math_unary_alway_nullable values
+ (5, NULL), (6, NULL), (7, NULL), (8, NULL)
+ """
+
+ qt_acos_tbl_1 """
+ select rowid, acos(val), acos(val) is null from test_math_unary_alway_nullable order by rowid;
+ """
+
+ qt_asin_tbl_1 """
+ select rowid, asin(val), asin(val) is null from test_math_unary_alway_nullable order by rowid;
+ """
+
+ qt_sqrt_tbl_1 """
+ select rowid, sqrt(val), sqrt(val) is null from test_math_unary_alway_nullable order by rowid;
+ """
+
+ qt_dsqrt_tbl_1 """
+ select rowid, dsqrt(val), dsqrt(val) is null from test_math_unary_alway_nullable order by rowid;
+ """
+
+}
\ No newline at end of file
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org