-
Notifications
You must be signed in to change notification settings - Fork 74
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
Add _TSK_BIG_TABLES typedef. #1531
Conversation
b52e167
to
bf11f79
Compare
Codecov Report
@@ Coverage Diff @@
## main #1531 +/- ##
==========================================
- Coverage 93.70% 93.70% -0.01%
==========================================
Files 27 27
Lines 22760 22759 -1
Branches 1076 1076
==========================================
- Hits 21327 21326 -1
Misses 1399 1399
Partials 34 34
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
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.
Awesome! Looks good to me - you mentioned you had some concerns about this way of overflow checking, are you happy with this now?
I thought some more about it and you're right, it is much better! I got rid of the checks for > 0 since these are unsigned types, but basically it's what you proposed. I'm slightly worried about the fact we don't have any tests for the boundary behaviour for num_rows though, I tried leaving out the "+1" and all the tests still passed. |
Yep, added an issue for that #1532 |
@Mergifyio rebase |
bf11f79
to
212fdfd
Compare
Command
|
Yep, this is working on CircleCI, just verified it's definitely running in 64 bit mode by checking the total memory used reported in valgrind (much more in 64 bit mode). |
212fdfd
to
6f1c7be
Compare
6f1c7be
to
59bd85f
Compare
Closes #1528