-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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 check to avoid runtime error and add tests for go/mysql/fastparse
#15000
Conversation
Signed-off-by: Noble Mittal <[email protected]>
Signed-off-by: Noble Mittal <[email protected]>
Review ChecklistHello reviewers! 👋 Please follow this checklist when reviewing this Pull Request. General
Tests
Documentation
New flags
If a workflow is added or modified:
Backward compatibility
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #15000 +/- ##
==========================================
+ Coverage 47.30% 47.34% +0.04%
==========================================
Files 1141 1144 +3
Lines 238965 238995 +30
==========================================
+ Hits 113040 113163 +123
+ Misses 117325 117243 -82
+ Partials 8600 8589 -11 ☔ View full report in Codecov by Sentry. |
We didn't parse the case properly where we had mod 2 base and the value was math.MinInt64, we'd reject that incorrectly. We need to check different for if we have a minus or not. Also adds automated tests for all these edge cases by testing the 1000 numbers before & after the limits for each type for each possible base. Signed-off-by: Dirkjan Bussink <[email protected]>
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.
LGTM! thank you 🙏🏻
Description
This PR adds a check to avoid runtime error and adds tests for
go/mysql/fastparse
Related Issue(s)
Fixes part of #14931
Checklist
Deployment Notes