This is an automated email from the ASF dual-hosted git repository.
ulyssesyou 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 33f993554 [GLUTEN-4652][VL] Fix min_by/max_by result mismatch when RDD
partition num > 1 (#5711)
33f993554 is described below
commit 33f993554bebc388c7011dd91b86eaadc729f0d5
Author: zhouyifan279 <[email protected]>
AuthorDate: Mon May 13 19:03:41 2024 +0800
[GLUTEN-4652][VL] Fix min_by/max_by result mismatch when RDD partition num
> 1 (#5711)
---
.../execution/VeloxAggregateFunctionsSuite.scala | 18 ++++-------
.../functions/RegistrationAllFunctions.cc | 23 ++++++++------
.../functions/RowConstructorWithAllNull.h | 37 ----------------------
.../operators/functions/RowConstructorWithNull.cc | 10 +-----
.../operators/functions/RowConstructorWithNull.h | 8 +++++
5 files changed, 28 insertions(+), 68 deletions(-)
diff --git
a/backends-velox/src/test/scala/org/apache/gluten/execution/VeloxAggregateFunctionsSuite.scala
b/backends-velox/src/test/scala/org/apache/gluten/execution/VeloxAggregateFunctionsSuite.scala
index 398f5e05e..faa361edf 100644
---
a/backends-velox/src/test/scala/org/apache/gluten/execution/VeloxAggregateFunctionsSuite.scala
+++
b/backends-velox/src/test/scala/org/apache/gluten/execution/VeloxAggregateFunctionsSuite.scala
@@ -194,18 +194,12 @@ abstract class VeloxAggregateFunctionsSuite extends
VeloxWholeStageTransformerSu
}
test("min_by/max_by") {
- withTempPath {
- path =>
- Seq((5: Integer, 6: Integer), (null: Integer, 11: Integer), (null:
Integer, 5: Integer))
- .toDF("a", "b")
- .write
- .parquet(path.getCanonicalPath)
- spark.read
- .parquet(path.getCanonicalPath)
- .createOrReplaceTempView("test")
- runQueryAndCompare("select min_by(a, b), max_by(a, b) from test") {
- checkGlutenOperatorMatch[HashAggregateExecTransformer]
- }
+ withSQLConf(("spark.sql.leafNodeDefaultParallelism", "2")) {
+ runQueryAndCompare(
+ "select min_by(a, b), max_by(a, b) from " +
+ "values (5, 6), (null, 11), (null, 5) test(a, b)") {
+ checkGlutenOperatorMatch[HashAggregateExecTransformer]
+ }
}
}
diff --git a/cpp/velox/operators/functions/RegistrationAllFunctions.cc
b/cpp/velox/operators/functions/RegistrationAllFunctions.cc
index c77fa47e5..5a6b0f6aa 100644
--- a/cpp/velox/operators/functions/RegistrationAllFunctions.cc
+++ b/cpp/velox/operators/functions/RegistrationAllFunctions.cc
@@ -15,11 +15,10 @@
* limitations under the License.
*/
#include "operators/functions/RegistrationAllFunctions.h"
+
#include "operators/functions/Arithmetic.h"
-#include "operators/functions/RowConstructorWithAllNull.h"
#include "operators/functions/RowConstructorWithNull.h"
#include "operators/functions/RowFunctionWithNull.h"
-
#include "velox/expression/SpecialFormRegistry.h"
#include "velox/expression/VectorFunction.h"
#include "velox/functions/lib/RegistrationHelpers.h"
@@ -45,29 +44,32 @@ void registerFunctionOverwrite() {
velox::registerFunction<RoundFunction, double, double, int32_t>({"round"});
velox::registerFunction<RoundFunction, float, float, int32_t>({"round"});
+ auto kRowConstructorWithNull =
RowConstructorWithNullCallToSpecialForm::kRowConstructorWithNull;
velox::exec::registerVectorFunction(
- "row_constructor_with_null",
+ kRowConstructorWithNull,
std::vector<std::shared_ptr<velox::exec::FunctionSignature>>{},
std::make_unique<RowFunctionWithNull</*allNull=*/false>>(),
RowFunctionWithNull</*allNull=*/false>::metadata());
velox::exec::registerFunctionCallToSpecialForm(
- RowConstructorWithNullCallToSpecialForm::kRowConstructorWithNull,
- std::make_unique<RowConstructorWithNullCallToSpecialForm>());
+ kRowConstructorWithNull,
std::make_unique<RowConstructorWithNullCallToSpecialForm>(kRowConstructorWithNull));
+
+ auto kRowConstructorWithAllNull =
RowConstructorWithNullCallToSpecialForm::kRowConstructorWithAllNull;
velox::exec::registerVectorFunction(
- "row_constructor_with_all_null",
+ kRowConstructorWithAllNull,
std::vector<std::shared_ptr<velox::exec::FunctionSignature>>{},
std::make_unique<RowFunctionWithNull</*allNull=*/true>>(),
RowFunctionWithNull</*allNull=*/true>::metadata());
velox::exec::registerFunctionCallToSpecialForm(
- RowConstructorWithAllNullCallToSpecialForm::kRowConstructorWithAllNull,
- std::make_unique<RowConstructorWithAllNullCallToSpecialForm>());
+ kRowConstructorWithAllNull,
+
std::make_unique<RowConstructorWithNullCallToSpecialForm>(kRowConstructorWithAllNull));
velox::functions::sparksql::registerBitwiseFunctions("spark_");
}
} // namespace
void registerAllFunctions() {
// The registration order matters. Spark sql functions are registered after
- // presto sql functions to overwrite the registration for same named
functions.
+ // presto sql functions to overwrite the registration for same named
+ // functions.
velox::functions::prestosql::registerAllScalarFunctions();
velox::functions::sparksql::registerFunctions("");
velox::aggregate::prestosql::registerAllAggregateFunctions(
@@ -76,7 +78,8 @@ void registerAllFunctions() {
"", true /*registerCompanionFunctions*/, true /*overwrite*/);
velox::window::prestosql::registerAllWindowFunctions();
velox::functions::window::sparksql::registerWindowFunctions("");
- // Using function overwrite to handle function names mismatch between Spark
and Velox.
+ // Using function overwrite to handle function names mismatch between Spark
+ // and Velox.
registerFunctionOverwrite();
}
diff --git a/cpp/velox/operators/functions/RowConstructorWithAllNull.h
b/cpp/velox/operators/functions/RowConstructorWithAllNull.h
deleted file mode 100644
index dfc79e1a9..000000000
--- a/cpp/velox/operators/functions/RowConstructorWithAllNull.h
+++ /dev/null
@@ -1,37 +0,0 @@
-/*
- * 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 "RowConstructorWithNull.h"
-
-namespace gluten {
-class RowConstructorWithAllNullCallToSpecialForm : public
RowConstructorWithNullCallToSpecialForm {
- public:
- static constexpr const char* kRowConstructorWithAllNull =
"row_constructor_with_all_null";
-
- protected:
- facebook::velox::exec::ExprPtr constructSpecialForm(
- const std::string& name,
- const facebook::velox::TypePtr& type,
- std::vector<facebook::velox::exec::ExprPtr>&& compiledChildren,
- bool trackCpuUsage,
- const facebook::velox::core::QueryConfig& config) {
- return constructSpecialForm(kRowConstructorWithAllNull, type,
std::move(compiledChildren), trackCpuUsage, config);
- }
-};
-} // namespace gluten
diff --git a/cpp/velox/operators/functions/RowConstructorWithNull.cc
b/cpp/velox/operators/functions/RowConstructorWithNull.cc
index 955d957e2..e8b8a2883 100644
--- a/cpp/velox/operators/functions/RowConstructorWithNull.cc
+++ b/cpp/velox/operators/functions/RowConstructorWithNull.cc
@@ -32,11 +32,11 @@ facebook::velox::TypePtr
RowConstructorWithNullCallToSpecialForm::resolveType(
}
facebook::velox::exec::ExprPtr
RowConstructorWithNullCallToSpecialForm::constructSpecialForm(
- const std::string& name,
const facebook::velox::TypePtr& type,
std::vector<facebook::velox::exec::ExprPtr>&& compiledChildren,
bool trackCpuUsage,
const facebook::velox::core::QueryConfig& config) {
+ auto name = this->rowFunctionName;
auto [function, metadata] =
facebook::velox::exec::vectorFunctionFactories().withRLock(
[&config, &name](auto& functionMap) -> std::pair<
std::shared_ptr<facebook::velox::exec::VectorFunction>,
@@ -52,12 +52,4 @@ facebook::velox::exec::ExprPtr
RowConstructorWithNullCallToSpecialForm::construc
return std::make_shared<facebook::velox::exec::Expr>(
type, std::move(compiledChildren), function, metadata, name,
trackCpuUsage);
}
-
-facebook::velox::exec::ExprPtr
RowConstructorWithNullCallToSpecialForm::constructSpecialForm(
- const facebook::velox::TypePtr& type,
- std::vector<facebook::velox::exec::ExprPtr>&& compiledChildren,
- bool trackCpuUsage,
- const facebook::velox::core::QueryConfig& config) {
- return constructSpecialForm(kRowConstructorWithNull, type,
std::move(compiledChildren), trackCpuUsage, config);
-}
} // namespace gluten
diff --git a/cpp/velox/operators/functions/RowConstructorWithNull.h
b/cpp/velox/operators/functions/RowConstructorWithNull.h
index 6cfeaee37..66b745e3e 100644
--- a/cpp/velox/operators/functions/RowConstructorWithNull.h
+++ b/cpp/velox/operators/functions/RowConstructorWithNull.h
@@ -23,6 +23,10 @@
namespace gluten {
class RowConstructorWithNullCallToSpecialForm : public
facebook::velox::exec::FunctionCallToSpecialForm {
public:
+ RowConstructorWithNullCallToSpecialForm(const std::string& rowFunctionName) {
+ this->rowFunctionName = rowFunctionName;
+ }
+
facebook::velox::TypePtr resolveType(const
std::vector<facebook::velox::TypePtr>& argTypes) override;
facebook::velox::exec::ExprPtr constructSpecialForm(
@@ -32,6 +36,7 @@ class RowConstructorWithNullCallToSpecialForm : public
facebook::velox::exec::Fu
const facebook::velox::core::QueryConfig& config) override;
static constexpr const char* kRowConstructorWithNull =
"row_constructor_with_null";
+ static constexpr const char* kRowConstructorWithAllNull =
"row_constructor_with_all_null";
protected:
facebook::velox::exec::ExprPtr constructSpecialForm(
@@ -40,5 +45,8 @@ class RowConstructorWithNullCallToSpecialForm : public
facebook::velox::exec::Fu
std::vector<facebook::velox::exec::ExprPtr>&& compiledChildren,
bool trackCpuUsage,
const facebook::velox::core::QueryConfig& config);
+
+ private:
+ std::string rowFunctionName;
};
} // namespace gluten
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
