You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2021/04/15 17:13:25 UTC
[GitHub] [arrow] rodrigojdebem opened a new pull request #10059: [ARROW-12410][C++][Gandiva] Implement regexp_replace function on Gandiva
rodrigojdebem opened a new pull request #10059:
URL: https://github.com/apache/arrow/pull/10059
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] anthonylouisbsb commented on a change in pull request #10059: ARROW-12410: [C++][Gandiva] Implement regexp_replace function on Gandiva
Posted by GitBox <gi...@apache.org>.
anthonylouisbsb commented on a change in pull request #10059:
URL: https://github.com/apache/arrow/pull/10059#discussion_r620635496
##########
File path: cpp/src/gandiva/gdv_function_stubs.cc
##########
@@ -42,6 +43,12 @@ bool gdv_fn_like_utf8_utf8(int64_t ptr, const char* data, int data_len,
return (*holder)(std::string(data, data_len));
}
+const char* gdv_fn_regexp_replace_utf8_utf8(int64_t ptr, const char* data, int data_len,
Review comment:
The stub function needs to accept 2 more additional parameters, the **length** of the **replace_string** and the length of the output string.
Remember to do the changes in the line the add the function inside the GlobalMapping functions
##########
File path: cpp/src/gandiva/gdv_function_stubs.cc
##########
@@ -31,6 +31,7 @@
#include "gandiva/precompiled/types.h"
#include "gandiva/random_generator_holder.h"
#include "gandiva/to_date_holder.h"
+#include "replace_holder.h"
Review comment:
Change to "gandiva/replace_holder.h"
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] projjal commented on a change in pull request #10059: ARROW-12410: [C++][Gandiva] Implement regexp_replace function on Gandiva
Posted by GitBox <gi...@apache.org>.
projjal commented on a change in pull request #10059:
URL: https://github.com/apache/arrow/pull/10059#discussion_r622732364
##########
File path: cpp/src/gandiva/precompiled/types.h
##########
@@ -263,6 +263,9 @@ const char* substr_utf8_int64_int64(gdv_int64 context, const char* input,
const char* substr_utf8_int64(gdv_int64 context, const char* input, gdv_int32 in_len,
gdv_int64 offset64, gdv_int32* out_len);
+const char* regexp_replace(const char* regexp, const char* str,
Review comment:
this is not required
##########
File path: java/gandiva/src/test/java/org/apache/arrow/gandiva/evaluator/ProjectorTest.java
##########
@@ -648,6 +648,66 @@ public void testRegex() throws GandivaException {
eval.close();
}
+ @Test
+ public void testRegexpReplace() throws GandivaException {
+
+ Field x = Field.nullable("x", new ArrowType.Utf8());
+ Field replaceString = Field.nullable("replaceString", new ArrowType.Utf8());
+
+ Field retType = Field.nullable("c", new ArrowType.Utf8());
+
+ TreeNode cond =
+ TreeBuilder.makeFunction(
+ "regexp_replace",
+ Lists.newArrayList(TreeBuilder.makeField(x), TreeBuilder.makeStringLiteral("ana"),
+ TreeBuilder.makeField(replaceString)),
+ new ArrowType.Utf8());
+ ExpressionTree expr = TreeBuilder.makeExpression(cond, retType);
+ Schema schema = new Schema(Lists.newArrayList(x, replaceString));
+ Projector eval = Projector.make(schema, Lists.newArrayList(expr));
+
+ int numRows = 5;
+ byte[] validity = new byte[]{(byte) 15, 0};
+ String[] valuesX = new String[]{"banana", "bananaana", "bananana", "anaana", "anaana"};
+ String[] valuesReplace = new String[]{"", "", "", "", ""};
Review comment:
atleast keep one non-empty replace string in the test
##########
File path: cpp/src/gandiva/gdv_function_stubs.cc
##########
@@ -42,6 +43,41 @@ bool gdv_fn_like_utf8_utf8(int64_t ptr, const char* data, int data_len,
return (*holder)(std::string(data, data_len));
}
+const char* gdv_fn_regexp_replace_utf8_utf8(
+ int64_t ptr, int64_t holder_ptr, const char* data, int32_t data_len,
+ const char* /*pattern*/, int32_t /*pattern_len*/, const char* replace_string,
+ int32_t replace_string_len, int32_t* out_length) {
+ std::string user_input(data, data_len);
+ std::string replace_input(replace_string, replace_string_len);
+ gandiva::ExecutionContext* context = reinterpret_cast<gandiva::ExecutionContext*>(ptr);
+
+ gandiva::ReplaceHolder* holder = reinterpret_cast<gandiva::ReplaceHolder*>(holder_ptr);
+ bool was_replaced = (*holder)(context, user_input, replace_input, out_length);
+
+ if (!was_replaced) {
+ return data;
+ }
+
+ // This condition treats the case where the whole string is replaced by an empty string
+ if (*out_length == 0) {
+ return "";
+ }
+
+ char* result_buffer =
+ reinterpret_cast<char*>(gdv_fn_context_arena_malloc(ptr, *out_length));
+
+ if (result_buffer == nullptr) {
+ std::string err_msg = "Could not allocate memory for string: " + user_input;
+ gdv_fn_context_set_error_msg(ptr, err_msg.data());
+ *out_length = 0;
+ return "";
+ }
+
+ memcpy(result_buffer, user_input.c_str(), *out_length);
Review comment:
Use user_input.data() instead of c_str() (Latter I believe does another allocation and copy)
##########
File path: cpp/src/gandiva/gdv_function_stubs.cc
##########
@@ -42,6 +43,41 @@ bool gdv_fn_like_utf8_utf8(int64_t ptr, const char* data, int data_len,
return (*holder)(std::string(data, data_len));
}
+const char* gdv_fn_regexp_replace_utf8_utf8(
+ int64_t ptr, int64_t holder_ptr, const char* data, int32_t data_len,
+ const char* /*pattern*/, int32_t /*pattern_len*/, const char* replace_string,
+ int32_t replace_string_len, int32_t* out_length) {
+ std::string user_input(data, data_len);
+ std::string replace_input(replace_string, replace_string_len);
+ gandiva::ExecutionContext* context = reinterpret_cast<gandiva::ExecutionContext*>(ptr);
+
+ gandiva::ReplaceHolder* holder = reinterpret_cast<gandiva::ReplaceHolder*>(holder_ptr);
+ bool was_replaced = (*holder)(context, user_input, replace_input, out_length);
+
+ if (!was_replaced) {
+ return data;
+ }
+
+ // This condition treats the case where the whole string is replaced by an empty string
+ if (*out_length == 0) {
+ return "";
+ }
+
+ char* result_buffer =
+ reinterpret_cast<char*>(gdv_fn_context_arena_malloc(ptr, *out_length));
+
+ if (result_buffer == nullptr) {
+ std::string err_msg = "Could not allocate memory for string: " + user_input;
Review comment:
This involves implicit allocation. We should not have another allocation in the error path of allocation failure. Just keep set error message to "Could not allocate memory for result"
##########
File path: cpp/src/gandiva/replace_holder_test.cc
##########
@@ -0,0 +1,115 @@
+// 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 "gandiva/replace_holder.h"
+#include "gandiva/regex_util.h"
+
+#include <memory>
+#include <vector>
+
+#include <gtest/gtest.h>
+
+namespace gandiva {
+
+class TestReplaceHolder : public ::testing::Test {
+ public:
+ FunctionNode BuildReplace(std::string pattern) {
Review comment:
This function seems unnecessary. It is also building "replace" function node which is a different function
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] projjal commented on a change in pull request #10059: ARROW-12410: [C++][Gandiva] Implement regexp_replace function on Gandiva
Posted by GitBox <gi...@apache.org>.
projjal commented on a change in pull request #10059:
URL: https://github.com/apache/arrow/pull/10059#discussion_r630833349
##########
File path: cpp/src/gandiva/replace_holder.cc
##########
@@ -0,0 +1,66 @@
+// 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 "gandiva/replace_holder.h"
+
+#include <regex>
Review comment:
this include doesn't seem required
##########
File path: cpp/src/gandiva/replace_holder.h
##########
@@ -0,0 +1,97 @@
+// 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 <re2/re2.h>
+
+#include <memory>
+#include <string>
+
+#include "arrow/status.h"
+#include "gandiva/execution_context.h"
+#include "gandiva/function_holder.h"
+#include "gandiva/node.h"
+#include "gandiva/visibility.h"
+
+namespace gandiva {
+
+/// Function Holder for 'replace'
+class GANDIVA_EXPORT ReplaceHolder : public FunctionHolder {
+ public:
+ ~ReplaceHolder() override = default;
+
+ static Status Make(const FunctionNode& node, std::shared_ptr<ReplaceHolder>* holder);
+
+ static Status Make(const std::string& sql_pattern,
+ std::shared_ptr<ReplaceHolder>* holder);
+
+ /// Return a new string with the pattern that matched the regex replaced for
+ /// the replace_input parameter.
+ const char* operator()(ExecutionContext* ctx, const char* user_input,
+ int32_t user_input_len, const char* replace_input,
+ int32_t replace_input_len, int32_t* out_length) {
+ std::string user_input_as_str(user_input, user_input_len);
+ std::string replace_input_as_str(replace_input, replace_input_len);
+
+ int32_t total_replaces =
+ RE2::GlobalReplace(&user_input_as_str, regex_, replace_input);
+
+ if (total_replaces < 0) {
+ return_error(ctx, user_input_as_str, replace_input_as_str);
+ *out_length = 0;
+ return "";
+ }
+
+ if (total_replaces == 0) {
+ *out_length = user_input_len;
+ return user_input;
+ }
+
+ *out_length = static_cast<int32_t>(user_input_as_str.size());
+
+ // This condition treats the case where the whole string is replaced by an empty
+ // string
+ if (*out_length == 0) {
+ return "";
+ }
+
+ char* result_buffer = reinterpret_cast<char*>(ctx->arena()->Allocate(*out_length));
+
+ if (result_buffer == NULLPTR) {
Review comment:
> nullptr
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] praveenbingo closed pull request #10059: ARROW-12410: [C++][Gandiva] Implement regexp_replace function on Gandiva
Posted by GitBox <gi...@apache.org>.
praveenbingo closed pull request #10059:
URL: https://github.com/apache/arrow/pull/10059
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #10059: ARROW-12410: [C++][Gandiva] Implement regexp_replace function on Gandiva
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #10059:
URL: https://github.com/apache/arrow/pull/10059#issuecomment-820621984
https://issues.apache.org/jira/browse/ARROW-12410
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] rodrigojdebem commented on a change in pull request #10059: ARROW-12410: [C++][Gandiva] Implement regexp_replace function on Gandiva
Posted by GitBox <gi...@apache.org>.
rodrigojdebem commented on a change in pull request #10059:
URL: https://github.com/apache/arrow/pull/10059#discussion_r631080733
##########
File path: cpp/src/gandiva/replace_holder.h
##########
@@ -0,0 +1,97 @@
+// 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 <re2/re2.h>
+
+#include <memory>
+#include <string>
+
+#include "arrow/status.h"
+#include "gandiva/execution_context.h"
+#include "gandiva/function_holder.h"
+#include "gandiva/node.h"
+#include "gandiva/visibility.h"
+
+namespace gandiva {
+
+/// Function Holder for 'replace'
+class GANDIVA_EXPORT ReplaceHolder : public FunctionHolder {
+ public:
+ ~ReplaceHolder() override = default;
+
+ static Status Make(const FunctionNode& node, std::shared_ptr<ReplaceHolder>* holder);
+
+ static Status Make(const std::string& sql_pattern,
+ std::shared_ptr<ReplaceHolder>* holder);
+
+ /// Return a new string with the pattern that matched the regex replaced for
+ /// the replace_input parameter.
+ const char* operator()(ExecutionContext* ctx, const char* user_input,
+ int32_t user_input_len, const char* replace_input,
+ int32_t replace_input_len, int32_t* out_length) {
+ std::string user_input_as_str(user_input, user_input_len);
+ std::string replace_input_as_str(replace_input, replace_input_len);
+
+ int32_t total_replaces =
+ RE2::GlobalReplace(&user_input_as_str, regex_, replace_input);
+
+ if (total_replaces < 0) {
+ return_error(ctx, user_input_as_str, replace_input_as_str);
+ *out_length = 0;
+ return "";
+ }
+
+ if (total_replaces == 0) {
+ *out_length = user_input_len;
+ return user_input;
+ }
+
+ *out_length = static_cast<int32_t>(user_input_as_str.size());
+
+ // This condition treats the case where the whole string is replaced by an empty
+ // string
+ if (*out_length == 0) {
+ return "";
+ }
+
+ char* result_buffer = reinterpret_cast<char*>(ctx->arena()->Allocate(*out_length));
+
+ if (result_buffer == NULLPTR) {
Review comment:
@projjal the cpp linter [runs a checker](https://github.com/apache/arrow/blob/02d6675cca525d1e5faee1286a704c7d4101ab15/cpp/build-support/lint_cpp_cli.py#L31) that raises an error if there is some **nullptr** construction inside a header file. I need to use the NULLPTR macro to works.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] projjal commented on a change in pull request #10059: ARROW-12410: [C++][Gandiva] Implement regexp_replace function on Gandiva
Posted by GitBox <gi...@apache.org>.
projjal commented on a change in pull request #10059:
URL: https://github.com/apache/arrow/pull/10059#discussion_r617624562
##########
File path: cpp/src/gandiva/replace_holder.cc
##########
@@ -0,0 +1,100 @@
+// 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 "gandiva/replace_holder.h"
+
+#include <regex>
+#include "gandiva/node.h"
+#include "gandiva/regex_util.h"
+
+namespace gandiva {
+
+RE2 LikeHolder::starts_with_regex_(R"((\w|\s)*\.\*)");
Review comment:
Looks like you just copy pasted like_function_holder code. This two functions do different things!
##########
File path: cpp/src/gandiva/precompiled/string_ops.cc
##########
@@ -131,6 +133,16 @@ bool is_substr_utf8_utf8(const char* data, int32_t data_len, const char* substr,
return false;
}
+FORCE_INLINE
+const char* regexp_replace(const char* regexp, const char* str, const char* replace_string) {
+ std::string s = str;
+ std::regex r (regexp);
Review comment:
I meant for to not use this function and use a function holder instead. Looks like you just added a holder and not using that and instead still calling this function
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] anthonylouisbsb commented on a change in pull request #10059: ARROW-12410: [C++][Gandiva] Implement regexp_replace function on Gandiva
Posted by GitBox <gi...@apache.org>.
anthonylouisbsb commented on a change in pull request #10059:
URL: https://github.com/apache/arrow/pull/10059#discussion_r627764037
##########
File path: cpp/src/gandiva/replace_holder.h
##########
@@ -0,0 +1,99 @@
+// 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 <memory>
+#include <string>
+
+#include <re2/re2.h>
+
+#include "arrow/status.h"
+
+#include "gandiva/execution_context.h"
+#include "gandiva/function_holder.h"
+#include "gandiva/node.h"
+#include "gandiva/visibility.h"
+
+namespace gandiva {
+
+/// Function Holder for 'replace'
+class GANDIVA_EXPORT ReplaceHolder : public FunctionHolder {
+ public:
+ ~ReplaceHolder() override = default;
+
+ static Status Make(const FunctionNode& node, std::shared_ptr<ReplaceHolder>* holder);
+
+ static Status Make(const std::string& sql_pattern,
+ std::shared_ptr<ReplaceHolder>* holder);
+
+ /// Return true if the data matches the pattern.
+ const char* operator()(ExecutionContext* ptr, std::string& user_input,
+ std::string& replace_input, int32_t* out_length) {
+ int32_t user_input_length = static_cast<int32_t>(user_input.size());
+ int32_t total_replaces = RE2::GlobalReplace(&user_input, regex_, replace_input);
+ bool was_replaced = false;
+
+ if (total_replaces < 0) {
+ return_error(ptr, user_input, replace_input);
+ *out_length = 0;
+ } else if (total_replaces == 0) {
+ *out_length = user_input_length;
+ } else {
+ was_replaced = true;
+ }
+
+ *out_length = static_cast<int32_t>(user_input.size());
+
+ if (!was_replaced) {
+ return user_input.data();
Review comment:
Problem solved, now, for this case is returning the user input char pointer
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] augustoasilva commented on a change in pull request #10059: ARROW-12410: [C++][Gandiva] Implement regexp_replace function on Gandiva
Posted by GitBox <gi...@apache.org>.
augustoasilva commented on a change in pull request #10059:
URL: https://github.com/apache/arrow/pull/10059#discussion_r635595413
##########
File path: cpp/src/gandiva/replace_holder.h
##########
@@ -0,0 +1,97 @@
+// 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 <re2/re2.h>
+
+#include <memory>
+#include <string>
+
+#include "arrow/status.h"
+#include "gandiva/execution_context.h"
+#include "gandiva/function_holder.h"
+#include "gandiva/node.h"
+#include "gandiva/visibility.h"
+
+namespace gandiva {
+
+/// Function Holder for 'replace'
+class GANDIVA_EXPORT ReplaceHolder : public FunctionHolder {
+ public:
+ ~ReplaceHolder() override = default;
+
+ static Status Make(const FunctionNode& node, std::shared_ptr<ReplaceHolder>* holder);
+
+ static Status Make(const std::string& sql_pattern,
+ std::shared_ptr<ReplaceHolder>* holder);
+
+ /// Return a new string with the pattern that matched the regex replaced for
+ /// the replace_input parameter.
+ const char* operator()(ExecutionContext* ctx, const char* user_input,
+ int32_t user_input_len, const char* replace_input,
+ int32_t replace_input_len, int32_t* out_length) {
+ std::string user_input_as_str(user_input, user_input_len);
+ std::string replace_input_as_str(replace_input, replace_input_len);
+
+ int32_t total_replaces =
+ RE2::GlobalReplace(&user_input_as_str, regex_, replace_input);
Review comment:
@projjal yeah, it should so I did changed the argument to replace_input_as_str
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] praveenbingo closed pull request #10059: ARROW-12410: [C++][Gandiva] Implement regexp_replace function on Gandiva
Posted by GitBox <gi...@apache.org>.
praveenbingo closed pull request #10059:
URL: https://github.com/apache/arrow/pull/10059
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] projjal commented on a change in pull request #10059: ARROW-12410: [C++][Gandiva] Implement regexp_replace function on Gandiva
Posted by GitBox <gi...@apache.org>.
projjal commented on a change in pull request #10059:
URL: https://github.com/apache/arrow/pull/10059#discussion_r631561351
##########
File path: cpp/src/gandiva/replace_holder.h
##########
@@ -0,0 +1,97 @@
+// 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 <re2/re2.h>
+
+#include <memory>
+#include <string>
+
+#include "arrow/status.h"
+#include "gandiva/execution_context.h"
+#include "gandiva/function_holder.h"
+#include "gandiva/node.h"
+#include "gandiva/visibility.h"
+
+namespace gandiva {
+
+/// Function Holder for 'replace'
+class GANDIVA_EXPORT ReplaceHolder : public FunctionHolder {
+ public:
+ ~ReplaceHolder() override = default;
+
+ static Status Make(const FunctionNode& node, std::shared_ptr<ReplaceHolder>* holder);
+
+ static Status Make(const std::string& sql_pattern,
+ std::shared_ptr<ReplaceHolder>* holder);
+
+ /// Return a new string with the pattern that matched the regex replaced for
+ /// the replace_input parameter.
+ const char* operator()(ExecutionContext* ctx, const char* user_input,
+ int32_t user_input_len, const char* replace_input,
+ int32_t replace_input_len, int32_t* out_length) {
+ std::string user_input_as_str(user_input, user_input_len);
+ std::string replace_input_as_str(replace_input, replace_input_len);
+
+ int32_t total_replaces =
+ RE2::GlobalReplace(&user_input_as_str, regex_, replace_input);
Review comment:
shouldn't the third argument be replace_input_as_str
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] rodrigojdebem commented on a change in pull request #10059: ARROW-12410: [C++][Gandiva] Implement regexp_replace function on Gandiva
Posted by GitBox <gi...@apache.org>.
rodrigojdebem commented on a change in pull request #10059:
URL: https://github.com/apache/arrow/pull/10059#discussion_r621657220
##########
File path: cpp/src/gandiva/replace_holder.cc
##########
@@ -0,0 +1,100 @@
+// 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 "gandiva/replace_holder.h"
+
+#include <regex>
+#include "gandiva/node.h"
+#include "gandiva/regex_util.h"
+
+namespace gandiva {
+
+RE2 LikeHolder::starts_with_regex_(R"((\w|\s)*\.\*)");
Review comment:
Fixed it on the newer commits. Sorry about that, I used it as a base and forgot to remove the part I wouldn't use.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] projjal commented on a change in pull request #10059: ARROW-12410: [C++][Gandiva] Implement regexp_replace function on Gandiva
Posted by GitBox <gi...@apache.org>.
projjal commented on a change in pull request #10059:
URL: https://github.com/apache/arrow/pull/10059#discussion_r631558917
##########
File path: cpp/src/gandiva/replace_holder.h
##########
@@ -0,0 +1,97 @@
+// 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 <re2/re2.h>
+
+#include <memory>
+#include <string>
+
+#include "arrow/status.h"
+#include "gandiva/execution_context.h"
+#include "gandiva/function_holder.h"
+#include "gandiva/node.h"
+#include "gandiva/visibility.h"
+
+namespace gandiva {
+
+/// Function Holder for 'replace'
+class GANDIVA_EXPORT ReplaceHolder : public FunctionHolder {
+ public:
+ ~ReplaceHolder() override = default;
+
+ static Status Make(const FunctionNode& node, std::shared_ptr<ReplaceHolder>* holder);
+
+ static Status Make(const std::string& sql_pattern,
+ std::shared_ptr<ReplaceHolder>* holder);
+
+ /// Return a new string with the pattern that matched the regex replaced for
+ /// the replace_input parameter.
+ const char* operator()(ExecutionContext* ctx, const char* user_input,
+ int32_t user_input_len, const char* replace_input,
+ int32_t replace_input_len, int32_t* out_length) {
+ std::string user_input_as_str(user_input, user_input_len);
+ std::string replace_input_as_str(replace_input, replace_input_len);
+
+ int32_t total_replaces =
+ RE2::GlobalReplace(&user_input_as_str, regex_, replace_input);
+
+ if (total_replaces < 0) {
+ return_error(ctx, user_input_as_str, replace_input_as_str);
+ *out_length = 0;
+ return "";
+ }
+
+ if (total_replaces == 0) {
+ *out_length = user_input_len;
+ return user_input;
+ }
+
+ *out_length = static_cast<int32_t>(user_input_as_str.size());
+
+ // This condition treats the case where the whole string is replaced by an empty
+ // string
+ if (*out_length == 0) {
+ return "";
+ }
+
+ char* result_buffer = reinterpret_cast<char*>(ctx->arena()->Allocate(*out_length));
+
+ if (result_buffer == NULLPTR) {
Review comment:
ok
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] rodrigojdebem commented on a change in pull request #10059: ARROW-12410: [C++][Gandiva] Implement regexp_replace function on Gandiva
Posted by GitBox <gi...@apache.org>.
rodrigojdebem commented on a change in pull request #10059:
URL: https://github.com/apache/arrow/pull/10059#discussion_r631094117
##########
File path: cpp/src/gandiva/replace_holder.cc
##########
@@ -0,0 +1,66 @@
+// 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 "gandiva/replace_holder.h"
+
+#include <regex>
Review comment:
Fixed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] projjal commented on a change in pull request #10059: ARROW-12410: [C++][Gandiva] Implement regexp_replace function on Gandiva
Posted by GitBox <gi...@apache.org>.
projjal commented on a change in pull request #10059:
URL: https://github.com/apache/arrow/pull/10059#discussion_r624647491
##########
File path: cpp/src/gandiva/replace_holder.h
##########
@@ -0,0 +1,99 @@
+// 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 <memory>
+#include <string>
+
+#include <re2/re2.h>
+
+#include "arrow/status.h"
+
+#include "gandiva/execution_context.h"
+#include "gandiva/function_holder.h"
+#include "gandiva/node.h"
+#include "gandiva/visibility.h"
+
+namespace gandiva {
+
+/// Function Holder for 'replace'
+class GANDIVA_EXPORT ReplaceHolder : public FunctionHolder {
+ public:
+ ~ReplaceHolder() override = default;
+
+ static Status Make(const FunctionNode& node, std::shared_ptr<ReplaceHolder>* holder);
+
+ static Status Make(const std::string& sql_pattern,
+ std::shared_ptr<ReplaceHolder>* holder);
+
+ /// Return true if the data matches the pattern.
+ const char* operator()(ExecutionContext* ptr, std::string& user_input,
+ std::string& replace_input, int32_t* out_length) {
+ int32_t user_input_length = static_cast<int32_t>(user_input.size());
+ int32_t total_replaces = RE2::GlobalReplace(&user_input, regex_, replace_input);
+ bool was_replaced = false;
+
+ if (total_replaces < 0) {
+ return_error(ptr, user_input, replace_input);
+ *out_length = 0;
+ } else if (total_replaces == 0) {
+ *out_length = user_input_length;
+ } else {
+ was_replaced = true;
+ }
+
+ *out_length = static_cast<int32_t>(user_input.size());
+
+ if (!was_replaced) {
+ return user_input.data();
+ }
+
+ // This condition treats the case where the whole string is replaced by an empty
+ // string
+ if (*out_length == 0) {
+ return "";
+ }
+ auto context = reinterpret_cast<gandiva::ExecutionContext*>(ptr);
Review comment:
This cast doesn't seem required. Just rename the argument ptr to context
##########
File path: cpp/src/gandiva/replace_holder.h
##########
@@ -0,0 +1,99 @@
+// 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 <memory>
+#include <string>
+
+#include <re2/re2.h>
+
+#include "arrow/status.h"
+
+#include "gandiva/execution_context.h"
+#include "gandiva/function_holder.h"
+#include "gandiva/node.h"
+#include "gandiva/visibility.h"
+
+namespace gandiva {
+
+/// Function Holder for 'replace'
+class GANDIVA_EXPORT ReplaceHolder : public FunctionHolder {
+ public:
+ ~ReplaceHolder() override = default;
+
+ static Status Make(const FunctionNode& node, std::shared_ptr<ReplaceHolder>* holder);
+
+ static Status Make(const std::string& sql_pattern,
+ std::shared_ptr<ReplaceHolder>* holder);
+
+ /// Return true if the data matches the pattern.
+ const char* operator()(ExecutionContext* ptr, std::string& user_input,
+ std::string& replace_input, int32_t* out_length) {
+ int32_t user_input_length = static_cast<int32_t>(user_input.size());
+ int32_t total_replaces = RE2::GlobalReplace(&user_input, regex_, replace_input);
+ bool was_replaced = false;
+
+ if (total_replaces < 0) {
+ return_error(ptr, user_input, replace_input);
+ *out_length = 0;
+ } else if (total_replaces == 0) {
+ *out_length = user_input_length;
+ } else {
+ was_replaced = true;
+ }
+
+ *out_length = static_cast<int32_t>(user_input.size());
Review comment:
Its probably better to move this statement inside the else statement above
##########
File path: cpp/src/gandiva/replace_holder.h
##########
@@ -0,0 +1,99 @@
+// 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 <memory>
+#include <string>
+
+#include <re2/re2.h>
+
+#include "arrow/status.h"
+
+#include "gandiva/execution_context.h"
+#include "gandiva/function_holder.h"
+#include "gandiva/node.h"
+#include "gandiva/visibility.h"
+
+namespace gandiva {
+
+/// Function Holder for 'replace'
+class GANDIVA_EXPORT ReplaceHolder : public FunctionHolder {
+ public:
+ ~ReplaceHolder() override = default;
+
+ static Status Make(const FunctionNode& node, std::shared_ptr<ReplaceHolder>* holder);
+
+ static Status Make(const std::string& sql_pattern,
+ std::shared_ptr<ReplaceHolder>* holder);
+
+ /// Return true if the data matches the pattern.
+ const char* operator()(ExecutionContext* ptr, std::string& user_input,
+ std::string& replace_input, int32_t* out_length) {
+ int32_t user_input_length = static_cast<int32_t>(user_input.size());
+ int32_t total_replaces = RE2::GlobalReplace(&user_input, regex_, replace_input);
+ bool was_replaced = false;
+
+ if (total_replaces < 0) {
+ return_error(ptr, user_input, replace_input);
+ *out_length = 0;
+ } else if (total_replaces == 0) {
+ *out_length = user_input_length;
+ } else {
+ was_replaced = true;
+ }
+
+ *out_length = static_cast<int32_t>(user_input.size());
+
+ if (!was_replaced) {
+ return user_input.data();
+ }
+
+ // This condition treats the case where the whole string is replaced by an empty
+ // string
+ if (*out_length == 0) {
+ return "";
+ }
+ auto context = reinterpret_cast<gandiva::ExecutionContext*>(ptr);
+
+ char* result_buffer =
+ reinterpret_cast<char*>(context->arena()->Allocate(*out_length));
+
+ if (result_buffer == nullptr) {
+ context = reinterpret_cast<gandiva::ExecutionContext*>(ptr);
Review comment:
cast not required
##########
File path: cpp/src/gandiva/replace_holder.h
##########
@@ -0,0 +1,99 @@
+// 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 <memory>
+#include <string>
+
+#include <re2/re2.h>
+
+#include "arrow/status.h"
+
+#include "gandiva/execution_context.h"
+#include "gandiva/function_holder.h"
+#include "gandiva/node.h"
+#include "gandiva/visibility.h"
+
+namespace gandiva {
+
+/// Function Holder for 'replace'
+class GANDIVA_EXPORT ReplaceHolder : public FunctionHolder {
+ public:
+ ~ReplaceHolder() override = default;
+
+ static Status Make(const FunctionNode& node, std::shared_ptr<ReplaceHolder>* holder);
+
+ static Status Make(const std::string& sql_pattern,
+ std::shared_ptr<ReplaceHolder>* holder);
+
+ /// Return true if the data matches the pattern.
+ const char* operator()(ExecutionContext* ptr, std::string& user_input,
+ std::string& replace_input, int32_t* out_length) {
+ int32_t user_input_length = static_cast<int32_t>(user_input.size());
+ int32_t total_replaces = RE2::GlobalReplace(&user_input, regex_, replace_input);
+ bool was_replaced = false;
+
+ if (total_replaces < 0) {
+ return_error(ptr, user_input, replace_input);
+ *out_length = 0;
Review comment:
return "" here
##########
File path: cpp/src/gandiva/replace_holder.h
##########
@@ -0,0 +1,99 @@
+// 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 <memory>
+#include <string>
+
+#include <re2/re2.h>
+
+#include "arrow/status.h"
+
+#include "gandiva/execution_context.h"
+#include "gandiva/function_holder.h"
+#include "gandiva/node.h"
+#include "gandiva/visibility.h"
+
+namespace gandiva {
+
+/// Function Holder for 'replace'
+class GANDIVA_EXPORT ReplaceHolder : public FunctionHolder {
+ public:
+ ~ReplaceHolder() override = default;
+
+ static Status Make(const FunctionNode& node, std::shared_ptr<ReplaceHolder>* holder);
+
+ static Status Make(const std::string& sql_pattern,
+ std::shared_ptr<ReplaceHolder>* holder);
+
+ /// Return true if the data matches the pattern.
+ const char* operator()(ExecutionContext* ptr, std::string& user_input,
+ std::string& replace_input, int32_t* out_length) {
+ int32_t user_input_length = static_cast<int32_t>(user_input.size());
+ int32_t total_replaces = RE2::GlobalReplace(&user_input, regex_, replace_input);
+ bool was_replaced = false;
+
+ if (total_replaces < 0) {
+ return_error(ptr, user_input, replace_input);
+ *out_length = 0;
+ } else if (total_replaces == 0) {
+ *out_length = user_input_length;
+ } else {
+ was_replaced = true;
+ }
+
+ *out_length = static_cast<int32_t>(user_input.size());
+
+ if (!was_replaced) {
+ return user_input.data();
Review comment:
You can't return user_input.data() since it wil get deallocated when returning from the parent function. Its better just pass the user_input char* and len as parameters to the method, instead of passing std::string and memcpying for no replace
##########
File path: cpp/src/gandiva/replace_holder.h
##########
@@ -0,0 +1,99 @@
+// 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 <memory>
+#include <string>
+
+#include <re2/re2.h>
+
+#include "arrow/status.h"
+
+#include "gandiva/execution_context.h"
+#include "gandiva/function_holder.h"
+#include "gandiva/node.h"
+#include "gandiva/visibility.h"
+
+namespace gandiva {
+
+/// Function Holder for 'replace'
+class GANDIVA_EXPORT ReplaceHolder : public FunctionHolder {
+ public:
+ ~ReplaceHolder() override = default;
+
+ static Status Make(const FunctionNode& node, std::shared_ptr<ReplaceHolder>* holder);
+
+ static Status Make(const std::string& sql_pattern,
+ std::shared_ptr<ReplaceHolder>* holder);
+
+ /// Return true if the data matches the pattern.
Review comment:
Update the comment
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] anthonylouisbsb commented on pull request #10059: ARROW-12410: [C++][Gandiva] Implement regexp_replace function on Gandiva
Posted by GitBox <gi...@apache.org>.
anthonylouisbsb commented on pull request #10059:
URL: https://github.com/apache/arrow/pull/10059#issuecomment-855891824
@praveenbingo The rebase was applied in the Pull Request.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] projjal commented on a change in pull request #10059: ARROW-12410: [C++][Gandiva] Implement regexp_replace function on Gandiva
Posted by GitBox <gi...@apache.org>.
projjal commented on a change in pull request #10059:
URL: https://github.com/apache/arrow/pull/10059#discussion_r623623465
##########
File path: cpp/src/gandiva/gdv_function_stubs.cc
##########
@@ -42,6 +43,41 @@ bool gdv_fn_like_utf8_utf8(int64_t ptr, const char* data, int data_len,
return (*holder)(std::string(data, data_len));
}
+const char* gdv_fn_regexp_replace_utf8_utf8(
+ int64_t ptr, int64_t holder_ptr, const char* data, int32_t data_len,
+ const char* /*pattern*/, int32_t /*pattern_len*/, const char* replace_string,
+ int32_t replace_string_len, int32_t* out_length) {
+ std::string user_input(data, data_len);
+ std::string replace_input(replace_string, replace_string_len);
+ gandiva::ExecutionContext* context = reinterpret_cast<gandiva::ExecutionContext*>(ptr);
+
+ gandiva::ReplaceHolder* holder = reinterpret_cast<gandiva::ReplaceHolder*>(holder_ptr);
+ bool was_replaced = (*holder)(context, user_input, replace_input, out_length);
+
+ if (!was_replaced) {
+ return data;
+ }
+
+ // This condition treats the case where the whole string is replaced by an empty string
+ if (*out_length == 0) {
+ return "";
+ }
+
+ char* result_buffer =
+ reinterpret_cast<char*>(gdv_fn_context_arena_malloc(ptr, *out_length));
+
+ if (result_buffer == nullptr) {
+ std::string err_msg = "Could not allocate memory for result";
+ gdv_fn_context_set_error_msg(ptr, err_msg.data());
Review comment:
The above still involves an allocation.
change this to gdv_fn_context_set_error_msg(ptr, "Could not allocate memory for result");
##########
File path: java/gandiva/src/test/java/org/apache/arrow/gandiva/evaluator/ProjectorTest.java
##########
@@ -648,6 +648,66 @@ public void testRegex() throws GandivaException {
eval.close();
}
+ @Test
+ public void testRegexpReplace() throws GandivaException {
+
+ Field x = Field.nullable("x", new ArrowType.Utf8());
+ Field replaceString = Field.nullable("replaceString", new ArrowType.Utf8());
+
+ Field retType = Field.nullable("c", new ArrowType.Utf8());
+
+ TreeNode cond =
+ TreeBuilder.makeFunction(
+ "regexp_replace",
+ Lists.newArrayList(TreeBuilder.makeField(x), TreeBuilder.makeStringLiteral("ana"),
+ TreeBuilder.makeField(replaceString)),
+ new ArrowType.Utf8());
+ ExpressionTree expr = TreeBuilder.makeExpression(cond, retType);
+ Schema schema = new Schema(Lists.newArrayList(x, replaceString));
+ Projector eval = Projector.make(schema, Lists.newArrayList(expr));
+
+ int numRows = 5;
+ byte[] validity = new byte[]{(byte) 15, 0};
+ String[] valuesX = new String[]{"banana", "bananaana", "bananana", "anaana", "anaana"};
+ String[] valuesReplace = new String[]{"", "", "", "c", ""};
Review comment:
add some multi char replace string too
##########
File path: cpp/src/gandiva/gdv_function_stubs.cc
##########
@@ -42,6 +43,41 @@ bool gdv_fn_like_utf8_utf8(int64_t ptr, const char* data, int data_len,
return (*holder)(std::string(data, data_len));
}
+const char* gdv_fn_regexp_replace_utf8_utf8(
+ int64_t ptr, int64_t holder_ptr, const char* data, int32_t data_len,
+ const char* /*pattern*/, int32_t /*pattern_len*/, const char* replace_string,
+ int32_t replace_string_len, int32_t* out_length) {
+ std::string user_input(data, data_len);
+ std::string replace_input(replace_string, replace_string_len);
+ gandiva::ExecutionContext* context = reinterpret_cast<gandiva::ExecutionContext*>(ptr);
+
+ gandiva::ReplaceHolder* holder = reinterpret_cast<gandiva::ReplaceHolder*>(holder_ptr);
+ bool was_replaced = (*holder)(context, user_input, replace_input, out_length);
+
+ if (!was_replaced) {
Review comment:
Why not incorporate the following logic inside the holder operator() method too?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] projjal commented on a change in pull request #10059: ARROW-12410: [C++][Gandiva] Implement regexp_replace function on Gandiva
Posted by GitBox <gi...@apache.org>.
projjal commented on a change in pull request #10059:
URL: https://github.com/apache/arrow/pull/10059#discussion_r614659964
##########
File path: cpp/src/gandiva/precompiled/string_ops.cc
##########
@@ -131,6 +133,16 @@ bool is_substr_utf8_utf8(const char* data, int32_t data_len, const char* substr,
return false;
}
+FORCE_INLINE
+const char* regexp_replace(const char* regexp, const char* str, const char* replace_string) {
+ std::string s = str;
+ std::regex r (regexp);
Review comment:
Please use RE2 instead of std::regex. Also use a holder to prevent compiling the pattern on each call. See like_holder.h
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] anthonylouisbsb commented on pull request #10059: ARROW-12410: [C++][Gandiva] Implement regexp_replace function on Gandiva
Posted by GitBox <gi...@apache.org>.
anthonylouisbsb commented on pull request #10059:
URL: https://github.com/apache/arrow/pull/10059#issuecomment-879081040
@projjal @praveenbingo I applied the rebase
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] praveenbingo commented on pull request #10059: ARROW-12410: [C++][Gandiva] Implement regexp_replace function on Gandiva
Posted by GitBox <gi...@apache.org>.
praveenbingo commented on pull request #10059:
URL: https://github.com/apache/arrow/pull/10059#issuecomment-855809973
@projjal needs a rebase
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] rodrigojdebem commented on a change in pull request #10059: ARROW-12410: [C++][Gandiva] Implement regexp_replace function on Gandiva
Posted by GitBox <gi...@apache.org>.
rodrigojdebem commented on a change in pull request #10059:
URL: https://github.com/apache/arrow/pull/10059#discussion_r631080733
##########
File path: cpp/src/gandiva/replace_holder.h
##########
@@ -0,0 +1,97 @@
+// 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 <re2/re2.h>
+
+#include <memory>
+#include <string>
+
+#include "arrow/status.h"
+#include "gandiva/execution_context.h"
+#include "gandiva/function_holder.h"
+#include "gandiva/node.h"
+#include "gandiva/visibility.h"
+
+namespace gandiva {
+
+/// Function Holder for 'replace'
+class GANDIVA_EXPORT ReplaceHolder : public FunctionHolder {
+ public:
+ ~ReplaceHolder() override = default;
+
+ static Status Make(const FunctionNode& node, std::shared_ptr<ReplaceHolder>* holder);
+
+ static Status Make(const std::string& sql_pattern,
+ std::shared_ptr<ReplaceHolder>* holder);
+
+ /// Return a new string with the pattern that matched the regex replaced for
+ /// the replace_input parameter.
+ const char* operator()(ExecutionContext* ctx, const char* user_input,
+ int32_t user_input_len, const char* replace_input,
+ int32_t replace_input_len, int32_t* out_length) {
+ std::string user_input_as_str(user_input, user_input_len);
+ std::string replace_input_as_str(replace_input, replace_input_len);
+
+ int32_t total_replaces =
+ RE2::GlobalReplace(&user_input_as_str, regex_, replace_input);
+
+ if (total_replaces < 0) {
+ return_error(ctx, user_input_as_str, replace_input_as_str);
+ *out_length = 0;
+ return "";
+ }
+
+ if (total_replaces == 0) {
+ *out_length = user_input_len;
+ return user_input;
+ }
+
+ *out_length = static_cast<int32_t>(user_input_as_str.size());
+
+ // This condition treats the case where the whole string is replaced by an empty
+ // string
+ if (*out_length == 0) {
+ return "";
+ }
+
+ char* result_buffer = reinterpret_cast<char*>(ctx->arena()->Allocate(*out_length));
+
+ if (result_buffer == NULLPTR) {
Review comment:
@projjal the cpp linter [runs a checker (https://github.com/apache/arrow/blob/02d6675cca525d1e5faee1286a704c7d4101ab15/cpp/build-support/lint_cpp_cli.py#L31) that raises an error if there is some **nullptr** construction inside a header file. I need to use the NULLPTR macro to works.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #10059: [ARROW-12410][C++][Gandiva] Implement regexp_replace function on Gandiva
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #10059:
URL: https://github.com/apache/arrow/pull/10059#issuecomment-820610029
<!--
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.
-->
Thanks for opening a pull request!
If this is not a [minor PR](https://github.com/apache/arrow/blob/master/CONTRIBUTING.md#Minor-Fixes). Could you open an issue for this pull request on JIRA? https://issues.apache.org/jira/browse/ARROW
Opening JIRAs ahead of time contributes to the [Openness](http://theapacheway.com/open/#:~:text=Openness%20allows%20new%20users%20the,must%20happen%20in%20the%20open.) of the Apache Arrow project.
Then could you also rename pull request title in the following format?
ARROW-${JIRA_ID}: [${COMPONENT}] ${SUMMARY}
or
MINOR: [${COMPONENT}] ${SUMMARY}
See also:
* [Other pull requests](https://github.com/apache/arrow/pulls/)
* [Contribution Guidelines - How to contribute patches](https://arrow.apache.org/docs/developers/contributing.html#how-to-contribute-patches)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org