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/05/24 13:37:12 UTC

[GitHub] [arrow] rodrigojdebem opened a new pull request #10385: ARROW-12858: [C++][Gandiva] Add isNull, isTrue and isFalse functions on Gandiva

rodrigojdebem opened a new pull request #10385:
URL: https://github.com/apache/arrow/pull/10385


   


-- 
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] ursabot edited a comment on pull request #10385: ARROW-12858: [C++][Gandiva] Add isNull, isTrue, isFalse, isNotTrue, IsNotFalse and NVL functions on Gandiva

Posted by GitBox <gi...@apache.org>.
ursabot edited a comment on pull request #10385:
URL: https://github.com/apache/arrow/pull/10385#issuecomment-985452085


   Benchmark runs are scheduled for baseline = bdb064bd31f6a5eeec688ac6bad88d81e8be8ca5 and contender = a6c81e621b637479a5828c4bdb95ec442ad94bb5. a6c81e621b637479a5828c4bdb95ec442ad94bb5 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
   Conbench compare runs links:
   [Finished :arrow_down:0.0% :arrow_up:0.0%] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/a139c43c5bd74a67a01b7e9a4d1e5481...f0dc01b023534d96b0bac4d36ea14c3e/)
   [Failed] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/b23e7c48ae8f4b5c88545352c6da6cb8...602e996a876949659442f3c145b84f06/)
   [Scheduled] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/44170bd7c7464b36a45a2228461b372d...5c7ec03f40ac4f25a089b656edc6d4b8/)
   Supported benchmarks:
   ursa-i9-9960x: langs = Python, R, JavaScript
   ursa-thinkcentre-m75q: langs = C++, Java
   ec2-t3-xlarge-us-east-2: cloud = True
   


-- 
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] rodrigojdebem commented on a change in pull request #10385: ARROW-12858: [C++][Gandiva] Add isNull, isTrue, isFalse, isNotTrue, IsNotFalse and NVL functions on Gandiva

Posted by GitBox <gi...@apache.org>.
rodrigojdebem commented on a change in pull request #10385:
URL: https://github.com/apache/arrow/pull/10385#discussion_r641509120



##########
File path: cpp/src/gandiva/function_registry_datetime.cc
##########
@@ -36,6 +36,7 @@ namespace gandiva {
 
 std::vector<NativeFunction> GetDateTimeFunctionRegistry() {
   static std::vector<NativeFunction> date_time_fn_registry_ = {
+      UNARY_SAFE_NULL_NEVER_BOOL(isnull, {}, day_time_interval),

Review comment:
       @projjal I added `ISNULL` for `day_time_interval` and `month_interval` because the rest already has it implemented.




-- 
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 #10385: ARROW-12858: [C++][Gandiva] Add isNull, isTrue, isFalse, isNotTrue, IsNotFalse and NVL functions on Gandiva

Posted by GitBox <gi...@apache.org>.
praveenbingo commented on pull request #10385:
URL: https://github.com/apache/arrow/pull/10385#issuecomment-892550094


   @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.

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 #10385: ARROW-12858: [C++][Gandiva] Add isNull, isTrue and isFalse functions on Gandiva

Posted by GitBox <gi...@apache.org>.
projjal commented on a change in pull request #10385:
URL: https://github.com/apache/arrow/pull/10385#discussion_r639406673



##########
File path: cpp/src/gandiva/function_registry_datetime.cc
##########
@@ -36,6 +36,7 @@ namespace gandiva {
 
 std::vector<NativeFunction> GetDateTimeFunctionRegistry() {
   static std::vector<NativeFunction> date_time_fn_registry_ = {
+      UNARY_SAFE_NULL_NEVER_BOOL(isnull, {}, day_time_interval),

Review comment:
       can you add isnull to all the types?




-- 
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 #10385: ARROW-12858: [C++][Gandiva] Add isNull, isTrue and isFalse functions on Gandiva

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #10385:
URL: https://github.com/apache/arrow/pull/10385#issuecomment-847052853


   https://issues.apache.org/jira/browse/ARROW-12858


-- 
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 #10385: ARROW-12858: [C++][Gandiva] Add isNull, isTrue, isFalse, isNotTrue, IsNotFalse and NVL functions on Gandiva

Posted by GitBox <gi...@apache.org>.
projjal commented on pull request #10385:
URL: https://github.com/apache/arrow/pull/10385#issuecomment-983468495


   @anthonylouisbsb @jvictorhuguenin Can you 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] ursabot commented on pull request #10385: ARROW-12858: [C++][Gandiva] Add isNull, isTrue, isFalse, isNotTrue, IsNotFalse and NVL functions on Gandiva

Posted by GitBox <gi...@apache.org>.
ursabot commented on pull request #10385:
URL: https://github.com/apache/arrow/pull/10385#issuecomment-985452085


   Benchmark runs are scheduled for baseline = bdb064bd31f6a5eeec688ac6bad88d81e8be8ca5 and contender = a6c81e621b637479a5828c4bdb95ec442ad94bb5. a6c81e621b637479a5828c4bdb95ec442ad94bb5 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
   Conbench compare runs links:
   [Scheduled] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/a139c43c5bd74a67a01b7e9a4d1e5481...f0dc01b023534d96b0bac4d36ea14c3e/)
   [Scheduled] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/b23e7c48ae8f4b5c88545352c6da6cb8...602e996a876949659442f3c145b84f06/)
   [Scheduled] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/44170bd7c7464b36a45a2228461b372d...5c7ec03f40ac4f25a089b656edc6d4b8/)
   Supported benchmarks:
   ursa-i9-9960x: langs = Python, R, JavaScript
   ursa-thinkcentre-m75q: langs = C++, Java
   ec2-t3-xlarge-us-east-2: cloud = True
   


-- 
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] anthonylouisbsb commented on pull request #10385: ARROW-12858: [C++][Gandiva] Add isNull, isTrue, isFalse, isNotTrue, IsNotFalse and NVL functions on Gandiva

Posted by GitBox <gi...@apache.org>.
anthonylouisbsb commented on pull request #10385:
URL: https://github.com/apache/arrow/pull/10385#issuecomment-983863232


   @projjal I will do it


-- 
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 #10385: ARROW-12858: [C++][Gandiva] Add isNull, isTrue, isFalse, isNotTrue, IsNotFalse and NVL functions on Gandiva

Posted by GitBox <gi...@apache.org>.
praveenbingo commented on pull request #10385:
URL: https://github.com/apache/arrow/pull/10385#issuecomment-892550094


   @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.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] ursabot edited a comment on pull request #10385: ARROW-12858: [C++][Gandiva] Add isNull, isTrue, isFalse, isNotTrue, IsNotFalse and NVL functions on Gandiva

Posted by GitBox <gi...@apache.org>.
ursabot edited a comment on pull request #10385:
URL: https://github.com/apache/arrow/pull/10385#issuecomment-985452085


   Benchmark runs are scheduled for baseline = bdb064bd31f6a5eeec688ac6bad88d81e8be8ca5 and contender = a6c81e621b637479a5828c4bdb95ec442ad94bb5. a6c81e621b637479a5828c4bdb95ec442ad94bb5 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
   Conbench compare runs links:
   [Finished :arrow_down:0.0% :arrow_up:0.0%] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/a139c43c5bd74a67a01b7e9a4d1e5481...f0dc01b023534d96b0bac4d36ea14c3e/)
   [Failed :arrow_down:0.0% :arrow_up:0.0%] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/b23e7c48ae8f4b5c88545352c6da6cb8...602e996a876949659442f3c145b84f06/)
   [Scheduled] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/44170bd7c7464b36a45a2228461b372d...5c7ec03f40ac4f25a089b656edc6d4b8/)
   Supported benchmarks:
   ursa-i9-9960x: langs = Python, R, JavaScript
   ursa-thinkcentre-m75q: langs = C++, Java
   ec2-t3-xlarge-us-east-2: cloud = True
   


-- 
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] rodrigojdebem commented on a change in pull request #10385: ARROW-12858: [C++][Gandiva] Add isNull, isTrue and isFalse functions on Gandiva

Posted by GitBox <gi...@apache.org>.
rodrigojdebem commented on a change in pull request #10385:
URL: https://github.com/apache/arrow/pull/10385#discussion_r640604513



##########
File path: cpp/src/gandiva/function_registry_datetime.cc
##########
@@ -36,6 +36,7 @@ namespace gandiva {
 
 std::vector<NativeFunction> GetDateTimeFunctionRegistry() {
   static std::vector<NativeFunction> date_time_fn_registry_ = {
+      UNARY_SAFE_NULL_NEVER_BOOL(isnull, {}, day_time_interval),

Review comment:
       @projjal suggestion applied 👍🏻 




-- 
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 #10385: ARROW-12858: [C++][Gandiva] Add isNull, isTrue, isFalse, isNotTrue, IsNotFalse and NVL functions on Gandiva

Posted by GitBox <gi...@apache.org>.
jpedroantunes commented on a change in pull request #10385:
URL: https://github.com/apache/arrow/pull/10385#discussion_r646536956



##########
File path: cpp/src/gandiva/function_registry_math_ops.cc
##########
@@ -84,6 +84,17 @@ std::vector<NativeFunction> GetMathOpsFunctionRegistry() {
       BINARY_GENERIC_SAFE_NULL_IF_NULL(truncate, {"trunc"}, decimal128, int32,
                                        decimal128),
 
+      NativeFunction("nvl", {}, DataTypeVector{int32(), int32()}, int32(),

Review comment:
       Solved!

##########
File path: cpp/src/gandiva/precompiled/arithmetic_ops.cc
##########
@@ -156,6 +170,17 @@ NUMERIC_TYPES(VALIDITY_OP, isnumeric, +)
   DATE_FUNCTION(INNER)                    \
   INNER(boolean)
 
+#define NVL(TYPE)                                                                   \
+  FORCE_INLINE                                                                      \
+  gdv_##TYPE nvl_##TYPE##_##TYPE(gdv_##TYPE in, gdv_boolean is_valid_in,            \
+                                 gdv_##TYPE replace, gdv_boolean is_valid_value) {  \
+    return (is_valid_in ? in : replace);                                            \
+  }
+
+NUMERIC_BOOL_DATE_FUNCTION(NVL)
+
+#undef VALIDITY_OP

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] projjal commented on pull request #10385: ARROW-12858: [C++][Gandiva] Add isNull, isTrue, isFalse, isNotTrue, IsNotFalse and NVL functions on Gandiva

Posted by GitBox <gi...@apache.org>.
projjal commented on pull request #10385:
URL: https://github.com/apache/arrow/pull/10385#issuecomment-984647860


   @anthonylouisbsb looks like there is still a conflict.


-- 
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] ursabot edited a comment on pull request #10385: ARROW-12858: [C++][Gandiva] Add isNull, isTrue, isFalse, isNotTrue, IsNotFalse and NVL functions on Gandiva

Posted by GitBox <gi...@apache.org>.
ursabot edited a comment on pull request #10385:
URL: https://github.com/apache/arrow/pull/10385#issuecomment-985452085


   Benchmark runs are scheduled for baseline = bdb064bd31f6a5eeec688ac6bad88d81e8be8ca5 and contender = a6c81e621b637479a5828c4bdb95ec442ad94bb5. a6c81e621b637479a5828c4bdb95ec442ad94bb5 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
   Conbench compare runs links:
   [Finished :arrow_down:0.0% :arrow_up:0.0%] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/a139c43c5bd74a67a01b7e9a4d1e5481...f0dc01b023534d96b0bac4d36ea14c3e/)
   [Failed :arrow_down:0.0% :arrow_up:0.0%] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/b23e7c48ae8f4b5c88545352c6da6cb8...602e996a876949659442f3c145b84f06/)
   [Finished :arrow_down:0.0% :arrow_up:0.0%] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/44170bd7c7464b36a45a2228461b372d...5c7ec03f40ac4f25a089b656edc6d4b8/)
   Supported benchmarks:
   ursa-i9-9960x: langs = Python, R, JavaScript
   ursa-thinkcentre-m75q: langs = C++, Java
   ec2-t3-xlarge-us-east-2: cloud = True
   


-- 
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] pravindra closed pull request #10385: ARROW-12858: [C++][Gandiva] Add isNull, isTrue, isFalse, isNotTrue, IsNotFalse and NVL functions on Gandiva

Posted by GitBox <gi...@apache.org>.
pravindra closed pull request #10385:
URL: https://github.com/apache/arrow/pull/10385


   


-- 
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 #10385: ARROW-12858: [C++][Gandiva] Add isNull, isTrue, isFalse, isNotTrue, IsNotFalse and NVL functions on Gandiva

Posted by GitBox <gi...@apache.org>.
projjal commented on a change in pull request #10385:
URL: https://github.com/apache/arrow/pull/10385#discussion_r646145879



##########
File path: cpp/src/gandiva/function_registry_math_ops.cc
##########
@@ -84,6 +84,17 @@ std::vector<NativeFunction> GetMathOpsFunctionRegistry() {
       BINARY_GENERIC_SAFE_NULL_IF_NULL(truncate, {"trunc"}, decimal128, int32,
                                        decimal128),
 
+      NativeFunction("nvl", {}, DataTypeVector{int32(), int32()}, int32(),

Review comment:
       nit: can use an existing macro for this

##########
File path: cpp/src/gandiva/precompiled/arithmetic_ops.cc
##########
@@ -156,6 +170,17 @@ NUMERIC_TYPES(VALIDITY_OP, isnumeric, +)
   DATE_FUNCTION(INNER)                    \
   INNER(boolean)
 
+#define NVL(TYPE)                                                                   \
+  FORCE_INLINE                                                                      \
+  gdv_##TYPE nvl_##TYPE##_##TYPE(gdv_##TYPE in, gdv_boolean is_valid_in,            \
+                                 gdv_##TYPE replace, gdv_boolean is_valid_value) {  \
+    return (is_valid_in ? in : replace);                                            \
+  }
+
+NUMERIC_BOOL_DATE_FUNCTION(NVL)
+
+#undef VALIDITY_OP

Review comment:
       undef NVL?




-- 
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] ursabot edited a comment on pull request #10385: ARROW-12858: [C++][Gandiva] Add isNull, isTrue, isFalse, isNotTrue, IsNotFalse and NVL functions on Gandiva

Posted by GitBox <gi...@apache.org>.
ursabot edited a comment on pull request #10385:
URL: https://github.com/apache/arrow/pull/10385#issuecomment-985452085


   Benchmark runs are scheduled for baseline = bdb064bd31f6a5eeec688ac6bad88d81e8be8ca5 and contender = a6c81e621b637479a5828c4bdb95ec442ad94bb5. a6c81e621b637479a5828c4bdb95ec442ad94bb5 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
   Conbench compare runs links:
   [Finished :arrow_down:0.0% :arrow_up:0.0%] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/a139c43c5bd74a67a01b7e9a4d1e5481...f0dc01b023534d96b0bac4d36ea14c3e/)
   [Scheduled] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/b23e7c48ae8f4b5c88545352c6da6cb8...602e996a876949659442f3c145b84f06/)
   [Scheduled] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/44170bd7c7464b36a45a2228461b372d...5c7ec03f40ac4f25a089b656edc6d4b8/)
   Supported benchmarks:
   ursa-i9-9960x: langs = Python, R, JavaScript
   ursa-thinkcentre-m75q: langs = C++, Java
   ec2-t3-xlarge-us-east-2: cloud = True
   


-- 
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