Refactor SimpleAggregateAdapter to move is_aligned_ flag into Accumul…

…atorType (#9268)

Summary:
Pull Request resolved: #9268

The simple UDAF authors used to define the aligned_accumulator_ flag in
the function class. This diff move this flag to inside AccumulatorType
struct in the function class since it's a accumulator-specific property.

Reviewed By: kevinwilfong

Differential Revision: D55389826

fbshipit-source-id: ff7fa152ec10fbb027933568dd4416a3720683b1

velox/docs/develop/aggregate-functions.rst

@@ -255,7 +255,7 @@ For aggregaiton functions of default-null behavior, the author defines an
     static constexpr bool use_external_memory_ = true;
 
     // Optional. Default is false.
-    static constexpr bool aligned_accumulator_ = true;
+    static constexpr bool is_aligned_ = true;
 
     explicit AccumulatorType(HashStringAllocator* allocator);
 
@@ -278,7 +278,7 @@ every accumulator takes fixed amount of memory. This flag is true by default.
 Next, the author defines another optional flag `use_external_memory_`
 indicating whether the accumulator uses memory that is not tracked by Velox.
 This flag is false by default. Then, the author can define an optional flag
-`aligned_accumulator_` indicating whether the accumulator requires aligned
+`is_aligned_` indicating whether the accumulator requires aligned
 access. This flag is false by default.
 
 The author defines a constructor that takes a single argument of
@@ -351,7 +351,7 @@ For aggregaiton functions of non-default-null behavior, the author defines an
     static constexpr bool use_external_memory_ = true;
 
     // Optional. Default is false.
-    static constexpr bool aligned_accumulator_ = true;
+    static constexpr bool is_aligned_ = true;
 
     explicit AccumulatorType(HashStringAllocator* allocator);
 
@@ -370,7 +370,7 @@ For aggregaiton functions of non-default-null behavior, the author defines an
   };
 
 The definition of `is_fixed_size_`, `use_external_memory_`,
-`aligned_accumulator_`, the constructor, and the `destroy` method are exactly
+`is_aligned_`, the constructor, and the `destroy` method are exactly
 the same as those for default-null behavior.
 
 On the other hand, the C++ function signatures of `addInput`, `combine`,

velox/exec/SimpleAggregateAdapter.h

@@ -151,11 +151,11 @@ class SimpleAggregateAdapter : public Aggregate {
   // Otherwise, SimpleAggregateAdapter::accumulatorAlignmentSize() returns
   // Aggregate::accumulatorAlignmentSize(), with a default value of 1.
   template <typename T, typename = void>
-  struct aligned_accumulator : std::false_type {};
+  struct accumulator_is_aligned : std::false_type {};
 
   template <typename T>
-  struct aligned_accumulator<T, std::void_t<decltype(T::aligned_accumulator_)>>
-      : std::integral_constant<bool, T::aligned_accumulator_> {};
+  struct accumulator_is_aligned<T, std::void_t<decltype(T::is_aligned_)>>
+      : std::integral_constant<bool, T::is_aligned_> {};
 
   static constexpr bool aggregate_default_null_behavior_ =
       aggregate_default_null_behavior<FUNC>::value;
@@ -172,7 +172,8 @@ class SimpleAggregateAdapter : public Aggregate {
   static constexpr bool support_to_intermediate_ =
       support_to_intermediate<FUNC>::value;
 
-  static constexpr bool aligned_accumulator_ = aligned_accumulator<FUNC>::value;
+  static constexpr bool accumulator_is_aligned_ =
+      accumulator_is_aligned<typename FUNC::AccumulatorType>::value;
 
   bool isFixedSize() const override {
     return accumulator_is_fixed_size_;
@@ -187,7 +188,7 @@ class SimpleAggregateAdapter : public Aggregate {
   }
 
   int32_t accumulatorAlignmentSize() const override {
-    if constexpr (aligned_accumulator_) {
+    if constexpr (accumulator_is_aligned_) {
       return alignof(typename FUNC::AccumulatorType);
     }
     return Aggregate::accumulatorAlignmentSize();

velox/functions/sparksql/aggregates/DecimalSumAggregate.h

@@ -43,8 +43,6 @@ class DecimalSumAggregate {
   /// default-null behavior is disabled.
   static constexpr bool default_null_behavior_ = false;
 
-  static constexpr bool aligned_accumulator_ = true;
-
   static bool toIntermediate(
       exec::out_type<Row<TSumType, bool>>& out,
       exec::optional_arg_type<TInputType> in) {
@@ -68,6 +66,8 @@ class DecimalSumAggregate {
     int64_t overflow{0};
     bool isEmpty{true};
 
+    static constexpr bool is_aligned_ = true;
+
     AccumulatorType() = delete;
 
     explicit AccumulatorType(HashStringAllocator* /*allocator*/) {}