-
-
Notifications
You must be signed in to change notification settings - Fork 4.9k
Fix arithmetic overflow for MSSQL result backend #9904
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
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.
Pull Request Overview
This PR fixes an arithmetic overflow issue when using the MSSQL database backend with Celery by replacing Integer columns with BigInteger for MSSQL dialect while maintaining Integer for other databases.
- Introduces a dialect-specific column type that uses BigInteger for MSSQL and Integer for other databases
- Updates the primary key columns in Task and TaskSet models to use the new dialect-specific type
- Addresses issue #8634 where MSSQL's Integer type cannot accommodate large sequence values
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9904 +/- ##
=======================================
Coverage 78.67% 78.67%
=======================================
Files 153 153
Lines 19290 19291 +1
Branches 2568 2568
=======================================
+ Hits 15176 15177 +1
+ Misses 3816 3814 -2
- Partials 298 300 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Nusnus
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.
LGTM
Description
Apparently, not too many people are using celery with MSSQL. This change must fix #8634.
Perhaps there is no other way neither place to do this better:
SQLAlchemy seems to behave correctly: it uses Integer when specified, and it’s not SQLAlchemy's business that Integer cannot fit a sequence. As a consequence, the application should use proper field types.