Skip to content

Commit

Permalink
Rename presto 'replace' function to 'presto_replace' to avoid conflic…
Browse files Browse the repository at this point in the history
…tion (workaround of 4922)
  • Loading branch information
JkSelf authored and glutenperfbot committed Mar 30, 2024
1 parent 5f3b678 commit 83f0ba8
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,10 @@ void registerStringFunctions(const std::string& prefix) {
Varchar,
Varchar>({prefix + "split_to_map"});
VELOX_REGISTER_VECTOR_FUNCTION(udf_concat, prefix + "concat");
VELOX_REGISTER_VECTOR_FUNCTION(udf_replace, prefix + "replace");
// In Gluten, presto 'replace' function is conflict with sparksql, because
// presto 'replace' is registered as VectorFunction, and spark 'replace' is
// registered as SimpleFunction. Velox respect VectorFunction first.
VELOX_REGISTER_VECTOR_FUNCTION(udf_replace, prefix + "presto_replace");
VELOX_REGISTER_VECTOR_FUNCTION(udf_reverse, prefix + "reverse");
VELOX_REGISTER_VECTOR_FUNCTION(udf_to_utf8, prefix + "to_utf8");
VELOX_REGISTER_VECTOR_FUNCTION(udf_from_utf8, prefix + "from_utf8");
Expand Down
17 changes: 9 additions & 8 deletions velox/functions/prestosql/tests/StringFunctionsTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1230,7 +1230,7 @@ void StringFunctionsTest::testReplaceInPlace(
};

auto result = evaluate<FlatVector<StringView>>(
fmt::format("replace(c0, '{}', '{}')", search, replace),
fmt::format("presto_replace(c0, '{}', '{}')", search, replace),
makeRowVector({makeInput()}));
testResults(result.get());

Expand All @@ -1239,8 +1239,8 @@ void StringFunctionsTest::testReplaceInPlace(
auto applyReplaceFunction = [&](std::vector<VectorPtr>& functionInputs,
VectorPtr& resultPtr) {
core::QueryConfig config({});
auto replaceFunction =
exec::getVectorFunction("replace", {VARCHAR(), VARCHAR()}, {}, config);
auto replaceFunction = exec::getVectorFunction(
"presto_replace", {VARCHAR(), VARCHAR()}, {}, config);
SelectivityVector rows(tests.size());
ExprSet exprSet({}, &execCtx_);
RowVectorPtr inputRows = makeRowVector({});
Expand Down Expand Up @@ -1285,11 +1285,11 @@ void StringFunctionsTest::testReplaceFlatVector(

if (withReplaceArgument) {
result = evaluate<FlatVector<StringView>>(
"replace(c0, c1, c2)",
"presto_replace(c0, c1, c2)",
makeRowVector({stringVector, searchVector, replaceVector}));
} else {
result = evaluate<FlatVector<StringView>>(
"replace(c0, c1)", makeRowVector({stringVector, searchVector}));
"presto_replace(c0, c1)", makeRowVector({stringVector, searchVector}));
}

for (int32_t i = 0; i < tests.size(); ++i) {
Expand Down Expand Up @@ -1331,8 +1331,8 @@ TEST_F(StringFunctionsTest, replace) {

// Test constant vectors
auto rows = makeRowVector(makeRowType({BIGINT()}), 10);
auto result =
evaluate<SimpleVector<StringView>>("replace('high', 'ig', 'f')", rows);
auto result = evaluate<SimpleVector<StringView>>(
"presto_replace('high', 'ig', 'f')", rows);
for (int i = 0; i < 10; ++i) {
EXPECT_EQ(result->valueAt(i), StringView("hfh"));
}
Expand All @@ -1351,7 +1351,8 @@ TEST_F(StringFunctionsTest, replaceWithReusableInputButNoInplace) {
[](vector_size_t) { return 2851588633; },
[](auto row) { return row >= 50; });
auto result = evaluateSimplified<FlatVector<StringView>>(
"substr(replace('bar', rtrim(c0)), c1, c2)", makeRowVector({c0, c1, c2}));
"substr(presto_replace('bar', rtrim(c0)), c1, c2)",
makeRowVector({c0, c1, c2}));
ASSERT_EQ(result->size(), 100);
for (int i = 0; i < 50; ++i) {
EXPECT_FALSE(result->isNullAt(i));
Expand Down

0 comments on commit 83f0ba8

Please sign in to comment.