-
-
Notifications
You must be signed in to change notification settings - Fork 4.3k
Case sensitive MS SQL database operations fail because of uppercase system table references #9337
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
Case sensitive DB collation threw errors for certain DB operations for MS SQL databases (e.g.: `removeColumn`). This was because the names of system schemas and views are all lowercase. Fixed the referenced names in `query-generator.js`
| inherits(DATE, BaseTypes.DATE); | ||
|
|
||
| DATE.prototype.toSql = function toSql() { | ||
| return 'DATETIMEOFFSET'; |
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.
?
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.
Sorry for the mixup and confusion: I started doing changes for a project of ours in the first place. The PR contained the initial changes we needed but we went further. The mistake was that I forgot that if I commit and push the new changes, this PR will also be updated.
Moved the new -and unrelated to this PR- changes to a different branch and reverted the changes you questioned.
lib/dialects/mssql/data-types.js
Outdated
| return 'DATETIME'; | ||
| }; | ||
|
|
||
| DATE.prototype._stringify = function _stringify(date) { |
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.
???
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.
Same applies to this comment as I previously mentioned.
sushantdhiman
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.
Some unrelated DATE changes in PR needs to be removed
Pull Request check-list
Please make sure to review and check all of these items:
npm run testornpm run test-DIALECTpass with this change (including linting)? (see below)npm run test-mssqlfails but it failed for me before my changes. After my changes it only fails at the same place:Description of change
Please read PR #9336 for description.