fix(rust): use wrapping_add in csv line snooping #15109
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #15109 +/- ##
==========================================
- Coverage 81.08% 81.08% -0.01%
==========================================
Files 1342 1342
Lines 174112 174112
Branches 2459 2459
==========================================
- Hits 141178 141174 -4
- Misses 32467 32470 +3
- Partials 467 468 +1 ☔ View full report in Codecov by Sentry. |
| @@ -153,7 +153,7 @@ pub(crate) fn next_line_position( | |||
| if rejected_line_groups >= 3 { | |||
| return None; | |||
| } | |||
| lines_checked += 1; | |||
| lines_checked = lines_checked.wrapping_add(1); | |||
There was a problem hiding this comment.
So we decrement expected fields at 256 lines, and afterwards we decrement it every 65536 lines? Since lines_checked is a u16.
There was a problem hiding this comment.
Yeah, good point. Should probably just use u8 here.
| // headers might have an extra value | ||
| // So if we have churned through enough lines | ||
| // we try one field less. | ||
| if lines_checked == 256 { | ||
| if lines_checked == u8::MAX { |
There was a problem hiding this comment.
minor nit - this is off-by-one on before the first wrap-around 😆
There was a problem hiding this comment.
Yeah, that's fine. It is just a heuristic to try less fields after some reasonable attempts. Off by one is still reasonable. 😄
fixes #15101