-
Notifications
You must be signed in to change notification settings - Fork 901
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
Fix bool column corruption with ORC Reader #7483
Fix bool column corruption with ORC Reader #7483
Conversation
…ool_issue_with_reader
…ool_issue_with_reader
Codecov Report
@@ Coverage Diff @@
## branch-0.19 #7483 +/- ##
===============================================
+ Coverage 81.80% 82.26% +0.45%
===============================================
Files 101 101
Lines 16695 17244 +549
===============================================
+ Hits 13658 14185 +527
- Misses 3037 3059 +22
Continue to review full report at Codecov.
|
Looks like generating a test for the fix is more trouble than it's worth. Approving without the test. |
@@ -1556,7 +1556,7 @@ __global__ void __launch_bounds__(block_size) | |||
if (t == 0) { s->top.data.buffered_count = n; } | |||
} | |||
|
|||
numvals = min(numvals * 8, is_last_set ? s->top.data.max_vals : blockDim.x); | |||
numvals = min(numvals * 8, is_last_set ? (s->top.data.max_vals + 7) & (~0x7) : blockDim.x); |
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.
Given how common this is, we should have a ceil to next byte function rather than have a +7)&~7 everywhere.
@gpucibot merge |
Since there was a possibility that only partial set of bits would be read in case the values are buffered, the max_vals had to be rounded-off to higher next byte.
closes #7345