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/14 13:17:02 UTC
[GitHub] [arrow] jpedroantunes opened a new pull request #10023: ARROW-12378: [C++][Gandiva] Implement castVARBINARY(varchar, len) and castVARBINARY(varbinary, len)
jpedroantunes opened a new pull request #10023:
URL: https://github.com/apache/arrow/pull/10023
Implement the following functions:
- castVARBINARY(varchar, len)
- castVARBINARY(varbinary, len)
--
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 #10023: ARROW-12378: [C++][Gandiva] Implement castVARBINARY functions
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #10023:
URL: https://github.com/apache/arrow/pull/10023#issuecomment-819664023
https://issues.apache.org/jira/browse/ARROW-12378
--
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] jpedroantunes commented on a change in pull request #10023: ARROW-12378: [C++][Gandiva] Implement castVARBINARY functions
Posted by GitBox <gi...@apache.org>.
jpedroantunes commented on a change in pull request #10023:
URL: https://github.com/apache/arrow/pull/10023#discussion_r630961867
##########
File path: cpp/src/gandiva/gdv_function_stubs.cc
##########
@@ -307,85 +307,114 @@ CAST_NUMERIC_FROM_STRING(double, arrow::DoubleType, FLOAT8)
#undef CAST_NUMERIC_FROM_STRING
-#define GDV_FN_CAST_VARCHAR_INTEGER(IN_TYPE, ARROW_TYPE) \
- GANDIVA_EXPORT \
- const char* gdv_fn_castVARCHAR_##IN_TYPE##_int64(int64_t context, gdv_##IN_TYPE value, \
- int64_t len, int32_t * out_len) { \
- if (len < 0) { \
- gdv_fn_context_set_error_msg(context, "Buffer length can not be negative"); \
- *out_len = 0; \
- return ""; \
- } \
- if (len == 0) { \
- *out_len = 0; \
- return ""; \
- } \
- arrow::internal::StringFormatter<arrow::ARROW_TYPE> formatter; \
- char* ret = reinterpret_cast<char*>( \
- gdv_fn_context_arena_malloc(context, static_cast<int32_t>(len))); \
- if (ret == nullptr) { \
- gdv_fn_context_set_error_msg(context, "Could not allocate memory"); \
- *out_len = 0; \
- return ""; \
- } \
- arrow::Status status = formatter(value, [&](arrow::util::string_view v) { \
- int64_t size = static_cast<int64_t>(v.size()); \
- *out_len = static_cast<int32_t>(len < size ? len : size); \
- memcpy(ret, v.data(), *out_len); \
- return arrow::Status::OK(); \
- }); \
- if (!status.ok()) { \
- std::string err = "Could not cast " + std::to_string(value) + " to string"; \
- gdv_fn_context_set_error_msg(context, err.c_str()); \
- *out_len = 0; \
- return ""; \
- } \
- return ret; \
+// Add functions for castVARBINARY for utf8 and binary
+#define CAST_VARBINARY_FROM_STRING_AND_BINARY(TYPE_NAME) \
Review comment:
Solved
##########
File path: cpp/src/gandiva/gdv_function_stubs.cc
##########
@@ -307,85 +307,114 @@ CAST_NUMERIC_FROM_STRING(double, arrow::DoubleType, FLOAT8)
#undef CAST_NUMERIC_FROM_STRING
-#define GDV_FN_CAST_VARCHAR_INTEGER(IN_TYPE, ARROW_TYPE) \
- GANDIVA_EXPORT \
- const char* gdv_fn_castVARCHAR_##IN_TYPE##_int64(int64_t context, gdv_##IN_TYPE value, \
- int64_t len, int32_t * out_len) { \
- if (len < 0) { \
- gdv_fn_context_set_error_msg(context, "Buffer length can not be negative"); \
- *out_len = 0; \
- return ""; \
- } \
- if (len == 0) { \
- *out_len = 0; \
- return ""; \
- } \
- arrow::internal::StringFormatter<arrow::ARROW_TYPE> formatter; \
- char* ret = reinterpret_cast<char*>( \
- gdv_fn_context_arena_malloc(context, static_cast<int32_t>(len))); \
- if (ret == nullptr) { \
- gdv_fn_context_set_error_msg(context, "Could not allocate memory"); \
- *out_len = 0; \
- return ""; \
- } \
- arrow::Status status = formatter(value, [&](arrow::util::string_view v) { \
- int64_t size = static_cast<int64_t>(v.size()); \
- *out_len = static_cast<int32_t>(len < size ? len : size); \
- memcpy(ret, v.data(), *out_len); \
- return arrow::Status::OK(); \
- }); \
- if (!status.ok()) { \
- std::string err = "Could not cast " + std::to_string(value) + " to string"; \
- gdv_fn_context_set_error_msg(context, err.c_str()); \
- *out_len = 0; \
- return ""; \
- } \
- return ret; \
+// Add functions for castVARBINARY for utf8 and binary
+#define CAST_VARBINARY_FROM_STRING_AND_BINARY(TYPE_NAME) \
+ GANDIVA_EXPORT \
+ const char* gdv_fn_castVARBINARY_##TYPE_NAME(int64_t context, const char* data, \
+ int32_t data_len, int64_t out_len, \
+ int64_t* out_length) { \
+ if (out_len < 0) { \
+ gdv_fn_context_set_error_msg(context, "Output buffer length can't be negative"); \
+ *out_length = 0; \
+ return ""; \
+ } \
+ \
+ if (out_len >= data_len || out_len == 0) { \
+ *out_length = data_len; \
+ } else { \
+ *out_length = out_len; \
+ } \
+ return data; \
}
-#define GDV_FN_CAST_VARCHAR_REAL(IN_TYPE, ARROW_TYPE) \
- GANDIVA_EXPORT \
- const char* gdv_fn_castVARCHAR_##IN_TYPE##_int64(int64_t context, gdv_##IN_TYPE value, \
- int64_t len, int32_t * out_len) { \
- if (len < 0) { \
- gdv_fn_context_set_error_msg(context, "Buffer length can not be negative"); \
- *out_len = 0; \
- return ""; \
- } \
- if (len == 0) { \
- *out_len = 0; \
- return ""; \
- } \
- gandiva::GdvStringFormatter<arrow::ARROW_TYPE> formatter; \
- char* ret = reinterpret_cast<char*>( \
- gdv_fn_context_arena_malloc(context, static_cast<int32_t>(len))); \
- if (ret == nullptr) { \
- gdv_fn_context_set_error_msg(context, "Could not allocate memory"); \
- *out_len = 0; \
- return ""; \
- } \
- arrow::Status status = formatter(value, [&](arrow::util::string_view v) { \
- int64_t size = static_cast<int64_t>(v.size()); \
- *out_len = static_cast<int32_t>(len < size ? len : size); \
- memcpy(ret, v.data(), *out_len); \
- return arrow::Status::OK(); \
- }); \
- if (!status.ok()) { \
- std::string err = "Could not cast " + std::to_string(value) + " to string"; \
- gdv_fn_context_set_error_msg(context, err.c_str()); \
- *out_len = 0; \
- return ""; \
- } \
- return ret; \
+CAST_VARBINARY_FROM_STRING_AND_BINARY(utf8)
+CAST_VARBINARY_FROM_STRING_AND_BINARY(binary)
+
+#undef CAST_VARBINARY_FROM_STRING_AND_BINARY
+
+#define GDV_FN_CAST_VARCHAR_VARBINARY_INTEGER(IN_TYPE, CAST_NAME, ARROW_TYPE) \
+ GANDIVA_EXPORT \
+ const char* gdv_fn_cast##CAST_NAME##_##IN_TYPE##_int64( \
+ int64_t context, gdv_##IN_TYPE value, int64_t len, int32_t * out_len) { \
+ if (len < 0) { \
+ gdv_fn_context_set_error_msg(context, "Buffer length can not be negative"); \
+ *out_len = 0; \
+ return ""; \
+ } \
+ if (len == 0) { \
+ *out_len = 0; \
+ return ""; \
+ } \
+ arrow::internal::StringFormatter<arrow::ARROW_TYPE> formatter; \
+ char* ret = reinterpret_cast<char*>( \
+ gdv_fn_context_arena_malloc(context, static_cast<int32_t>(len))); \
+ if (ret == nullptr) { \
+ gdv_fn_context_set_error_msg(context, "Could not allocate memory"); \
+ *out_len = 0; \
+ return ""; \
+ } \
+ arrow::Status status = formatter(value, [&](arrow::util::string_view v) { \
+ int64_t size = static_cast<int64_t>(v.size()); \
+ *out_len = static_cast<int32_t>(len < size ? len : size); \
+ memcpy(ret, v.data(), *out_len); \
+ return arrow::Status::OK(); \
+ }); \
+ if (!status.ok()) { \
+ std::string err = "Could not cast " + std::to_string(value) + " to string"; \
+ gdv_fn_context_set_error_msg(context, err.c_str()); \
+ *out_len = 0; \
+ return ""; \
+ } \
+ return ret; \
}
-GDV_FN_CAST_VARCHAR_INTEGER(int32, Int32Type)
-GDV_FN_CAST_VARCHAR_INTEGER(int64, Int64Type)
-GDV_FN_CAST_VARCHAR_REAL(float32, FloatType)
-GDV_FN_CAST_VARCHAR_REAL(float64, DoubleType)
+#define GDV_FN_CAST_VARCHAR_VARBINARY_REAL(IN_TYPE, CAST_NAME, ARROW_TYPE) \
+ GANDIVA_EXPORT \
+ const char* gdv_fn_cast##CAST_NAME##_##IN_TYPE##_int64( \
+ int64_t context, gdv_##IN_TYPE value, int64_t len, int32_t * out_len) { \
+ if (len < 0) { \
+ gdv_fn_context_set_error_msg(context, "Buffer length can not be negative"); \
+ *out_len = 0; \
+ return ""; \
+ } \
+ if (len == 0) { \
+ *out_len = 0; \
+ return ""; \
+ } \
+ gandiva::GdvStringFormatter<arrow::ARROW_TYPE> formatter; \
+ char* ret = reinterpret_cast<char*>( \
+ gdv_fn_context_arena_malloc(context, static_cast<int32_t>(len))); \
+ if (ret == nullptr) { \
+ gdv_fn_context_set_error_msg(context, "Could not allocate memory"); \
+ *out_len = 0; \
+ return ""; \
+ } \
+ arrow::Status status = formatter(value, [&](arrow::util::string_view v) { \
+ int64_t size = static_cast<int64_t>(v.size()); \
+ *out_len = static_cast<int32_t>(len < size ? len : size); \
+ memcpy(ret, v.data(), *out_len); \
+ return arrow::Status::OK(); \
+ }); \
+ if (!status.ok()) { \
+ std::string err = "Could not cast " + std::to_string(value) + " to string"; \
+ gdv_fn_context_set_error_msg(context, err.c_str()); \
+ *out_len = 0; \
+ return ""; \
+ } \
+ return ret; \
+ }
-#undef GDV_FN_CAST_VARCHAR_INTEGER
-#undef GDV_FN_CAST_VARCHAR_REAL
+GDV_FN_CAST_VARCHAR_VARBINARY_INTEGER(int32, VARCHAR, Int32Type)
Review comment:
Solved
##########
File path: cpp/src/gandiva/function_registry_string.cc
##########
@@ -236,6 +236,30 @@ std::vector<NativeFunction> GetStringFunctionRegistry() {
NativeFunction("binary_string", {}, DataTypeVector{utf8()}, binary(),
kResultNullIfNull, "binary_string", NativeFunction::kNeedsContext),
+ NativeFunction("castVARBINARY", {}, DataTypeVector{binary(), int64()}, binary(),
+ kResultNullIfNull, "gdv_fn_castVARBINARY_binary",
Review comment:
Solved
--
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] jpedroantunes commented on a change in pull request #10023: ARROW-12378: [C++][Gandiva] Implement castVARBINARY functions
Posted by GitBox <gi...@apache.org>.
jpedroantunes commented on a change in pull request #10023:
URL: https://github.com/apache/arrow/pull/10023#discussion_r631027169
##########
File path: cpp/src/gandiva/precompiled/string_ops.cc
##########
@@ -533,89 +533,102 @@ const char* castVARCHAR_bool_int64(gdv_int64 context, gdv_boolean value,
return out;
}
-// Truncates the string to given length
-FORCE_INLINE
-const char* castVARCHAR_utf8_int64(gdv_int64 context, const char* data,
- gdv_int32 data_len, int64_t out_len,
- int32_t* out_length) {
- int32_t len = static_cast<int32_t>(out_len);
-
- if (len < 0) {
- gdv_fn_context_set_error_msg(context, "Output buffer length can't be negative");
- *out_length = 0;
- return "";
- }
-
- if (len >= data_len || len == 0) {
- *out_length = data_len;
- return data;
- }
-
- int32_t remaining = len;
- int32_t index = 0;
- bool is_multibyte = false;
- do {
- // In utf8, MSB of a single byte unicode char is always 0,
- // whereas for a multibyte character the MSB of each byte is 1.
- // So for a single byte char, a bitwise-and with x80 (10000000) will be 0
- // and it won't be 0 for bytes of a multibyte char
- char* data_ptr = const_cast<char*>(data);
-
- // we advance byte by byte till the 8 byte boundary then advance 8 bytes at a time
- auto num_bytes = reinterpret_cast<uintptr_t>(data_ptr) & 0x07;
- num_bytes = (8 - num_bytes) & 0x07;
- while (num_bytes > 0) {
- uint8_t* ptr = reinterpret_cast<uint8_t*>(data_ptr + index);
- if ((*ptr & 0x80) != 0) {
- is_multibyte = true;
- break;
- }
- index++;
- remaining--;
- num_bytes--;
- }
- if (is_multibyte) break;
- while (remaining >= 8) {
- uint64_t* ptr = reinterpret_cast<uint64_t*>(data_ptr + index);
- if ((*ptr & 0x8080808080808080) != 0) {
- is_multibyte = true;
- break;
- }
- index += 8;
- remaining -= 8;
- }
- if (is_multibyte) break;
- if (remaining >= 4) {
- uint32_t* ptr = reinterpret_cast<uint32_t*>(data_ptr + index);
- if ((*ptr & 0x80808080) != 0) break;
- index += 4;
- remaining -= 4;
- }
- while (remaining > 0) {
- uint8_t* ptr = reinterpret_cast<uint8_t*>(data_ptr + index);
- if ((*ptr & 0x80) != 0) {
- is_multibyte = true;
- break;
- }
- index++;
- remaining--;
- }
- if (is_multibyte) break;
- // reached here; all are single byte characters
- *out_length = len;
- return data;
- } while (false);
-
- // detected multibyte utf8 characters; slow path
- int32_t byte_pos = utf8_byte_pos(context, data + index, data_len - index, len - index);
- if (byte_pos < 0) {
- *out_length = 0;
- return "";
- }
-
- *out_length = index + byte_pos;
- return data;
-}
+// Add functions for castVARBINARY for utf8 and binary
+#define CAST_FROM_STRING_AND_BINARY(OUTPUT, TYPE_NAME) \
+ FORCE_INLINE \
+ const char* cast##OUTPUT##_##TYPE_NAME##_int64(gdv_int64 context, const char* data, \
+ gdv_int32 data_len, int64_t out_len, \
+ int32_t* out_length) { \
+ int32_t len = static_cast<int32_t>(out_len); \
+ \
+ if (len < 0) { \
+ gdv_fn_context_set_error_msg(context, "Output buffer length can't be negative"); \
+ *out_length = 0; \
+ return ""; \
+ } \
+ \
+ if (len >= data_len || len == 0) { \
+ *out_length = data_len; \
+ return data; \
+ } \
+ \
+ int32_t remaining = len; \
+ int32_t index = 0; \
+ bool is_multibyte = false; \
+ do { \
+ /* In utf8, MSB of a single byte unicode char is always 0, */ \
+ /* whereas for a multibyte character the MSB of each byte is 1. */ \
+ /* So for a single byte char, a bitwise-and with x80 (10000000) will be 0 */ \
+ /* and it won't be 0 for bytes of a multibyte char */ \
+ char* data_ptr = const_cast<char*>(data); \
+ \
+ /* advance byte by byte till the 8 byte boundary, advance 8 bytes at a time */ \
+ auto num_bytes = reinterpret_cast<uintptr_t>(data_ptr) & 0x07; \
+ num_bytes = (8 - num_bytes) & 0x07; \
+ while (num_bytes > 0) { \
+ uint8_t* ptr = reinterpret_cast<uint8_t*>(data_ptr + index); \
+ if ((*ptr & 0x80) != 0) { \
+ is_multibyte = true; \
+ break; \
+ } \
+ index++; \
+ remaining--; \
+ num_bytes--; \
+ } \
+ if (is_multibyte) break; \
+ while (remaining >= 8) { \
+ uint64_t* ptr = reinterpret_cast<uint64_t*>(data_ptr + index); \
+ if ((*ptr & 0x8080808080808080) != 0) { \
+ is_multibyte = true; \
+ break; \
+ } \
+ index += 8; \
+ remaining -= 8; \
+ } \
+ if (is_multibyte) { \
+ break; \
+ } \
+ if (remaining >= 4) { \
+ uint32_t* ptr = reinterpret_cast<uint32_t*>(data_ptr + index); \
+ if ((*ptr & 0x80808080) != 0) break; \
+ index += 4; \
+ remaining -= 4; \
+ } \
+ while (remaining > 0) { \
+ uint8_t* ptr = reinterpret_cast<uint8_t*>(data_ptr + index); \
+ if ((*ptr & 0x80) != 0) { \
+ is_multibyte = true; \
+ break; \
+ } \
+ index++; \
+ remaining--; \
+ } \
+ if (is_multibyte) { \
+ break; \
+ } \
+ /* reached here; all are single byte characters */ \
+ *out_length = len; \
+ return data; \
+ } while (false); \
+ \
+ /* detected multibyte utf8 characters; slow path */ \
+ int32_t byte_pos = \
+ utf8_byte_pos(context, data + index, data_len - index, len - index); \
+ if (byte_pos < 0) { \
+ *out_length = 0; \
+ return ""; \
+ } \
+ \
+ *out_length = index + byte_pos; \
+ return data; \
+ }
+
+CAST_FROM_STRING_AND_BINARY(VARCHAR, utf8)
Review comment:
Ok! Used the implementation I did before
--
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 commented on pull request #10023: ARROW-12378: [C++][Gandiva] Implement castVARBINARY functions
Posted by GitBox <gi...@apache.org>.
praveenbingo commented on pull request #10023:
URL: https://github.com/apache/arrow/pull/10023#issuecomment-855806266
@projjal can you please check the errors on ths one.
--
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 pull request #10023: ARROW-12378: [C++][Gandiva] Implement castVARBINARY functions
Posted by GitBox <gi...@apache.org>.
projjal commented on pull request #10023:
URL: https://github.com/apache/arrow/pull/10023#issuecomment-855819631
@jpedroantunes this 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] projjal commented on a change in pull request #10023: ARROW-12378: [C++][Gandiva] Implement castVARBINARY functions
Posted by GitBox <gi...@apache.org>.
projjal commented on a change in pull request #10023:
URL: https://github.com/apache/arrow/pull/10023#discussion_r630996233
##########
File path: cpp/src/gandiva/precompiled/string_ops.cc
##########
@@ -533,89 +533,102 @@ const char* castVARCHAR_bool_int64(gdv_int64 context, gdv_boolean value,
return out;
}
-// Truncates the string to given length
-FORCE_INLINE
-const char* castVARCHAR_utf8_int64(gdv_int64 context, const char* data,
- gdv_int32 data_len, int64_t out_len,
- int32_t* out_length) {
- int32_t len = static_cast<int32_t>(out_len);
-
- if (len < 0) {
- gdv_fn_context_set_error_msg(context, "Output buffer length can't be negative");
- *out_length = 0;
- return "";
- }
-
- if (len >= data_len || len == 0) {
- *out_length = data_len;
- return data;
- }
-
- int32_t remaining = len;
- int32_t index = 0;
- bool is_multibyte = false;
- do {
- // In utf8, MSB of a single byte unicode char is always 0,
- // whereas for a multibyte character the MSB of each byte is 1.
- // So for a single byte char, a bitwise-and with x80 (10000000) will be 0
- // and it won't be 0 for bytes of a multibyte char
- char* data_ptr = const_cast<char*>(data);
-
- // we advance byte by byte till the 8 byte boundary then advance 8 bytes at a time
- auto num_bytes = reinterpret_cast<uintptr_t>(data_ptr) & 0x07;
- num_bytes = (8 - num_bytes) & 0x07;
- while (num_bytes > 0) {
- uint8_t* ptr = reinterpret_cast<uint8_t*>(data_ptr + index);
- if ((*ptr & 0x80) != 0) {
- is_multibyte = true;
- break;
- }
- index++;
- remaining--;
- num_bytes--;
- }
- if (is_multibyte) break;
- while (remaining >= 8) {
- uint64_t* ptr = reinterpret_cast<uint64_t*>(data_ptr + index);
- if ((*ptr & 0x8080808080808080) != 0) {
- is_multibyte = true;
- break;
- }
- index += 8;
- remaining -= 8;
- }
- if (is_multibyte) break;
- if (remaining >= 4) {
- uint32_t* ptr = reinterpret_cast<uint32_t*>(data_ptr + index);
- if ((*ptr & 0x80808080) != 0) break;
- index += 4;
- remaining -= 4;
- }
- while (remaining > 0) {
- uint8_t* ptr = reinterpret_cast<uint8_t*>(data_ptr + index);
- if ((*ptr & 0x80) != 0) {
- is_multibyte = true;
- break;
- }
- index++;
- remaining--;
- }
- if (is_multibyte) break;
- // reached here; all are single byte characters
- *out_length = len;
- return data;
- } while (false);
-
- // detected multibyte utf8 characters; slow path
- int32_t byte_pos = utf8_byte_pos(context, data + index, data_len - index, len - index);
- if (byte_pos < 0) {
- *out_length = 0;
- return "";
- }
-
- *out_length = index + byte_pos;
- return data;
-}
+// Add functions for castVARBINARY for utf8 and binary
+#define CAST_FROM_STRING_AND_BINARY(OUTPUT, TYPE_NAME) \
+ FORCE_INLINE \
+ const char* cast##OUTPUT##_##TYPE_NAME##_int64(gdv_int64 context, const char* data, \
+ gdv_int32 data_len, int64_t out_len, \
+ int32_t* out_length) { \
+ int32_t len = static_cast<int32_t>(out_len); \
+ \
+ if (len < 0) { \
+ gdv_fn_context_set_error_msg(context, "Output buffer length can't be negative"); \
+ *out_length = 0; \
+ return ""; \
+ } \
+ \
+ if (len >= data_len || len == 0) { \
+ *out_length = data_len; \
+ return data; \
+ } \
+ \
+ int32_t remaining = len; \
+ int32_t index = 0; \
+ bool is_multibyte = false; \
+ do { \
+ /* In utf8, MSB of a single byte unicode char is always 0, */ \
+ /* whereas for a multibyte character the MSB of each byte is 1. */ \
+ /* So for a single byte char, a bitwise-and with x80 (10000000) will be 0 */ \
+ /* and it won't be 0 for bytes of a multibyte char */ \
+ char* data_ptr = const_cast<char*>(data); \
+ \
+ /* advance byte by byte till the 8 byte boundary, advance 8 bytes at a time */ \
+ auto num_bytes = reinterpret_cast<uintptr_t>(data_ptr) & 0x07; \
+ num_bytes = (8 - num_bytes) & 0x07; \
+ while (num_bytes > 0) { \
+ uint8_t* ptr = reinterpret_cast<uint8_t*>(data_ptr + index); \
+ if ((*ptr & 0x80) != 0) { \
+ is_multibyte = true; \
+ break; \
+ } \
+ index++; \
+ remaining--; \
+ num_bytes--; \
+ } \
+ if (is_multibyte) break; \
+ while (remaining >= 8) { \
+ uint64_t* ptr = reinterpret_cast<uint64_t*>(data_ptr + index); \
+ if ((*ptr & 0x8080808080808080) != 0) { \
+ is_multibyte = true; \
+ break; \
+ } \
+ index += 8; \
+ remaining -= 8; \
+ } \
+ if (is_multibyte) { \
+ break; \
+ } \
+ if (remaining >= 4) { \
+ uint32_t* ptr = reinterpret_cast<uint32_t*>(data_ptr + index); \
+ if ((*ptr & 0x80808080) != 0) break; \
+ index += 4; \
+ remaining -= 4; \
+ } \
+ while (remaining > 0) { \
+ uint8_t* ptr = reinterpret_cast<uint8_t*>(data_ptr + index); \
+ if ((*ptr & 0x80) != 0) { \
+ is_multibyte = true; \
+ break; \
+ } \
+ index++; \
+ remaining--; \
+ } \
+ if (is_multibyte) { \
+ break; \
+ } \
+ /* reached here; all are single byte characters */ \
+ *out_length = len; \
+ return data; \
+ } while (false); \
+ \
+ /* detected multibyte utf8 characters; slow path */ \
+ int32_t byte_pos = \
+ utf8_byte_pos(context, data + index, data_len - index, len - index); \
+ if (byte_pos < 0) { \
+ *out_length = 0; \
+ return ""; \
+ } \
+ \
+ *out_length = index + byte_pos; \
+ return data; \
+ }
+
+CAST_FROM_STRING_AND_BINARY(VARCHAR, utf8)
Review comment:
Looks like you can't reuse the same function. varchar one takes character count which needs utf8 decoding
--
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 #10023: ARROW-12378: [C++][Gandiva] Implement castVARBINARY functions
Posted by GitBox <gi...@apache.org>.
projjal commented on a change in pull request #10023:
URL: https://github.com/apache/arrow/pull/10023#discussion_r623573772
##########
File path: cpp/src/gandiva/gdv_function_stubs.cc
##########
@@ -307,85 +307,114 @@ CAST_NUMERIC_FROM_STRING(double, arrow::DoubleType, FLOAT8)
#undef CAST_NUMERIC_FROM_STRING
-#define GDV_FN_CAST_VARCHAR_INTEGER(IN_TYPE, ARROW_TYPE) \
- GANDIVA_EXPORT \
- const char* gdv_fn_castVARCHAR_##IN_TYPE##_int64(int64_t context, gdv_##IN_TYPE value, \
- int64_t len, int32_t * out_len) { \
- if (len < 0) { \
- gdv_fn_context_set_error_msg(context, "Buffer length can not be negative"); \
- *out_len = 0; \
- return ""; \
- } \
- if (len == 0) { \
- *out_len = 0; \
- return ""; \
- } \
- arrow::internal::StringFormatter<arrow::ARROW_TYPE> formatter; \
- char* ret = reinterpret_cast<char*>( \
- gdv_fn_context_arena_malloc(context, static_cast<int32_t>(len))); \
- if (ret == nullptr) { \
- gdv_fn_context_set_error_msg(context, "Could not allocate memory"); \
- *out_len = 0; \
- return ""; \
- } \
- arrow::Status status = formatter(value, [&](arrow::util::string_view v) { \
- int64_t size = static_cast<int64_t>(v.size()); \
- *out_len = static_cast<int32_t>(len < size ? len : size); \
- memcpy(ret, v.data(), *out_len); \
- return arrow::Status::OK(); \
- }); \
- if (!status.ok()) { \
- std::string err = "Could not cast " + std::to_string(value) + " to string"; \
- gdv_fn_context_set_error_msg(context, err.c_str()); \
- *out_len = 0; \
- return ""; \
- } \
- return ret; \
+// Add functions for castVARBINARY for utf8 and binary
+#define CAST_VARBINARY_FROM_STRING_AND_BINARY(TYPE_NAME) \
Review comment:
You should move this function to the precompiled/string_ops.cc. There is arealy a similar castVARCHAR function, you can probably resuse that.
Also they seem to cast the int64 to int32 during arena_malloc. So probably you don't need to change the arena_malloc singature
##########
File path: cpp/src/gandiva/gdv_function_stubs.cc
##########
@@ -307,85 +307,114 @@ CAST_NUMERIC_FROM_STRING(double, arrow::DoubleType, FLOAT8)
#undef CAST_NUMERIC_FROM_STRING
-#define GDV_FN_CAST_VARCHAR_INTEGER(IN_TYPE, ARROW_TYPE) \
- GANDIVA_EXPORT \
- const char* gdv_fn_castVARCHAR_##IN_TYPE##_int64(int64_t context, gdv_##IN_TYPE value, \
- int64_t len, int32_t * out_len) { \
- if (len < 0) { \
- gdv_fn_context_set_error_msg(context, "Buffer length can not be negative"); \
- *out_len = 0; \
- return ""; \
- } \
- if (len == 0) { \
- *out_len = 0; \
- return ""; \
- } \
- arrow::internal::StringFormatter<arrow::ARROW_TYPE> formatter; \
- char* ret = reinterpret_cast<char*>( \
- gdv_fn_context_arena_malloc(context, static_cast<int32_t>(len))); \
- if (ret == nullptr) { \
- gdv_fn_context_set_error_msg(context, "Could not allocate memory"); \
- *out_len = 0; \
- return ""; \
- } \
- arrow::Status status = formatter(value, [&](arrow::util::string_view v) { \
- int64_t size = static_cast<int64_t>(v.size()); \
- *out_len = static_cast<int32_t>(len < size ? len : size); \
- memcpy(ret, v.data(), *out_len); \
- return arrow::Status::OK(); \
- }); \
- if (!status.ok()) { \
- std::string err = "Could not cast " + std::to_string(value) + " to string"; \
- gdv_fn_context_set_error_msg(context, err.c_str()); \
- *out_len = 0; \
- return ""; \
- } \
- return ret; \
+// Add functions for castVARBINARY for utf8 and binary
+#define CAST_VARBINARY_FROM_STRING_AND_BINARY(TYPE_NAME) \
+ GANDIVA_EXPORT \
+ const char* gdv_fn_castVARBINARY_##TYPE_NAME(int64_t context, const char* data, \
+ int32_t data_len, int64_t out_len, \
+ int64_t* out_length) { \
+ if (out_len < 0) { \
+ gdv_fn_context_set_error_msg(context, "Output buffer length can't be negative"); \
+ *out_length = 0; \
+ return ""; \
+ } \
+ \
+ if (out_len >= data_len || out_len == 0) { \
+ *out_length = data_len; \
+ } else { \
+ *out_length = out_len; \
+ } \
+ return data; \
}
-#define GDV_FN_CAST_VARCHAR_REAL(IN_TYPE, ARROW_TYPE) \
- GANDIVA_EXPORT \
- const char* gdv_fn_castVARCHAR_##IN_TYPE##_int64(int64_t context, gdv_##IN_TYPE value, \
- int64_t len, int32_t * out_len) { \
- if (len < 0) { \
- gdv_fn_context_set_error_msg(context, "Buffer length can not be negative"); \
- *out_len = 0; \
- return ""; \
- } \
- if (len == 0) { \
- *out_len = 0; \
- return ""; \
- } \
- gandiva::GdvStringFormatter<arrow::ARROW_TYPE> formatter; \
- char* ret = reinterpret_cast<char*>( \
- gdv_fn_context_arena_malloc(context, static_cast<int32_t>(len))); \
- if (ret == nullptr) { \
- gdv_fn_context_set_error_msg(context, "Could not allocate memory"); \
- *out_len = 0; \
- return ""; \
- } \
- arrow::Status status = formatter(value, [&](arrow::util::string_view v) { \
- int64_t size = static_cast<int64_t>(v.size()); \
- *out_len = static_cast<int32_t>(len < size ? len : size); \
- memcpy(ret, v.data(), *out_len); \
- return arrow::Status::OK(); \
- }); \
- if (!status.ok()) { \
- std::string err = "Could not cast " + std::to_string(value) + " to string"; \
- gdv_fn_context_set_error_msg(context, err.c_str()); \
- *out_len = 0; \
- return ""; \
- } \
- return ret; \
+CAST_VARBINARY_FROM_STRING_AND_BINARY(utf8)
+CAST_VARBINARY_FROM_STRING_AND_BINARY(binary)
+
+#undef CAST_VARBINARY_FROM_STRING_AND_BINARY
+
+#define GDV_FN_CAST_VARCHAR_VARBINARY_INTEGER(IN_TYPE, CAST_NAME, ARROW_TYPE) \
+ GANDIVA_EXPORT \
+ const char* gdv_fn_cast##CAST_NAME##_##IN_TYPE##_int64( \
+ int64_t context, gdv_##IN_TYPE value, int64_t len, int32_t * out_len) { \
+ if (len < 0) { \
+ gdv_fn_context_set_error_msg(context, "Buffer length can not be negative"); \
+ *out_len = 0; \
+ return ""; \
+ } \
+ if (len == 0) { \
+ *out_len = 0; \
+ return ""; \
+ } \
+ arrow::internal::StringFormatter<arrow::ARROW_TYPE> formatter; \
+ char* ret = reinterpret_cast<char*>( \
+ gdv_fn_context_arena_malloc(context, static_cast<int32_t>(len))); \
+ if (ret == nullptr) { \
+ gdv_fn_context_set_error_msg(context, "Could not allocate memory"); \
+ *out_len = 0; \
+ return ""; \
+ } \
+ arrow::Status status = formatter(value, [&](arrow::util::string_view v) { \
+ int64_t size = static_cast<int64_t>(v.size()); \
+ *out_len = static_cast<int32_t>(len < size ? len : size); \
+ memcpy(ret, v.data(), *out_len); \
+ return arrow::Status::OK(); \
+ }); \
+ if (!status.ok()) { \
+ std::string err = "Could not cast " + std::to_string(value) + " to string"; \
+ gdv_fn_context_set_error_msg(context, err.c_str()); \
+ *out_len = 0; \
+ return ""; \
+ } \
+ return ret; \
}
-GDV_FN_CAST_VARCHAR_INTEGER(int32, Int32Type)
-GDV_FN_CAST_VARCHAR_INTEGER(int64, Int64Type)
-GDV_FN_CAST_VARCHAR_REAL(float32, FloatType)
-GDV_FN_CAST_VARCHAR_REAL(float64, DoubleType)
+#define GDV_FN_CAST_VARCHAR_VARBINARY_REAL(IN_TYPE, CAST_NAME, ARROW_TYPE) \
+ GANDIVA_EXPORT \
+ const char* gdv_fn_cast##CAST_NAME##_##IN_TYPE##_int64( \
+ int64_t context, gdv_##IN_TYPE value, int64_t len, int32_t * out_len) { \
+ if (len < 0) { \
+ gdv_fn_context_set_error_msg(context, "Buffer length can not be negative"); \
+ *out_len = 0; \
+ return ""; \
+ } \
+ if (len == 0) { \
+ *out_len = 0; \
+ return ""; \
+ } \
+ gandiva::GdvStringFormatter<arrow::ARROW_TYPE> formatter; \
+ char* ret = reinterpret_cast<char*>( \
+ gdv_fn_context_arena_malloc(context, static_cast<int32_t>(len))); \
+ if (ret == nullptr) { \
+ gdv_fn_context_set_error_msg(context, "Could not allocate memory"); \
+ *out_len = 0; \
+ return ""; \
+ } \
+ arrow::Status status = formatter(value, [&](arrow::util::string_view v) { \
+ int64_t size = static_cast<int64_t>(v.size()); \
+ *out_len = static_cast<int32_t>(len < size ? len : size); \
+ memcpy(ret, v.data(), *out_len); \
+ return arrow::Status::OK(); \
+ }); \
+ if (!status.ok()) { \
+ std::string err = "Could not cast " + std::to_string(value) + " to string"; \
+ gdv_fn_context_set_error_msg(context, err.c_str()); \
+ *out_len = 0; \
+ return ""; \
+ } \
+ return ret; \
+ }
-#undef GDV_FN_CAST_VARCHAR_INTEGER
-#undef GDV_FN_CAST_VARCHAR_REAL
+GDV_FN_CAST_VARCHAR_VARBINARY_INTEGER(int32, VARCHAR, Int32Type)
Review comment:
This name looks confusing...make the original macro name GDV_FN_CAST_VARLEN_TYPE_FROM_INTEGER, then create two macros CAST_VARCHAR_FROM_INTEGER and CAST_VARBINARY_FROM_INTEGER that call the original macro and call these two macros with different numeric types
##########
File path: cpp/src/gandiva/function_registry_string.cc
##########
@@ -236,6 +236,30 @@ std::vector<NativeFunction> GetStringFunctionRegistry() {
NativeFunction("binary_string", {}, DataTypeVector{utf8()}, binary(),
kResultNullIfNull, "binary_string", NativeFunction::kNeedsContext),
+ NativeFunction("castVARBINARY", {}, DataTypeVector{binary(), int64()}, binary(),
+ kResultNullIfNull, "gdv_fn_castVARBINARY_binary",
Review comment:
add "_int64" to all the function names
--
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 #10023: ARROW-12378: [C++][Gandiva] Implement castVARBINARY functions
Posted by GitBox <gi...@apache.org>.
projjal commented on a change in pull request #10023:
URL: https://github.com/apache/arrow/pull/10023#discussion_r622719721
##########
File path: cpp/src/gandiva/context_helper.cc
##########
@@ -69,6 +69,11 @@ uint8_t* gdv_fn_context_arena_malloc(int64_t context_ptr, int32_t size) {
return context->arena()->Allocate(size);
}
+uint8_t* gdv_fn_context_arena_malloc_int64(int64_t context_ptr, int64_t size) {
Review comment:
why not keep only one method.
##########
File path: cpp/src/gandiva/gdv_function_stubs.cc
##########
@@ -216,6 +216,72 @@ SHA_VAR_LEN_PARAMS(SHA1_HASH_FUNCTION_BUF)
#undef SHA_NUMERIC_BOOL_DATE_PARAMS
#undef SHA_VAR_LEN_PARAMS
+// Add functions for castVARBINARY for utf8 and binary
+#define CAST_VARBINARY_FROM_STRING_AND_BINARY(TYPE_NAME) \
+ GANDIVA_EXPORT \
+ const char* gdv_fn_castVARBINARY_##TYPE_NAME(int64_t context, const char* data, \
+ int32_t data_len, int64_t out_len, \
+ int64_t* out_length) { \
+ if (out_len < 0) { \
+ gdv_fn_context_set_error_msg(context, "Output buffer length can't be negative"); \
+ *out_length = 0; \
+ return ""; \
+ } \
+ \
+ if (out_len >= data_len || out_len == 0) { \
+ *out_length = data_len; \
+ } else { \
+ *out_length = out_len; \
+ } \
+ return data; \
+ }
+
+CAST_VARBINARY_FROM_STRING_AND_BINARY(utf8)
+CAST_VARBINARY_FROM_STRING_AND_BINARY(binary)
+
+#undef CAST_VARBINARY_FROM_STRING_AND_BINARY
+
+// Add functions for castVARBINARY for numeric
+#define CAST_VARBINARY_FROM_NUMERIC(TYPE_NAME) \
+ GANDIVA_EXPORT \
+ const char* gdv_fn_castVARBINARY_##TYPE_NAME(int64_t context, gdv_##TYPE_NAME value, \
Review comment:
I think you should return the the bytes corresponding to the string representation of int/float. (opposite of what the change castINT from varbinary is doing)
--
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 #10023: ARROW-12378: [C++][Gandiva] Implement castVARBINARY functions
Posted by GitBox <gi...@apache.org>.
projjal commented on a change in pull request #10023:
URL: https://github.com/apache/arrow/pull/10023#discussion_r622948791
##########
File path: cpp/src/gandiva/gdv_function_stubs.cc
##########
@@ -216,6 +216,72 @@ SHA_VAR_LEN_PARAMS(SHA1_HASH_FUNCTION_BUF)
#undef SHA_NUMERIC_BOOL_DATE_PARAMS
#undef SHA_VAR_LEN_PARAMS
+// Add functions for castVARBINARY for utf8 and binary
+#define CAST_VARBINARY_FROM_STRING_AND_BINARY(TYPE_NAME) \
+ GANDIVA_EXPORT \
+ const char* gdv_fn_castVARBINARY_##TYPE_NAME(int64_t context, const char* data, \
+ int32_t data_len, int64_t out_len, \
+ int64_t* out_length) { \
+ if (out_len < 0) { \
+ gdv_fn_context_set_error_msg(context, "Output buffer length can't be negative"); \
+ *out_length = 0; \
+ return ""; \
+ } \
+ \
+ if (out_len >= data_len || out_len == 0) { \
+ *out_length = data_len; \
+ } else { \
+ *out_length = out_len; \
+ } \
+ return data; \
+ }
+
+CAST_VARBINARY_FROM_STRING_AND_BINARY(utf8)
+CAST_VARBINARY_FROM_STRING_AND_BINARY(binary)
+
+#undef CAST_VARBINARY_FROM_STRING_AND_BINARY
+
+// Add functions for castVARBINARY for numeric
+#define CAST_VARBINARY_FROM_NUMERIC(TYPE_NAME) \
+ GANDIVA_EXPORT \
+ const char* gdv_fn_castVARBINARY_##TYPE_NAME(int64_t context, gdv_##TYPE_NAME value, \
Review comment:
yes..you can resuse that macro i suppose
--
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] jpedroantunes commented on a change in pull request #10023: ARROW-12378: [C++][Gandiva] Implement castVARBINARY functions
Posted by GitBox <gi...@apache.org>.
jpedroantunes commented on a change in pull request #10023:
URL: https://github.com/apache/arrow/pull/10023#discussion_r622948149
##########
File path: cpp/src/gandiva/gdv_function_stubs.cc
##########
@@ -216,6 +216,72 @@ SHA_VAR_LEN_PARAMS(SHA1_HASH_FUNCTION_BUF)
#undef SHA_NUMERIC_BOOL_DATE_PARAMS
#undef SHA_VAR_LEN_PARAMS
+// Add functions for castVARBINARY for utf8 and binary
+#define CAST_VARBINARY_FROM_STRING_AND_BINARY(TYPE_NAME) \
+ GANDIVA_EXPORT \
+ const char* gdv_fn_castVARBINARY_##TYPE_NAME(int64_t context, const char* data, \
+ int32_t data_len, int64_t out_len, \
+ int64_t* out_length) { \
+ if (out_len < 0) { \
+ gdv_fn_context_set_error_msg(context, "Output buffer length can't be negative"); \
+ *out_length = 0; \
+ return ""; \
+ } \
+ \
+ if (out_len >= data_len || out_len == 0) { \
+ *out_length = data_len; \
+ } else { \
+ *out_length = out_len; \
+ } \
+ return data; \
+ }
+
+CAST_VARBINARY_FROM_STRING_AND_BINARY(utf8)
+CAST_VARBINARY_FROM_STRING_AND_BINARY(binary)
+
+#undef CAST_VARBINARY_FROM_STRING_AND_BINARY
+
+// Add functions for castVARBINARY for numeric
+#define CAST_VARBINARY_FROM_NUMERIC(TYPE_NAME) \
+ GANDIVA_EXPORT \
+ const char* gdv_fn_castVARBINARY_##TYPE_NAME(int64_t context, gdv_##TYPE_NAME value, \
Review comment:
If I get it correctly, I should do the same as the castVARCHAR for numeric types, but returning the correspondent bytes, correct?
--
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 #10023: ARROW-12378: [C++][Gandiva] Implement castVARBINARY functions
Posted by GitBox <gi...@apache.org>.
praveenbingo closed pull request #10023:
URL: https://github.com/apache/arrow/pull/10023
--
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