-
Notifications
You must be signed in to change notification settings - Fork 891
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use std::size_t when computing join output size #9626
Conversation
std::size_t size; | ||
if constexpr (JoinKind == join_kind::LEFT_JOIN) { | ||
size = static_cast<size_type>( | ||
hash_table.pair_count_outer(iter, iter + probe_table_num_rows, equality, stream.value())); | ||
size = hash_table.pair_count_outer(iter, iter + probe_table_num_rows, equality, stream.value()); | ||
} else { | ||
size = static_cast<size_type>( | ||
hash_table.pair_count(iter, iter + probe_table_num_rows, equality, stream.value())); | ||
size = hash_table.pair_count(iter, iter + probe_table_num_rows, equality, stream.value()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: Indeed you can just do this:
if constexpr (JoinKind == join_kind::LEFT_JOIN) {
return hash_table.pair_count_outer(iter, iter + probe_table_num_rows, equality, stream.value());
} else {
return hash_table.pair_count(iter, iter + probe_table_num_rows, equality, stream.value());
}
Rerun tests. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! It's a wonder that this worked before for (moderately) explosive joins.
Codecov Report
@@ Coverage Diff @@
## branch-21.12 #9626 +/- ##
================================================
- Coverage 10.79% 10.66% -0.13%
================================================
Files 116 117 +1
Lines 18869 19825 +956
================================================
+ Hits 2036 2115 +79
- Misses 16833 17710 +877
Continue to review full report at Codecov.
|
Oh, it's my mistake when refactoring hash join. Thanks for fixing it! |
@gpucibot merge |
Fixes #9625. Updates
hash_join::compute_join_output_size
to use std::size_t instead of cudf::size_type as the intermediate type to hold the computed output size.