-
Notifications
You must be signed in to change notification settings - Fork 4k
ARROW-5791: [C++] Fix infinite loop with more the 32768 columns. #4762
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
Conversation
Codecov Report
@@ Coverage Diff @@
## master #4762 +/- ##
==========================================
+ Coverage 88.39% 88.95% +0.56%
==========================================
Files 908 717 -191
Lines 114157 98953 -15204
Branches 1418 0 -1418
==========================================
- Hits 100904 88021 -12883
+ Misses 12891 10932 -1959
+ Partials 362 0 -362
Continue to review full report at Codecov.
|
|
Thanks for tracking this down! Interesting that it's just a magic number and not an overflow. This LGTM, but I guess someone else should take a look. |
|
actually, I should remove code duplication. I also think maybe I allowed for too many columns. |
pitrou
left a comment
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.
Thanks for the fix!
|
Just in case its useful run from travis on my branch: https://travis-ci.org/emkornfield/arrow/builds/552739714 |
|
@ursabot build |
wesm
left a comment
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.
+1
But really 32768 columns should be enough for anyone :) Author: Micah Kornfield <emkornfield@gmail.com> Closes #4762 from emkornfield/csv and squashes the following commits: ab0504c <Micah Kornfield> lower number of columns in test to satisfy ming 8f53a8a <Micah Kornfield> remove test acfe2d8 <Micah Kornfield> remove cap, make min rows_in_chunk 512 08ddc22 <Micah Kornfield> remove floor duplication 211472a <Micah Kornfield> powers of 2 are better b91a9e1 <Micah Kornfield> ARROW-5791: Fix infinite loop with more the 32768 columns. Cap max columns
But really 32768 columns should be enough for anyone :) Author: Micah Kornfield <emkornfield@gmail.com> Closes #4762 from emkornfield/csv and squashes the following commits: ab0504c <Micah Kornfield> lower number of columns in test to satisfy ming 8f53a8a <Micah Kornfield> remove test acfe2d8 <Micah Kornfield> remove cap, make min rows_in_chunk 512 08ddc22 <Micah Kornfield> remove floor duplication 211472a <Micah Kornfield> powers of 2 are better b91a9e1 <Micah Kornfield> ARROW-5791: Fix infinite loop with more the 32768 columns. Cap max columns
But really 32768 columns should be enough for anyone :)