Fix scale handling of fixed-point types in minmax aggregation for cudf::reduce (#23752)

Fixes the logic in `cudf::reduce` for the minmax aggregation handling fixed-point types with non-zero scale.
Also adds new gtests with non-zero fixed-point types.

Closes: #23752

Authors:
  - David Wendt (https://github.com/davidwendt)

Approvers:
  - Muhammad Haseeb (https://github.com/mhaseeb123)
  - Vukasin Milovanovic (https://github.com/vuule)

URL: #23752

Author: davidwendt

cpp/src/reductions/minmax.cu

@@ -17,6 +17,7 @@
 #include <cudf/scalar/scalar_factories.hpp>
 #include <cudf/utilities/default_stream.hpp>
 #include <cudf/utilities/memory_resource.hpp>
+#include <cudf/utilities/traits.hpp>
 
 #include <cuda/iterator>
 #include <cuda/std/functional>
@@ -144,6 +145,18 @@ struct assign_min_max {
   T* max_data;
 };
 
+template <typename T>
+std::unique_ptr<cudf::scalar_type_t<T>> make_minmax_scalar(cudf::data_type type,
+                                                           cuda::stream_ref stream,
+                                                           rmm::device_async_resource_ref mr)
+{
+  if constexpr (cudf::is_fixed_point<T>()) {
+    return std::make_unique<cudf::scalar_type_t<T>>(
+      device_storage_type_t<T>{}, numeric::scale_type{type.scale()}, true, stream, mr);
+  }
+  return std::make_unique<cudf::scalar_type_t<T>>(T{}, true, stream, mr);
+}
+
 /**
  * @brief Computes a minmax_pair<T> reduction directly over a dictionary column's decoded key
  * values, i.e. `keys[indices[i]]` for each row `i`.
@@ -185,10 +198,9 @@ struct minmax_dictionary_functor {
   {
     using storage_type  = device_storage_type_t<T>;
     auto dev_result     = reduce_dictionary<storage_type>(col, stream);
-    using ScalarType    = cudf::scalar_type_t<T>;
     auto const key_type = dictionary_column_view(col).keys().type();
-    auto minimum        = std::make_unique<ScalarType>(T{}, true, stream, mr);
-    auto maximum        = std::make_unique<ScalarType>(T{}, true, stream, mr);
+    auto minimum        = make_minmax_scalar<T>(key_type, stream, mr);
+    auto maximum        = make_minmax_scalar<T>(key_type, stream, mr);
     cudf::detail::device_single_thread(
       assign_min_max<storage_type>{dev_result.data(), minimum->data(), maximum->data()}, stream);
     return {std::move(minimum), std::move(maximum)};
@@ -255,13 +267,12 @@ struct minmax_functor {
     // compute minimum and maximum values
     auto dev_result = reduce<storage_type>(col, stream);
     // create output scalars
-    using ScalarType = cudf::scalar_type_t<T>;
-    auto minimum     = new ScalarType(T{}, true, stream, mr);
-    auto maximum     = new ScalarType(T{}, true, stream, mr);
+    auto minimum = make_minmax_scalar<T>(col.type(), stream, mr);
+    auto maximum = make_minmax_scalar<T>(col.type(), stream, mr);
     // copy dev_result to the output scalars
     cudf::detail::device_single_thread(
       assign_min_max<storage_type>{dev_result.data(), minimum->data(), maximum->data()}, stream);
-    return {std::unique_ptr<scalar>(minimum), std::unique_ptr<scalar>(maximum)};
+    return {std::move(minimum), std::move(maximum)};
   }
 
   /**

cpp/tests/reductions/reduction_tests.cpp

@@ -13,6 +13,7 @@
 #include <cudf/copying.hpp>
 #include <cudf/detail/iterator.cuh>
 #include <cudf/dictionary/dictionary_column_view.hpp>
+#include <cudf/dictionary/encode.hpp>
 #include <cudf/dictionary/update_keys.hpp>
 #include <cudf/fixed_point/fixed_point.hpp>
 #include <cudf/reduction.hpp>
@@ -2179,6 +2180,61 @@ TYPED_TEST(FixedPointTestAllReps, FixedPointReductionMaxLarge)
   }
 }
 
+TYPED_TEST(FixedPointTestAllReps, FixedPointMinMax)
+{
+  using namespace numeric;
+  using decimalXX  = TypeParam;
+  using RepType    = cudf::device_storage_type_t<decimalXX>;
+  using fp_wrapper = cudf::test::fixed_point_column_wrapper<RepType>;
+
+  for (auto const i : {0, -1, -2, -3}) {
+    auto const scale  = scale_type{i};
+    auto const column = fp_wrapper{{2, 3, 1, 4}, scale};
+
+    auto const expected_min = decimalXX{scaled_integer<RepType>{1, scale}};
+    auto const expected_max = decimalXX{scaled_integer<RepType>{4, scale}};
+
+    auto const result     = cudf::minmax(column);
+    auto const min_scalar = static_cast<cudf::scalar_type_t<decimalXX>*>(result.first.get());
+    auto const max_scalar = static_cast<cudf::scalar_type_t<decimalXX>*>(result.second.get());
+
+    // Scale must be preserved in the output scalars.
+    EXPECT_EQ(min_scalar->type().scale(), i);
+    EXPECT_EQ(max_scalar->type().scale(), i);
+
+    EXPECT_EQ(min_scalar->fixed_point_value(), expected_min);
+    EXPECT_EQ(max_scalar->fixed_point_value(), expected_max);
+  }
+}
+
+TYPED_TEST(FixedPointTestAllReps, FixedPointMinMaxWithNulls)
+{
+  using namespace numeric;
+  using decimalXX  = TypeParam;
+  using RepType    = cudf::device_storage_type_t<decimalXX>;
+  using fp_wrapper = cudf::test::fixed_point_column_wrapper<RepType>;
+
+  for (auto const i : {0, -1, -2, -3}) {
+    auto const scale = scale_type{i};
+    // valid: {2, null, 1, null, 4} — min=1, max=4
+    auto const column = fp_wrapper{{2, 3, 1, 5, 4}, {true, false, true, false, true}, scale};
+
+    auto const expected_min = decimalXX{scaled_integer<RepType>{1, scale}};
+    auto const expected_max = decimalXX{scaled_integer<RepType>{4, scale}};
+
+    auto const result     = cudf::minmax(column);
+    auto const min_scalar = static_cast<cudf::scalar_type_t<decimalXX>*>(result.first.get());
+    auto const max_scalar = static_cast<cudf::scalar_type_t<decimalXX>*>(result.second.get());
+
+    // Scale must be preserved in the output scalars.
+    EXPECT_EQ(min_scalar->type().scale(), i);
+    EXPECT_EQ(max_scalar->type().scale(), i);
+
+    EXPECT_EQ(min_scalar->fixed_point_value(), expected_min);
+    EXPECT_EQ(max_scalar->fixed_point_value(), expected_max);
+  }
+}
+
 TYPED_TEST(FixedPointTestAllReps, FixedPointReductionNUnique)
 {
   using namespace numeric;
@@ -3098,6 +3154,65 @@ TYPED_TEST(DictionaryReductionTest, Quantile)
     45.0);
 }
 
+template <typename T>
+struct FixedPointDictionaryReductionTest : public cudf::test::BaseFixture {};
+
+TYPED_TEST_SUITE(FixedPointDictionaryReductionTest, cudf::test::FixedPointTypes);
+
+TYPED_TEST(FixedPointDictionaryReductionTest, FixedPointDictionaryMinMax)
+{
+  using namespace numeric;
+  using decimalXX  = TypeParam;
+  using RepType    = cudf::device_storage_type_t<decimalXX>;
+  using fp_wrapper = cudf::test::fixed_point_column_wrapper<RepType>;
+
+  for (auto const i : {0, -1, -2, -3}) {
+    auto const scale        = scale_type{i};
+    auto const col          = fp_wrapper{{1, 2, 3, 4}, scale};
+    auto const dict         = cudf::dictionary::encode(col);
+    auto const expected_min = decimalXX{scaled_integer<RepType>{1, scale}};
+    auto const expected_max = decimalXX{scaled_integer<RepType>{4, scale}};
+
+    auto const result     = cudf::minmax(dict->view());
+    auto const min_scalar = static_cast<cudf::scalar_type_t<decimalXX>*>(result.first.get());
+    auto const max_scalar = static_cast<cudf::scalar_type_t<decimalXX>*>(result.second.get());
+
+    EXPECT_EQ(min_scalar->type().scale(), i);
+    EXPECT_EQ(max_scalar->type().scale(), i);
+    EXPECT_EQ(min_scalar->fixed_point_value(), expected_min);
+    EXPECT_EQ(max_scalar->fixed_point_value(), expected_max);
+    EXPECT_TRUE(min_scalar->is_valid());
+    EXPECT_TRUE(max_scalar->is_valid());
+  }
+}
+
+TYPED_TEST(FixedPointDictionaryReductionTest, FixedPointDictionaryMinMaxWithNulls)
+{
+  using namespace numeric;
+  using decimalXX  = TypeParam;
+  using RepType    = cudf::device_storage_type_t<decimalXX>;
+  using fp_wrapper = cudf::test::fixed_point_column_wrapper<RepType>;
+
+  for (auto const i : {0, -1, -2, -3}) {
+    auto const scale        = scale_type{i};
+    auto const col          = fp_wrapper{{1, 2, 3, 4, 5}, {true, false, true, false, true}, scale};
+    auto const dict         = cudf::dictionary::encode(col);
+    auto const expected_min = decimalXX{scaled_integer<RepType>{1, scale}};
+    auto const expected_max = decimalXX{scaled_integer<RepType>{5, scale}};
+
+    auto const result     = cudf::minmax(dict->view());
+    auto const min_scalar = static_cast<cudf::scalar_type_t<decimalXX>*>(result.first.get());
+    auto const max_scalar = static_cast<cudf::scalar_type_t<decimalXX>*>(result.second.get());
+
+    EXPECT_EQ(min_scalar->type().scale(), i);
+    EXPECT_EQ(max_scalar->type().scale(), i);
+    EXPECT_EQ(min_scalar->fixed_point_value(), expected_min);
+    EXPECT_EQ(max_scalar->fixed_point_value(), expected_max);
+    EXPECT_TRUE(min_scalar->is_valid());
+    EXPECT_TRUE(max_scalar->is_valid());
+  }
+}
+
 struct ListReductionTest : public cudf::test::BaseFixture {
   void reduction_test(cudf::column_view const& input_data,
                       cudf::column_view const& expected_value,