[GitHub] [arrow] nealrichardson commented on a change in pull request #10942: ARROW-13627: [C++] Fully support ScalarAggregateOptions in (hash) any/all/sum/product/mean

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

nealrichardson commented on a change in pull request #10942:
URL: https://github.com/apache/arrow/pull/10942#discussion_r691211988



##########
File path: cpp/src/arrow/compute/kernels/aggregate_basic.cc
##########
@@ -172,6 +173,7 @@ struct ProductImpl : public ScalarAggregator {
     } else {
       const auto& data = *batch[0].scalar();
       this->count += data.is_valid * batch.length;
+      this->nulls_observed = this->nulls_observed || !data.is_valid;

Review comment:
       Is there a possible optimization: if options.skip_nulls, either check the bitmask up front for missings and exit early if any, or exit after the first one is found? It looks like as it is, we still go through and count/sum/etc. all non-null values always.




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