Fix min/max reduction logic for dictionary columns (rapidsai#20847)

Fixes the logic for the min, max, and minmax aggregration types in `cudf::reduce` for dictionary types.
The previous implementation assumes the keys are sorted and therefore only the indices are required.
The changes correctly computes the aggregation using the keys/indices appropriately.

This is part of a larger effort to no longer require sorted keys in dictionary columns.

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

Approvers:
  - Bradley Dice (https://github.com/bdice)
  - Yunsong Wang (https://github.com/PointKernel)

URL: rapidsai#20847

Author: davidwendt

cpp/include/cudf/reduction/detail/reduction_functions.hpp

@@ -129,6 +129,17 @@ std::unique_ptr<scalar> argmax(column_view const& col,
                                rmm::cuda_stream_view stream,
                                rmm::device_async_resource_ref mr);
 
+/**
+ * @brief Computes the minimum and maximum values of the input column
+ *
+ * @param col input column to compute minmax
+ * @param stream CUDA stream used for device memory operations and kernel launches
+ * @param mr Device memory resource used to allocate the returned column's device memory
+ * @return A pair consisting of the minimum value and the maximum value
+ */
+std::pair<std::unique_ptr<scalar>, std::unique_ptr<scalar>> minmax(
+  cudf::column_view const& col, rmm::cuda_stream_view stream, rmm::device_async_resource_ref mr);
+
 /**
  * @brief Computes any of elements in input column is true when typecasted to bool
  *

cpp/src/reductions/max.cu

@@ -1,5 +1,5 @@
 /*
- * SPDX-FileCopyrightText: Copyright (c) 2019-2024, NVIDIA CORPORATION.
+ * SPDX-FileCopyrightText: Copyright (c) 2019-2025, NVIDIA CORPORATION.
  * SPDX-License-Identifier: Apache-2.0
  */
 
@@ -24,12 +24,9 @@ std::unique_ptr<cudf::scalar> max(column_view const& col,
   auto const input_type =
     cudf::is_dictionary(col.type()) ? cudf::dictionary_column_view(col).keys().type() : col.type();
   CUDF_EXPECTS(input_type == output_dtype, "max() operation requires matching output type");
-  auto const dispatch_type = cudf::is_dictionary(col.type())
-                               ? cudf::dictionary_column_view(col).indices().type()
-                               : col.type();
 
   using reducer = simple::detail::same_element_type_dispatcher<op::max>;
-  return cudf::type_dispatcher(dispatch_type, reducer{}, col, init, stream, mr);
+  return cudf::type_dispatcher(input_type, reducer{}, col, init, stream, mr);
 }
 
 }  // namespace detail

cpp/src/reductions/min.cu

@@ -1,5 +1,5 @@
 /*
- * SPDX-FileCopyrightText: Copyright (c) 2019-2024, NVIDIA CORPORATION.
+ * SPDX-FileCopyrightText: Copyright (c) 2019-2025, NVIDIA CORPORATION.
  * SPDX-License-Identifier: Apache-2.0
  */
 
@@ -21,12 +21,9 @@ std::unique_ptr<cudf::scalar> min(column_view const& col,
   auto const input_type =
     cudf::is_dictionary(col.type()) ? cudf::dictionary_column_view(col).keys().type() : col.type();
   CUDF_EXPECTS(input_type == output_dtype, "min() operation requires matching output type");
-  auto const dispatch_type = cudf::is_dictionary(col.type())
-                               ? cudf::dictionary_column_view(col).indices().type()
-                               : col.type();
 
   using reducer = simple::detail::same_element_type_dispatcher<op::min>;
-  return cudf::type_dispatcher(dispatch_type, reducer{}, col, init, stream, mr);
+  return cudf::type_dispatcher(input_type, reducer{}, col, init, stream, mr);
 }
 }  // namespace detail
 }  // namespace reduction

cpp/src/reductions/minmax.cu

@@ -13,6 +13,7 @@
 #include <cudf/detail/utilities/device_operators.cuh>
 #include <cudf/dictionary/dictionary_column_view.hpp>
 #include <cudf/reduction.hpp>
+#include <cudf/reduction/detail/reduction_functions.hpp>
 #include <cudf/scalar/scalar_factories.hpp>
 #include <cudf/utilities/default_stream.hpp>
 #include <cudf/utilities/memory_resource.hpp>
@@ -30,6 +31,7 @@
 #include <type_traits>
 
 namespace cudf {
+namespace reduction {
 namespace detail {
 
 namespace {
@@ -191,7 +193,7 @@ struct minmax_functor {
     auto minimum     = new ScalarType(T{}, true, stream, mr);
     auto maximum     = new ScalarType(T{}, true, stream, mr);
     // copy dev_result to the output scalars
-    device_single_thread(
+    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)};
   }
@@ -207,8 +209,6 @@ struct minmax_functor {
     // compute minimum and maximum values
     auto dev_result = reduce<cudf::string_view>(col, stream);
     // copy the minmax_pair to the host; does not copy the strings
-    using OutputType = minmax_pair<cudf::string_view>;
-
     auto const host_result = dev_result.value(stream);
     // strings are copied to create the scalars here
     return {std::make_unique<string_scalar>(host_result.min_val, true, stream, mr),
@@ -223,15 +223,15 @@ struct minmax_functor {
     cudf::column_view const& col, rmm::cuda_stream_view stream, rmm::device_async_resource_ref mr)
     requires(cudf::is_dictionary<T>())
   {
-    // compute minimum and maximum values
-    auto dev_result = reduce<T>(col, stream);
-    // copy the minmax_pair to the host to call get_element
-    using OutputType       = minmax_pair<T>;
-    OutputType host_result = dev_result.value(stream);
-    // get the keys for those indexes
-    auto const keys = dictionary_column_view(col).keys();
-    return {detail::get_element(keys, static_cast<size_type>(host_result.min_val), stream, mr),
-            detail::get_element(keys, static_cast<size_type>(host_result.max_val), stream, mr)};
+    // computes minimum and maximum on the dictionary indices as dictionary32 values
+    auto d_indices     = reduce<T>(col, stream);
+    auto const indices = d_indices.value(stream);
+    // use these values to slice the keys column (add 1 for complete inclusion)
+    auto keys = cudf::detail::slice(dictionary_column_view(col).keys(),
+                                    {indices.min_val.value(), indices.max_val.value() + 1},
+                                    stream)
+                  .front();
+    return type_dispatcher(keys.type(), minmax_functor{}, keys, stream, mr);
   }
 
   template <typename T>
@@ -263,12 +263,13 @@ std::pair<std::unique_ptr<scalar>, std::unique_ptr<scalar>> minmax(
   return type_dispatcher(col.type(), minmax_functor{}, col, stream, mr);
 }
 }  // namespace detail
+}  // namespace reduction
 
 std::pair<std::unique_ptr<scalar>, std::unique_ptr<scalar>> minmax(
   column_view const& col, rmm::cuda_stream_view stream, rmm::device_async_resource_ref mr)
 {
   CUDF_FUNC_RANGE();
-  return detail::minmax(col, stream, mr);
+  return reduction::detail::minmax(col, stream, mr);
 }
 
 }  // namespace cudf

cpp/src/reductions/simple.cuh

@@ -13,6 +13,7 @@
 #include <cudf/dictionary/detail/iterator.cuh>
 #include <cudf/dictionary/dictionary_column_view.hpp>
 #include <cudf/reduction/detail/reduction.cuh>
+#include <cudf/reduction/detail/reduction_functions.hpp>
 #include <cudf/scalar/scalar_device_view.cuh>
 #include <cudf/scalar/scalar_factories.hpp>
 #include <cudf/utilities/memory_resource.hpp>
@@ -332,12 +333,9 @@ struct same_element_type_dispatcher {
     if (!cudf::is_dictionary(col.type())) {
       return simple_reduction<ElementType, ElementType, Op>(col, init, stream, mr);
     }
-    auto index = simple_reduction<ElementType, ElementType, Op>(
-      dictionary_column_view(col).get_indices_annotated(),
-      init,
-      stream,
-      cudf::get_current_device_resource_ref());
-    return resolve_key<ElementType>(dictionary_column_view(col).keys(), *index, stream, mr);
+    auto mm = cudf::reduction::detail::minmax(col, stream, mr);
+    return std::is_same_v<Op, cudf::reduction::detail::op::min> ? std::move(mm.first)
+                                                                : std::move(mm.second);
   }
 
   template <typename ElementType>