Skip to content
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

ENH: align dbtypes for forum/group keys #536

Closed
johnhenley opened this issue Oct 5, 2023 · 2 comments · Fixed by #676
Closed

ENH: align dbtypes for forum/group keys #536

johnhenley opened this issue Oct 5, 2023 · 2 comments · Fixed by #676
Assignees
Labels
enhancement New feature or request performance An update likely to improve site performance technical debt Issue that doesn't directly affect usability but will improve technical debt posture
Milestone

Comments

@johnhenley
Copy link
Collaborator

Is your feature request related to a problem?

Security and settings keys for Forums/Groups are used in database procedures, and are not using the same database type, resulting in implicit conversion during procedure executions.

activeforums_Settings.GroupKey is nvarchar
activeforums_Security.SecurityKey is nvarchar

activeforums_Forums.ForumSettingsKey is varchar
activeforums_Forums.ForumSecurityKey is varchar

activeforums_Groups.GroupSettingsKey is varchar
activeforums_Groups.GroupSecurityKey is varchar

Describe the solution you'd like

Align these types to be consistent.

@johnhenley johnhenley added enhancement New feature or request technical debt Issue that doesn't directly affect usability but will improve technical debt posture performance An update likely to improve site performance labels Oct 5, 2023
@johnhenley johnhenley added this to the Future milestone Oct 5, 2023
@WillStrohl
Copy link
Member

As far as I remember, NVARCHAR is always a more flexible option, but these values don't necessarily need the flexibility. This is why I use NVARCHAR for anything that might be localized.

@johnhenley
Copy link
Collaborator Author

As far as I remember, NVARCHAR is always a more flexible option, but these values don't necessarily need the flexibility. This is why I use NVARCHAR for anything that might be localized.

Agreed. No need for nvarchar for these. Easy enough to fix but very low priority.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request performance An update likely to improve site performance technical debt Issue that doesn't directly affect usability but will improve technical debt posture
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants