-
Notifications
You must be signed in to change notification settings - Fork 915
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::overflow_error when output would exceed column size limit #13323
Use std::overflow_error when output would exceed column size limit #13323
Conversation
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.
Do we need to do overflow comparisons with operands as std::size_t
always? Should we use a common detail function to do these comparisons, like:
#include <limits>
template<typename T>
bool check_column_size_overflow(T size) {
return static_cast<std::size_t>(size) <= static_cast<std::size_t>(std::numeric_limits<cudf::size_type>::max());
}
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.
Approving to unblock this because I think it's a net improvement. But I'm still in favor of doing this work with a helper function that makes this logic less repetitive across libcudf.
Note: While looking at the cudf/python/cudf/cudf/_lib/__init__.py Lines 43 to 46 in 707a480
I'll file a follow-up there. |
/merge |
Description
Replaces generic
cudf::logic_error
exception withstd::overflow_error
where appropriate in libcudf.Since this changes what is thrown in certain APIs, I think this technically is a breaking change.
Closes #12925
Checklist