Widen installation_history sql_server_versioncolumn #716
Widen installation_history sql_server_versioncolumn #716jakemorgangit wants to merge 7 commits intoerikdarlingdata:devfrom
Conversation
v2.2.0: Fix SignPath signing
Release v2.3.0
Release v2.4.0
Release v2.4.1
Increase sql_server_version and sql_server_edition to nvarchar(512) to prevent truncation of long SQL Server version strings.
erikdarlingdata
left a comment
There was a problem hiding this comment.
Thanks for tackling this — the upgrade script and schema change are solid. A couple of things to address before merge:
1. Remove the ALTER TABLE block from 01_install_database.sql (lines 644–672)
This project's convention is that schema changes for existing installations go in upgrades/ scripts only, not in the install script. Your CREATE TABLE on line 623 already has nvarchar(512), so new installs are covered. The upgrade script at upgrades/2.4.1-to-2.4.2/ handles existing installs. The ALTER TABLE block in the install script is redundant and goes against the project pattern — please remove it.
2. Branch hygiene
The PR head ref is main, which means this was committed directly to your fork's main branch. That pulls in unrelated upstream merge commits (there are 4 in the commit list). For future PRs, creating a feature branch (e.g. fix/widen-version-column) keeps the history clean. For this one, we can squash-merge, but a rebase onto a feature branch would be ideal.
3. Minor: commit message vs. actual change
The first commit message says "Increase sql_server_version and sql_server_edition to nvarchar(512)" but the diff only widens sql_server_version. sql_server_edition stays at nvarchar(255). That's probably fine (edition strings are short), but the commit message is misleading.
Everything else looks good — the upgrade script is properly idempotent with correct SET options, the upgrade.txt is in place, and the test helper is updated. Just need item #1 fixed and this is ready to go.
Increase sql_server_version to nvarchar(512) to prevent truncation of long SQL Server version strings
What does this PR do?
Fixes - #712
Updates config.installation_history to increase sql_server_version from nvarchar(255) to nvarchar(512).
Some SQL Server version and edition strings can exceed 255 characters. This change prevents truncation and ensures full version information is retained.
Which component(s) does this affect?
How was this tested?
Tested against SQL Server 2019.
Steps:
Checklist
dotnet build -c Debug)