[REVIEW] Verify that concatenating columns does not overflow size_type #6809
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Please update the changelog in order to start CI tests. View the gpuCI docs here. |
Codecov Report
@@ Coverage Diff @@
## branch-0.17 #6809 +/- ##
============================================
Coverage 81.94% 81.94%
============================================
Files 96 96
Lines 16164 16164
============================================
Hits 13246 13246
Misses 2918 2918 Continue to review full report at Codecov.
|
kkraus14
added
3 - Ready for Review
trxcllnt
approved these changes
Nov 30, 2020
rgsl888prabhu
approved these changes
Nov 30, 2020
…e logic for how it guarded offset columns didn't end up mattering.
cwharris
approved these changes
Dec 1, 2020
Comment on lines
287
to
292
| // these pragmas work around the nvcc issue where if a column_view is used | ||
| // inside of a __device__ code path, these functions will end up being created | ||
| // as __host__ __device__ because they are explicitly defaulted. However, if | ||
| // they then end up being called by a simple __host__ function | ||
| // (eg std::vector destructor) you get a compile error because you're trying to | ||
| // call a __host__ __device__ function from a __host__ function. |
There was a problem hiding this comment.
are we able to use constexpr for these constructors, instead? that would allow them to be used interchangeably.
harrism
reviewed
Dec 1, 2020
nvdbaranec
added
non-breaking
harrism
changed the title
harrism
added
6 - Okay to Auto-Merge
and removed
3 - Ready for Review
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When concatenating large columns, it is possible to overflow the max size of a column (size_type). We need to verify this won't happen ahead of time. Previously, we were doing it for non-nested types, and only in the fused kernel concatenation case. This addresses both issues.