icexelloss commented on code in PR #34912:
URL: https://github.com/apache/arrow/pull/34912#discussion_r1170585181


##########
cpp/src/arrow/compute/kernels/hash_aggregate.cc:
##########
@@ -1249,35 +1249,56 @@ HashAggregateKernel 
MakeApproximateMedianKernel(HashAggregateFunction* tdigest_f
 }
 
 // ----------------------------------------------------------------------
-// FirstLast implementation
+// MinMax implementation
 
 template <typename CType>
-struct NullSentinel {
-  static constexpr CType value() { return std::numeric_limits<CType>::min(); }
+struct AntiExtrema {
+  static constexpr CType anti_min() { return 
std::numeric_limits<CType>::max(); }
+  static constexpr CType anti_max() { return 
std::numeric_limits<CType>::min(); }
 };
 
 template <>
-struct NullSentinel<float> {
-  static constexpr float value() { return 
std::numeric_limits<float>::infinity(); }
+struct AntiExtrema<bool> {
+  static constexpr bool anti_min() { return true; }
+  static constexpr bool anti_max() { return false; }
 };
 
 template <>
-struct NullSentinel<double> {
-  static constexpr double value() { return 
std::numeric_limits<double>::infinity(); }
+struct AntiExtrema<float> {
+  static constexpr float anti_min() { return 
std::numeric_limits<float>::infinity(); }
+  static constexpr float anti_max() { return 
-std::numeric_limits<float>::infinity(); }
+};
+
+template <>
+struct AntiExtrema<double> {
+  static constexpr double anti_min() { return 
std::numeric_limits<double>::infinity(); }
+  static constexpr double anti_max() { return 
-std::numeric_limits<double>::infinity(); }
+};
+
+template <>
+struct AntiExtrema<Decimal128> {
+  static constexpr Decimal128 anti_min() { return 
BasicDecimal128::GetMaxSentinel(); }
+  static constexpr Decimal128 anti_max() { return 
BasicDecimal128::GetMinSentinel(); }
+};
+
+template <>
+struct AntiExtrema<Decimal256> {
+  static constexpr Decimal256 anti_min() { return 
BasicDecimal256::GetMaxSentinel(); }
+  static constexpr Decimal256 anti_max() { return 
BasicDecimal256::GetMinSentinel(); }
 };
 
 template <typename Type, typename Enable = void>
-struct GroupedFirstLastImpl final : public GroupedAggregator {

Review Comment:
   I think I moved the code around and as a result github diff is confusing 



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

Reply via email to