Skip to content

Commit cdc292a

Browse files
committed
Skip ALTER COLUMN for computed columns when only CLR type changes
The migration model differ produced an AlterColumnOperation when the CLR type of a property mapped to a computed column changed (e.g. int → long for a column with .HasComputedColumnSql("DATALENGTH(...)")). The generated ALTER TABLE ... ALTER COLUMN then failed at runtime with "Cannot alter column ... because it is 'COMPUTED'". A computed column's store type and collation are derived from the expression; they aren't user-configurable. CLR-type-only changes are metadata on the EF side and require no database-side change. SQL Server specifically rejects ALTER COLUMN on computed columns altogether. The fix lives in SqlServerMigrationsSqlGenerator (provider-specific): when both source and target are computed, set alterStatementNeeded to false. This suppresses the ALTER COLUMN emission while letting other separately-emitted facets (notably the comment block at line 488 via sp_addextendedproperty) still apply. The drop+add path for expression changes earlier in the method is unchanged. MigrationsModelDiffer remains provider-independent: it still produces AlterColumnOperation as before; other providers (MySQL with MODIFY COLUMN, PostgreSQL, etc.) decide for themselves how to handle it. Tests: - Unit: AlterColumnOperation_computed_column_with_only_clr_type_change_is_noop asserts the generator produces empty SQL for the bug case. - Unit: AlterColumnOperation_computed_column_with_changed_expression_drops_and_adds guards the existing drop+add path. - Functional: Alter_computed_column_clr_type_only_change_is_noop runs the scenario end-to-end against real SQL Server in the CI matrix; verified locally against Azure SQL Edge on Apple Silicon. Fixes #33425
1 parent a267c3e commit cdc292a

3 files changed

Lines changed: 110 additions & 0 deletions

File tree

src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -357,6 +357,18 @@ protected override void Generate(
357357
|| operation.Collation != operation.OldColumn.Collation
358358
|| HasDifferences(newAnnotations, oldAnnotations);
359359

360+
// SQL Server cannot ALTER COLUMN on a computed column (the type is derived from the
361+
// expression, not user-configurable). When source and target are both computed with the
362+
// same expression and persistence (the drop+add path above didn't trigger), suppress
363+
// the ALTER statement — type/precision/scale/nullability/collation/annotation diffs
364+
// either don't apply or cannot be applied this way; emitting ALTER COLUMN would fail
365+
// with "Cannot alter column ... because it is 'COMPUTED'". Comment changes are handled
366+
// separately via sp_addextendedproperty below and still apply. See #33425.
367+
if (operation.ComputedColumnSql != null && operation.OldColumn.ComputedColumnSql != null)
368+
{
369+
alterStatementNeeded = false;
370+
}
371+
360372
var (oldDefaultValue, oldDefaultValueSql) = (operation.OldColumn.DefaultValue, operation.OldColumn.DefaultValueSql);
361373

362374
if (alterStatementNeeded

test/EFCore.SqlServer.FunctionalTests/Migrations/MigrationsSqlServerTest.cs

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -833,6 +833,37 @@ await Test(
833833
""");
834834
}
835835

836+
[ConditionalFact]
837+
public virtual async Task Alter_computed_column_clr_type_only_change_is_noop()
838+
{
839+
// Regression test for #33425: when only the CLR type of a property mapped to a computed
840+
// column changes (the expression and IsStored are unchanged), SQL Server has nothing to
841+
// alter — the column's type is derived from the expression. The migration must complete
842+
// without emitting `ALTER TABLE ... ALTER COLUMN`, which would fail with
843+
// "Cannot alter column ... because it is 'COMPUTED'".
844+
await Test(
845+
builder => builder.Entity(
846+
"People", x =>
847+
{
848+
x.Property<int>("Id");
849+
x.Property<int>("Calc").HasComputedColumnSql("[Id] * 2");
850+
}),
851+
builder => builder.Entity(
852+
"People", x =>
853+
{
854+
x.Property<int>("Id");
855+
x.Property<long>("Calc").HasComputedColumnSql("[Id] * 2");
856+
}),
857+
model =>
858+
{
859+
var table = Assert.Single(model.Tables);
860+
var column = Assert.Single(table.Columns, c => c.Name == "Calc");
861+
Assert.Equal("([Id]*(2))", column.ComputedColumnSql);
862+
});
863+
864+
AssertSql();
865+
}
866+
836867
public override async Task Add_column_with_required()
837868
{
838869
await base.Add_column_with_required();

test/EFCore.SqlServer.FunctionalTests/Migrations/SqlServerMigrationsSqlGeneratorTest.cs

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -326,6 +326,73 @@ public override void AddForeignKeyOperation_without_principal_columns()
326326
""");
327327
}
328328

329+
[ConditionalFact]
330+
public virtual void AlterColumnOperation_computed_column_with_only_clr_type_change_is_noop()
331+
{
332+
// Regression test for #33425: when the CLR type of a property mapped to a computed column
333+
// changes (e.g. int → long for a column with .HasComputedColumnSql("DATALENGTH(...)")) but
334+
// the expression and IsStored are unchanged, no SQL should be emitted. SQL Server rejects
335+
// ALTER COLUMN on computed columns with "Cannot alter column ... because it is 'COMPUTED'",
336+
// and the underlying database column's type is derived from the expression — there is
337+
// nothing to change.
338+
Generate(
339+
new AlterColumnOperation
340+
{
341+
Table = "Files",
342+
Name = "FileSize",
343+
ClrType = typeof(long),
344+
ColumnType = "bigint",
345+
IsNullable = false,
346+
ComputedColumnSql = "DATALENGTH([FileContents])",
347+
OldColumn = new AddColumnOperation
348+
{
349+
ClrType = typeof(int),
350+
ColumnType = "int",
351+
IsNullable = false,
352+
ComputedColumnSql = "DATALENGTH([FileContents])"
353+
}
354+
});
355+
356+
AssertSql("");
357+
}
358+
359+
[ConditionalFact]
360+
public virtual void AlterColumnOperation_computed_column_with_changed_expression_drops_and_adds()
361+
{
362+
// Regression guard: when the computed column expression itself changes, SQL Server still
363+
// cannot ALTER COLUMN — but a drop+add is required to apply the new expression. This path
364+
// must remain intact.
365+
Generate(
366+
new AlterColumnOperation
367+
{
368+
Table = "Files",
369+
Name = "FileSize",
370+
ClrType = typeof(long),
371+
ColumnType = "bigint",
372+
IsNullable = false,
373+
ComputedColumnSql = "LEN([FileContents])",
374+
OldColumn = new AddColumnOperation
375+
{
376+
ClrType = typeof(int),
377+
ColumnType = "int",
378+
IsNullable = false,
379+
ComputedColumnSql = "DATALENGTH([FileContents])"
380+
}
381+
});
382+
383+
AssertSql(
384+
"""
385+
DECLARE @var nvarchar(max);
386+
SELECT @var = QUOTENAME([d].[name])
387+
FROM [sys].[default_constraints] [d]
388+
INNER JOIN [sys].[columns] [c] ON [d].[parent_column_id] = [c].[column_id] AND [d].[parent_object_id] = [c].[object_id]
389+
WHERE ([d].[parent_object_id] = OBJECT_ID(N'[Files]') AND [c].[name] = N'FileSize');
390+
IF @var IS NOT NULL EXEC(N'ALTER TABLE [Files] DROP CONSTRAINT ' + @var + ';');
391+
ALTER TABLE [Files] DROP COLUMN [FileSize];
392+
ALTER TABLE [Files] ADD [FileSize] AS LEN([FileContents]);
393+
""");
394+
}
395+
329396
[ConditionalFact]
330397
public virtual void AlterColumnOperation_with_identity_legacy()
331398
{

0 commit comments

Comments
 (0)