Adapt convert_from method to support single char on replacement (defined with dremio team)

jpedroantunes · jpedroantunes · commit a2ea61beebda · 2021-04-12T08:07:41.000-03:00
diff --git a/cpp/src/gandiva/function_registry_string.cc b/cpp/src/gandiva/function_registry_string.cc
@@ -204,7 +204,7 @@ std::vector<NativeFunction> GetStringFunctionRegistry() {
                      utf8(), kResultNullIfNull, "convert_fromUTF8_binary",
                      NativeFunction::kNeedsContext),
 
-      NativeFunction("convert_fromUTF8", {"convert_fromutf8"},
+      NativeFunction("convert_replaceUTF8", {"convert_replaceutf8"},
                      DataTypeVector{binary(), utf8()}, utf8(), kResultNullIfNull,
                      "convert_replace_invalid_fromUTF8_binary",
                      NativeFunction::kNeedsContext),
diff --git a/cpp/src/gandiva/precompiled/string_ops.cc b/cpp/src/gandiva/precompiled/string_ops.cc
@@ -1249,8 +1249,10 @@ const char* convert_fromUTF8_binary(gdv_int64 context, const char* bin_in, gdv_i
 FORCE_INLINE
 const char* convert_replace_invalid_fromUTF8_binary(
     gdv_int64 context, const char* text_in, gdv_int32 text_len,
-    const char* char_to_replace, gdv_int32 char_to_replace_len, gdv_int32* out_len) {
-  *out_len = text_len * char_to_replace_len;
+    const char* char_to_replace, gdv_int32 /*char_to_replace_len*/, gdv_int32* out_len) {
+  // actually the convert_replace function replaces the invalid bytes with a single byte
+  // so the output length will be the same as the input length
+  *out_len = text_len;
   char* ret = reinterpret_cast<char*>(gdv_fn_context_arena_malloc(context, *out_len));
   if (ret == nullptr) {
     gdv_fn_context_set_error_msg(context, "Could not allocate memory for output string");
@@ -1264,8 +1266,8 @@ const char* convert_replace_invalid_fromUTF8_binary(
   for (int text_index = 0; text_index < text_len; text_index += char_len) {
     char_len = utf8_char_length(text_in[text_index]);
     if (char_len == 0 || text_index + char_len > text_len) {
-      memcpy(ret + out_byte_counter, char_to_replace, char_to_replace_len);
-      out_byte_counter += char_to_replace_len;
+      memcpy(ret + out_byte_counter, char_to_replace, 1);
+      out_byte_counter += 1;
       // define char_len = 1 to increase text_index by 1 (as ASCII char fits in 1 byte)
       char_len = 1;
     } else {
diff --git a/cpp/src/gandiva/precompiled/string_ops_test.cc b/cpp/src/gandiva/precompiled/string_ops_test.cc
@@ -129,6 +129,21 @@ TEST(TestStringOps, TestConvertReplaceInvalidUtf8Char) {
   EXPECT_EQ(std::string(a_str, a_in_out_len), "ok-a(-a");
   EXPECT_FALSE(ctx.has_error());
 
+  // invalid utf8 (xa0 and xa1 are invalid)
+  std::string b("ok-\xa0\xa1-valid");
+  auto b_in_out_len = static_cast<int>(b.length());
+  const char* b_str = convert_replace_invalid_fromUTF8_binary(
+      ctx_ptr, b.data(), b_in_out_len, "b", 1, &b_in_out_len);
+  EXPECT_EQ(std::string(b_str, b_in_out_len), "ok-bb-valid");
+  EXPECT_FALSE(ctx.has_error());
+
+  // full valid utf8
+  std::string c("all-valid");
+  auto c_in_out_len = static_cast<int>(c.length());
+  const char* c_str = convert_replace_invalid_fromUTF8_binary(
+      ctx_ptr, c.data(), c_in_out_len, "c", 1, &c_in_out_len);
+  EXPECT_EQ(std::string(c_str, c_in_out_len), "all-valid");
+
   // valid utf8 (महसुस is a 4-byte utf-8 char)
   std::string e("ok-महसुस-valid-new");
   auto e_in_out_len = static_cast<int>(e.length());
diff --git a/cpp/src/gandiva/tests/utf8_test.cc b/cpp/src/gandiva/tests/utf8_test.cc
@@ -556,7 +556,7 @@ TEST_F(TestUtf8, TestConvertUtf8) {
   auto node_b = TreeExprBuilder::MakeStringLiteral("z");
 
   auto convert_replace_utf8 =
-      TreeExprBuilder::MakeFunction("convert_fromUTF8", {node_a, node_b}, utf8());
+      TreeExprBuilder::MakeFunction("convert_replaceUTF8", {node_a, node_b}, utf8());
   auto equals =
       TreeExprBuilder::MakeFunction("equal", {convert_replace_utf8, node_c}, boolean());
   auto expr = TreeExprBuilder::MakeExpression(equals, res);
