pitrou commented on a change in pull request #10895:
URL: https://github.com/apache/arrow/pull/10895#discussion_r686223277
##########
File path: cpp/src/arrow/compute/kernels/aggregate_basic.cc
##########
@@ -82,23 +82,31 @@ struct CountImpl : public ScalarAggregator {
Status Finalize(KernelContext* ctx, Datum* out) override {
const auto& state = checked_cast<const CountImpl&>(*ctx->state());
- if (state.options.skip_nulls) {
- *out = Datum(state.non_nulls);
- } else {
- *out = Datum(state.nulls);
+ switch (state.options.mode) {
+ case CountOptions::NON_NULL:
+ *out = Datum(state.non_nulls);
+ break;
+ case CountOptions::NULLS:
+ *out = Datum(state.nulls);
+ break;
+ case CountOptions::ALL:
+ *out = Datum(state.non_nulls + state.nulls);
Review comment:
In this case, it's probably not necessary to popcount the null bitmap?
##########
File path: cpp/src/arrow/compute/api_aggregate.h
##########
@@ -53,6 +53,26 @@ class ARROW_EXPORT ScalarAggregateOptions : public
FunctionOptions {
uint32_t min_count;
};
+/// \brief Control count aggregate kernel behavior.
+///
+/// By default, only non-null values are counted.
+class ARROW_EXPORT CountOptions : public FunctionOptions {
+ public:
+ enum CountMode {
+ /// Count only non-null values.
+ NON_NULL = 0,
+ /// Count only null values.
+ NULLS,
Review comment:
Nit, but the plural / singular is a bit inconsistent. Perhaps ONLY_NULL
vs ONLY_VALID (or NULLS vs NON_NULLS, or NULL_VALUES vs VALID_VALUES...)?
##########
File path: cpp/src/arrow/compute/kernels/aggregate_basic.cc
##########
@@ -562,7 +570,7 @@ const FunctionDoc count_doc{"Count the number of null /
non-null values",
("By default, only non-null values are counted.\n"
"This can be changed through
ScalarAggregateOptions."),
Review comment:
"CountOptions"
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
