This is an automated email from the ASF dual-hosted git repository.
liuneng pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/incubator-gluten.git
The following commit(s) were added to refs/heads/main by this push:
new 9cf62cf24 [CH] Fix diff of factorial function (#5330)
9cf62cf24 is described below
commit 9cf62cf2434d198dffc4bc524cb9056b6b6924cd
Author: exmy <[email protected]>
AuthorDate: Fri Apr 12 15:20:31 2024 +0800
[CH] Fix diff of factorial function (#5330)
What changes were proposed in this pull request?
Fix diff of factorial function, factorial(x) should return null when x > 20
or x < 0
enable spark ut about ceil and floor
How was this patch tested?
PASS CI
---
cpp-ch/local-engine/Parser/SerializedPlanParser.h | 1 -
.../Parser/scalar_function_parser/factorial.cpp | 74 ++++++++++++++++++++++
.../utils/clickhouse/ClickHouseTestSettings.scala | 3 -
.../utils/clickhouse/ClickHouseTestSettings.scala | 3 -
4 files changed, 74 insertions(+), 7 deletions(-)
diff --git a/cpp-ch/local-engine/Parser/SerializedPlanParser.h
b/cpp-ch/local-engine/Parser/SerializedPlanParser.h
index d929aabad..cac16426e 100644
--- a/cpp-ch/local-engine/Parser/SerializedPlanParser.h
+++ b/cpp-ch/local-engine/Parser/SerializedPlanParser.h
@@ -114,7 +114,6 @@ static const std::map<std::string, std::string>
SCALAR_FUNCTIONS
{"shiftleft", "bitShiftLeft"},
{"shiftright", "bitShiftRight"},
{"check_overflow", "checkDecimalOverflowSpark"},
- {"factorial", "factorial"},
{"rand", "randCanonical"},
{"isnan", "isNaN"},
diff --git a/cpp-ch/local-engine/Parser/scalar_function_parser/factorial.cpp
b/cpp-ch/local-engine/Parser/scalar_function_parser/factorial.cpp
new file mode 100644
index 000000000..c2f0a383b
--- /dev/null
+++ b/cpp-ch/local-engine/Parser/scalar_function_parser/factorial.cpp
@@ -0,0 +1,74 @@
+/*
+ * 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.
+ */
+
+#include <Parser/FunctionParser.h>
+#include <DataTypes/IDataType.h>
+#include <DataTypes/DataTypeNullable.h>
+
+namespace DB
+{
+
+namespace ErrorCodes
+{
+ extern const int NUMBER_OF_ARGUMENTS_DOESNT_MATCH;
+}
+}
+
+namespace local_engine
+{
+
+class FunctionParserFactorial : public FunctionParser
+{
+public:
+ explicit FunctionParserFactorial(SerializedPlanParser * plan_parser_) :
FunctionParser(plan_parser_) {}
+ ~FunctionParserFactorial() override = default;
+
+ static constexpr auto name = "factorial";
+
+ String getName() const override { return name; }
+
+ const ActionsDAG::Node * parse(
+ const substrait::Expression_ScalarFunction & substrait_func,
+ ActionsDAGPtr & actions_dag) const override
+ {
+ /// parse factorial(x) as if (x > 20 || x < 0) null else factorial(x)
+ auto parsed_args = parseFunctionArguments(substrait_func, "",
actions_dag);
+ if (parsed_args.size() != 1)
+ throw Exception(ErrorCodes::NUMBER_OF_ARGUMENTS_DOESNT_MATCH,
"Function {} requires exactly one arguments", getName());
+
+ const auto * x = parsed_args[0];
+
+ const auto * zero_const_node = addColumnToActionsDAG(actions_dag,
std::make_shared<DataTypeInt32>(), 0);
+ const auto * twenty_const_node = addColumnToActionsDAG(actions_dag,
std::make_shared<DataTypeInt32>(), 20);
+
+ const auto * greater_than_twenty_node = toFunctionNode(actions_dag,
"greater", {x, twenty_const_node});
+ const auto * less_than_zero_node = toFunctionNode(actions_dag, "less",
{x, zero_const_node});
+ const auto * or_node = toFunctionNode(actions_dag, "or",
{greater_than_twenty_node, less_than_zero_node});
+
+ // tricky: use tricky_if_node instead of x node to avoid exception:
The maximum value for the input argument of function factorial is 20
+ const auto * tricky_if_node = toFunctionNode(actions_dag, "if",
{or_node, zero_const_node, x});
+ const auto * factorial_node = toFunctionNode(actions_dag, "factorial",
{tricky_if_node});
+ const auto * null_const_node = addColumnToActionsDAG(actions_dag,
makeNullable(factorial_node->result_type), Field());
+
+ const auto * result_node = toFunctionNode(actions_dag, "if", {or_node,
null_const_node, factorial_node});
+
+ return convertNodeTypeIfNeeded(substrait_func, result_node,
actions_dag);;
+ }
+};
+
+static FunctionParserRegister<FunctionParserFactorial> register_factorial;
+}
diff --git
a/gluten-ut/spark32/src/test/scala/org/apache/gluten/utils/clickhouse/ClickHouseTestSettings.scala
b/gluten-ut/spark32/src/test/scala/org/apache/gluten/utils/clickhouse/ClickHouseTestSettings.scala
index 8bae61559..9b987b0e4 100644
---
a/gluten-ut/spark32/src/test/scala/org/apache/gluten/utils/clickhouse/ClickHouseTestSettings.scala
+++
b/gluten-ut/spark32/src/test/scala/org/apache/gluten/utils/clickhouse/ClickHouseTestSettings.scala
@@ -839,9 +839,6 @@ class ClickHouseTestSettings extends BackendTestSettings {
.excludeGlutenTest("default")
enableSuite[GlutenMathExpressionsSuite]
.exclude("tanh")
- .exclude("ceil")
- .exclude("floor")
- .exclude("factorial")
.exclude("rint")
.exclude("expm1")
.exclude("unhex")
diff --git
a/gluten-ut/spark33/src/test/scala/org/apache/gluten/utils/clickhouse/ClickHouseTestSettings.scala
b/gluten-ut/spark33/src/test/scala/org/apache/gluten/utils/clickhouse/ClickHouseTestSettings.scala
index e59ca3f0d..8328723a5 100644
---
a/gluten-ut/spark33/src/test/scala/org/apache/gluten/utils/clickhouse/ClickHouseTestSettings.scala
+++
b/gluten-ut/spark33/src/test/scala/org/apache/gluten/utils/clickhouse/ClickHouseTestSettings.scala
@@ -844,9 +844,6 @@ class ClickHouseTestSettings extends BackendTestSettings {
.exclude("SPARK-37967: Literal.create support ObjectType")
enableSuite[GlutenMathExpressionsSuite]
.exclude("tanh")
- .exclude("ceil")
- .exclude("floor")
- .exclude("factorial")
.exclude("rint")
.exclude("expm1")
.exclude("bin")
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
