diff --git a/cpp/include/cudf/join/filtered_join.hpp b/cpp/include/cudf/join/filtered_join.hpp index 191a569cec4f..a8c23cf1397a 100644 --- a/cpp/include/cudf/join/filtered_join.hpp +++ b/cpp/include/cudf/join/filtered_join.hpp @@ -80,6 +80,8 @@ class filtered_join { * The right table is used as the filter applied to multiple left tables in subsequent * `semi_join` or `anti_join` calls. * + * @throws std::invalid_argument if `load_factor` is not in (0, 1] + * * @param right The right (filter) table used to build the hash table * @param compare_nulls Controls whether null join-key values should match or not * @param load_factor The desired ratio of filled slots to total slots in the hash table, must be diff --git a/cpp/include/cudf/join/mark_join.hpp b/cpp/include/cudf/join/mark_join.hpp index de455f9a8555..e24f7ed07625 100644 --- a/cpp/include/cudf/join/mark_join.hpp +++ b/cpp/include/cudf/join/mark_join.hpp @@ -88,7 +88,10 @@ class mark_join { cudf::get_current_device_resource_ref()); /** - * @brief Constructs a mark join object with explicit prefilter selection. + * @brief Constructs a mark join object with explicit prefilter selection and the given load + * factor. + * + * @throws std::invalid_argument if `load_factor` is not in (0, 1] * * @param left The left table; the hash table is built from this table * @param load_factor Hash table load factor in range (0,1] diff --git a/cpp/src/join/distinct_hash_join.cu b/cpp/src/join/distinct_hash_join.cu index 23697c611df1..9ba67702abf2 100644 --- a/cpp/src/join/distinct_hash_join.cu +++ b/cpp/src/join/distinct_hash_join.cu @@ -167,7 +167,7 @@ distinct_hash_join::distinct_hash_join(cudf::table_view const& right, _right{right}, _preprocessed_right{cudf::detail::row::equality::preprocessed_table::create(_right, stream)}, _hash_table{cuco::extent{static_cast(right.num_rows())}, - load_factor, + checked_load_factor(load_factor), cuco::empty_key{cuco::pair{std::numeric_limits::max(), rhs_index_type{cudf::JoinNoMatch}}}, always_not_equal{}, @@ -179,10 +179,6 @@ distinct_hash_join::distinct_hash_join(cudf::table_view const& right, { CUDF_FUNC_RANGE(); CUDF_EXPECTS(0 != this->_right.num_columns(), "Hash join right table is empty"); - CUDF_EXPECTS(load_factor > 0 && load_factor <= 1, - "Invalid load factor: must be greater than 0 and less than or equal to 1.", - std::invalid_argument); - size_type const right_table_num_rows{_right.num_rows()}; if (right_table_num_rows == 0) { return; } diff --git a/cpp/src/join/filtered_join.cu b/cpp/src/join/filtered_join.cu index 1247cdb78bb3..c2ca281e0e49 100644 --- a/cpp/src/join/filtered_join.cu +++ b/cpp/src/join/filtered_join.cu @@ -226,7 +226,8 @@ filtered_join::filtered_join(cudf::table_view const& right, _nulls_equal{compare_nulls}, _right{right}, _preprocessed_right{cudf::detail::row::equality::preprocessed_table::create(_right, stream)}, - _bucket_storage{cuco::extent{compute_bucket_storage_size(right, load_factor)}, + _bucket_storage{cuco::extent{ + compute_bucket_storage_size(right, checked_load_factor(load_factor))}, rmm::mr::polymorphic_allocator{std::move(mr)}, stream.value()} { diff --git a/cpp/src/join/hash_join/hash_join.cu b/cpp/src/join/hash_join/hash_join.cu index e2478802fc89..b699e04fff42 100644 --- a/cpp/src/join/hash_join/hash_join.cu +++ b/cpp/src/join/hash_join/hash_join.cu @@ -121,7 +121,7 @@ hash_join::hash_join(cudf::table_view const& right, _nulls_equal{compare_nulls}, _impl{std::make_unique(impl{typename impl::hash_table_t{ cuco::extent{static_cast(right.num_rows())}, - load_factor, + checked_load_factor(load_factor), cuco::empty_key{cuco::pair{std::numeric_limits::max(), cudf::JoinNoMatch}}, {}, {}, @@ -134,10 +134,6 @@ hash_join::hash_join(cudf::table_view const& right, { CUDF_FUNC_RANGE(); CUDF_EXPECTS(0 != right.num_columns(), "Hash join right table is empty", std::invalid_argument); - CUDF_EXPECTS(load_factor > 0 && load_factor <= 1, - "Invalid load factor: must be greater than 0 and less than or equal to 1.", - std::invalid_argument); - if (_is_empty) { return; } auto const row_bitmask = diff --git a/cpp/src/join/join_common_utils.hpp b/cpp/src/join/join_common_utils.hpp index 94ba17d52453..a5bf0ce6785c 100644 --- a/cpp/src/join/join_common_utils.hpp +++ b/cpp/src/join/join_common_utils.hpp @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: Copyright (c) 2019-2026, NVIDIA CORPORATION. + * SPDX-FileCopyrightText: Copyright (c) 2019-2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved. * SPDX-License-Identifier: Apache-2.0 */ #pragma once @@ -20,6 +20,15 @@ namespace cudf::detail { constexpr int DEFAULT_JOIN_BLOCK_SIZE = 128; +/** + * @brief Validates and returns a hash-table load factor. + * + * @param load_factor The load factor to validate + * @return The validated load factor + * @throws std::invalid_argument if `load_factor` is not in (0, 1] + */ +double checked_load_factor(double load_factor); + // Convenient alias for a pair of unique pointers to device uvectors. using VectorPair = std::pair>, std::unique_ptr>>; diff --git a/cpp/src/join/join_utils.cu b/cpp/src/join/join_utils.cu index c345eb6a9153..94b820c7eae7 100644 --- a/cpp/src/join/join_utils.cu +++ b/cpp/src/join/join_utils.cu @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: Copyright (c) 2021-2026, NVIDIA CORPORATION. + * SPDX-FileCopyrightText: Copyright (c) 2021-2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved. * SPDX-License-Identifier: Apache-2.0 */ @@ -36,6 +36,14 @@ namespace cudf { namespace detail { +double checked_load_factor(double load_factor) +{ + CUDF_EXPECTS(load_factor > 0.0 && load_factor <= 1.0, + "Invalid load factor: must be greater than 0 and less than or equal to 1.", + std::invalid_argument); + return load_factor; +} + VectorPair get_trivial_left_join_indices(table_view const& left, rmm::cuda_stream_view stream, rmm::device_async_resource_ref mr) diff --git a/cpp/src/join/mark_join.cu b/cpp/src/join/mark_join.cu index e34f28836c4b..c050170e3853 100644 --- a/cpp/src/join/mark_join.cu +++ b/cpp/src/join/mark_join.cu @@ -598,9 +598,10 @@ mark_join::mark_join(cudf::table_view const& left, _nulls_equal{compare_nulls}, _prefilter{prefilter}, _preprocessed_left{cudf::detail::row::equality::preprocessed_table::create(left, stream)}, - _bucket_storage{cuco::extent{compute_mark_join_capacity(left, load_factor)}, - rmm::mr::polymorphic_allocator{mr}, - stream.value()} + _bucket_storage{ + cuco::extent{compute_mark_join_capacity(left, checked_load_factor(load_factor))}, + rmm::mr::polymorphic_allocator{mr}, + stream.value()} { cudf::scoped_range range{"mark_join::mark_join"}; if (_left.num_rows() == 0) return; diff --git a/cpp/tests/join/distinct_join_tests.cpp b/cpp/tests/join/distinct_join_tests.cpp index 6e251c3d9413..37b3af814c3b 100644 --- a/cpp/tests/join/distinct_join_tests.cpp +++ b/cpp/tests/join/distinct_join_tests.cpp @@ -18,10 +18,9 @@ #include -#include - #include #include +#include #include template @@ -723,12 +722,15 @@ TEST_F(DistinctJoinTest, InvalidLoadFactor) Table t0(std::move(cols0)); // Test load factor of -0.1 - EXPECT_THROW(cudf::distinct_hash_join(t0, cudf::null_equality::EQUAL, -0.1), cuco::logic_error); + EXPECT_THROW(cudf::distinct_hash_join(t0, cudf::null_equality::EQUAL, -0.1), + std::invalid_argument); // Test load factor of 0 - EXPECT_THROW(cudf::distinct_hash_join(t0, cudf::null_equality::EQUAL, 0.0), cuco::logic_error); + EXPECT_THROW(cudf::distinct_hash_join(t0, cudf::null_equality::EQUAL, 0.0), + std::invalid_argument); // Test load factor > 1 - EXPECT_THROW(cudf::distinct_hash_join(t0, cudf::null_equality::EQUAL, 1.1), cuco::logic_error); + EXPECT_THROW(cudf::distinct_hash_join(t0, cudf::null_equality::EQUAL, 1.1), + std::invalid_argument); } TEST_F(DistinctJoinTest, DistinctLargeExtentOverflowPrevention) diff --git a/cpp/tests/join/join_tests.cpp b/cpp/tests/join/join_tests.cpp index 00dd1ce1431d..d328777418b8 100644 --- a/cpp/tests/join/join_tests.cpp +++ b/cpp/tests/join/join_tests.cpp @@ -30,14 +30,13 @@ #include #include -#include - #include #include #include #include #include #include +#include #include #include #include @@ -357,13 +356,13 @@ TEST_F(JoinTest, InvalidLoadFactor) // Test load factor of -0.1 EXPECT_THROW(cudf::hash_join(t0, cudf::nullable_join::NO, cudf::null_equality::EQUAL, -0.1), - cuco::logic_error); + std::invalid_argument); // Test load factor of 0 EXPECT_THROW(cudf::hash_join(t0, cudf::nullable_join::NO, cudf::null_equality::EQUAL, 0.0), - cuco::logic_error); + std::invalid_argument); // Test load factor > 1 EXPECT_THROW(cudf::hash_join(t0, cudf::nullable_join::NO, cudf::null_equality::EQUAL, 1.5), - cuco::logic_error); + std::invalid_argument); } struct JoinParameterizedTest : public JoinTest, public testing::WithParamInterface {}; diff --git a/cpp/tests/join/semi_anti_join_tests.cpp b/cpp/tests/join/semi_anti_join_tests.cpp index 9c02c3b4c805..37aee3640314 100644 --- a/cpp/tests/join/semi_anti_join_tests.cpp +++ b/cpp/tests/join/semi_anti_join_tests.cpp @@ -29,6 +29,7 @@ #include #include +#include template using column_wrapper = cudf::test::fixed_width_column_wrapper; @@ -516,6 +517,20 @@ TEST_F(SemiAntiJoinTest, MarkJoinPrefilterLoadFactorOverload) CUDF_TEST_EXPECT_TABLES_EQUIVALENT(*sorted_expected, *sorted_result); } +TEST_F(SemiAntiJoinTest, InvalidLoadFactor) +{ + column_wrapper keys{0, 1, 2}; + auto const table = cudf::table_view{{keys}}; + + for (auto const load_factor : {-0.1, 0.0, 1.1}) { + SCOPED_TRACE(load_factor); + EXPECT_THROW(cudf::filtered_join( + table, cudf::null_equality::EQUAL, load_factor, cudf::get_default_stream()), + std::invalid_argument); + EXPECT_THROW(cudf::mark_join(table, load_factor), std::invalid_argument); + } +} + TEST_F(SemiAntiJoinTest, FilteredJoinMemoryResource) { column_wrapper left_col0{0, 1, 2};