Fix SQL Server string type mapping to NVARCHAR

[kakins]

Summary

This pull request improves the handling of primary key constraints when altering the type of a primary key column in SQL Server table migrations and rollbacks. The changes ensure that the primary key constraint is dropped before the column type is altered (to avoid SQL errors) and then re-added afterward, both when applying schema updates and when rolling them back. The update also introduces targeted tests to verify the correct order of SQL operations.

This does not modify any default data type mapping, in order to preserve backwards compatibility and avoid forcing unexpected migrations.

The intent here is to facilitate a Wolverine-specific column opt-in override for Saga table PKs, using nvarchar(100) for the data type instead of the implicit varchar(100). The override would remediate a SQL server data type mismatch created by Weasel defaults resulting in significant performance issues (see below) while using Wolverine sagas. The opt in would trigger a SQL migration that drops the PK constraint and recreates it using the new data type.

Motivation

• Current behavior: The SQL Server provider maps .NET string to varchar(100) in DDL, but Weasel’s own command infrastructure (and SQL Server/Ado.NET defaults) send string parameters as NVarChar.
• Problem: This type mismatch causes SQL Server to perform implicit conversions in queries, especially on primary key/index columns (noticed in Wolverine saga tables), which can prevent index seeks and lead to full table scans. This has a considerable and measurable performance impact, especially at scale.
• Evidence: Weasel’s own tests (e.g., in CommandExtensionsTests.cs) assert the parameter type is NVarChar, creating a contradiction with the varchar column mapping. Found in production with a saga table containing hundreds of thousands of rows. Had to scale Azure SQL Server from 4vcores to 18vcores.

Key changes

Primary key constraint handling in migrations and rollbacks:

• Refactored the logic in TableDelta (in TableDelta.cs) to drop the primary key constraint before altering a primary key column's type and to re-add the constraint afterward, both for updates and rollbacks. This is achieved by introducing new helper methods (requiresPrimaryKeyDropBeforeUpdate, requiresPrimaryKeyDropBeforeRollback, writePrimaryKeyUpdates, and writePrimaryKeyRollback). [1] [2] [3] [4] [5]

Expanded and improved tests:

• Added a test (can_patch_existing_primary_key_column_type_change) to ensure that patching a table where the primary key column type is changed works as expected.
• Added a test class (table_delta_sql_ordering) to verify that the generated SQL for updating a primary key column type change drops the constraint before altering the column and re-adds it afterward.
• Added a test class (table_delta_rollback_sql_ordering) to verify that the generated SQL for rolling back a primary key column type change follows the correct order: dropping the constraint, altering the column, and re-adding the constraint.

[jeremydmiller]

Hey, I'm going to pull this one in now for 8.10

[jeremydmiller]

No dice, @kakins would you want to try to adjust this?

[kakins]

Hey, I'm going to pull this one in now for 8.10

@jeremydmiller Sorry I was out of the loop for a few days after I found this. Just now circling back. Yeah I haven't jumped very deep into Weasel yet but I can dig a little further and figure out the test issues.

[kakins]

@jeremydmiller I chewed on this most of yesterday. This may be the least disruptive option. Let me know if you have thoughts/objections. And then the sister PR for Wolverine is linked.