From ec080eba750e51fe624dd40020084690d88b9d38 Mon Sep 17 00:00:00 2001 From: Divye Gala Date: Tue, 31 Oct 2023 16:16:28 -0400 Subject: [PATCH] Fix overflow check in `cudf::merge` (#14345) https://github.com/rapidsai/cudf/pull/14250 added a check to ensure `cudf::merge` throws when the total number of merged rows exceed `cudf::size_type` limit, however @bdice pointed out that the check was not correct because the accumulation was still occurring in `cudf::size_type`. This PR computes the accumulation in `std::size_t`. Authors: - Divye Gala (https://github.com/divyegala) Approvers: - Bradley Dice (https://github.com/bdice) - MithunR (https://github.com/mythrocks) - Nghia Truong (https://github.com/ttnghia) URL: https://github.com/rapidsai/cudf/pull/14345 --- cpp/src/merge/merge.cu | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/cpp/src/merge/merge.cu b/cpp/src/merge/merge.cu index e47abd6ede4..ee29c207cf1 100644 --- a/cpp/src/merge/merge.cu +++ b/cpp/src/merge/merge.cu @@ -638,13 +638,14 @@ table_ptr_type merge(std::vector const& tables_to_merge, CUDF_EXPECTS(key_cols.size() == column_order.size(), "Mismatched size between key_cols and column_order"); - CUDF_EXPECTS(std::accumulate(tables_to_merge.cbegin(), - tables_to_merge.cend(), - cudf::size_type{0}, - [](auto const& running_sum, auto const& tbl) { - return running_sum + tbl.num_rows(); - }) <= std::numeric_limits::max(), - "Total number of merged rows exceeds row limit"); + CUDF_EXPECTS( + std::accumulate(tables_to_merge.cbegin(), + tables_to_merge.cend(), + std::size_t{0}, + [](auto const& running_sum, auto const& tbl) { + return running_sum + static_cast(tbl.num_rows()); + }) <= static_cast(std::numeric_limits::max()), + "Total number of merged rows exceeds row limit"); // This utility will ensure all corresponding dictionary columns have matching keys. // It will return any new dictionary columns created as well as updated table_views.