[GitHub] [arrow] rok commented on a change in pull request #9758: ARROW-9054: [C++] Add ScalarAggregateOptions

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

rok commented on a change in pull request #9758:
URL: https://github.com/apache/arrow/pull/9758#discussion_r628648646



##########
File path: cpp/src/arrow/compute/api_aggregate.h
##########
@@ -37,43 +37,17 @@ class ExecContext;
 // ----------------------------------------------------------------------
 // Aggregate functions
 
-/// \addtogroup compute-concrete-options
-/// @{
-
-/// \brief Control Count kernel behavior
-///
-/// By default, all non-null values are counted.
-struct ARROW_EXPORT CountOptions : public FunctionOptions {
-  enum Mode {
-    /// Count all non-null values.
-    COUNT_NON_NULL = 0,
-    /// Count all null values.
-    COUNT_NULL,
-  };
-
-  explicit CountOptions(enum Mode count_mode = COUNT_NON_NULL) : count_mode(count_mode) {}
-
-  static CountOptions Defaults() { return CountOptions(COUNT_NON_NULL); }
-
-  enum Mode count_mode;
-};
-
-/// \brief Control MinMax kernel behavior
+/// \brief Control general scalar aggregate kernel behavior
 ///
 /// By default, null values are ignored
-struct ARROW_EXPORT MinMaxOptions : public FunctionOptions {
-  enum Mode {
-    /// Skip null values
-    SKIP = 0,
-    /// Any nulls will result in null output
-    EMIT_NULL
-  };
+struct ARROW_EXPORT ScalarAggregateOptions : public FunctionOptions {
+  explicit ScalarAggregateOptions(bool skip_nulls = true, uint32_t min_count = 1)
+      : skip_nulls(skip_nulls), min_count(min_count) {}
 
-  explicit MinMaxOptions(enum Mode null_handling = SKIP) : null_handling(null_handling) {}
+  static ScalarAggregateOptions Defaults() { return ScalarAggregateOptions{}; }
 
-  static MinMaxOptions Defaults() { return MinMaxOptions{}; }
-
-  enum Mode null_handling;
+  bool skip_nulls = true;
+  uint32_t min_count = 1;

Review comment:
       Switching back to `min_count=0` as it seems to be preferred behavior in c++ (e.g. hash aggregate tests).
   Defaults in python and R can still be set differently.




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