-
Notifications
You must be signed in to change notification settings - Fork 541
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
Use native unicode data types for notification templates #6078
base: master
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #6078 +/- ##
=========================================
Coverage 40.80% 40.80%
Complexity 14478 14478
=========================================
Files 1764 1764
Lines 117814 117814
Branches 19131 19131
=========================================
Hits 48072 48072
Misses 62460 62460
Partials 7282 7282
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
BODY TEXT, | ||
FOOTER TEXT, | ||
SUBJECT NVARCHAR(4000), | ||
BODY NVARCHAR(MAX), |
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.
Cannot we consider VARBINARY
here?
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.
VARBINARY
is used to store binary data.. Here we are handling unicode text.. While we can technically use VARBINARY
to store this data in the binary format, wouldn't it better to use more native datatype which doesn't require additional binary-text conversion?
Also, can you share what's the problem we can solve, if we are to changing the datatype from NVARCHAR
to VARBINARY
?
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.
You're correct; if we only need to support Unicode text (+ HTML), then using NVARCHAR
should be sufficient.
d3daec3
to
8cbd0e9
Compare
26930dd
to
4041f72
Compare
Quality Gate passedIssues Measures |
Proposed changes in this pull request
$subject.
Config
When should this PR be merged
Immediate
Follow up actions
Merge,
Checklist (for reviewing)
General
Functionality
Code
Tests
Security
Documentation