[GitHub] [arrow-datafusion] Dandandan commented on a change in pull request #844: Add ScalarValue::eq_array optimized comparison function

[GitBox <gi...@apache.org>]

Dandandan commented on a change in pull request #844:
URL: https://github.com/apache/arrow-datafusion/pull/844#discussion_r686066032



##########
File path: datafusion/src/scalar.rs
##########
@@ -399,6 +424,17 @@ macro_rules! build_array_from_option {
     }};
 }
 
+macro_rules! eq_array_primitive {
+    ($array:expr, $index:expr, $ARRAYTYPE:ident, $VALUE:expr) => {{
+        let array = $array.as_any().downcast_ref::<$ARRAYTYPE>().unwrap();
+        let is_valid = array.is_valid($index);
+        match $VALUE {

Review comment:
       I think it could if there was a specific `eq_array` implementation for non-null arrays.
   On most kernels / code, this has a non-negligible impact on performance.
   The code path in the hash aggregate could then check whether the array contains 0 nulls and choose a different implementation if this is the case.
   I think at this moment it might not have that much of an impact, maybe for the "easier" hash-aggregates with only few groups at might have a higher relative impact.




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