Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,14 @@ protected override void Generate(

var narrowed = false;
var oldColumnSupported = IsOldColumnSupported(model);
if (oldColumnSupported)

// SQL Server can't ALTER COLUMN on a computed column when the expression is unchanged; see #33425.
var computedColumnIsNoOp = operation.ComputedColumnSql != null
&& operation.OldColumn.ComputedColumnSql != null
&& operation.ComputedColumnSql == operation.OldColumn.ComputedColumnSql
&& operation.IsStored == operation.OldColumn.IsStored;

if (oldColumnSupported && !computedColumnIsNoOp)
{
if (IsIdentity(operation) != IsIdentity(operation.OldColumn))
{
Expand Down Expand Up @@ -357,6 +364,11 @@ protected override void Generate(
|| operation.Collation != operation.OldColumn.Collation
|| HasDifferences(newAnnotations, oldAnnotations);

if (computedColumnIsNoOp)
{
alterStatementNeeded = false;
}

var (oldDefaultValue, oldDefaultValueSql) = (operation.OldColumn.DefaultValue, operation.OldColumn.DefaultValueSql);

if (alterStatementNeeded
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -833,6 +833,71 @@ await Test(
""");
}

[ConditionalFact]
public virtual async Task Alter_computed_column_clr_type_only_change_is_noop()
{
// Regression test for #33425: when only the CLR type of a property mapped to a computed
// column changes (the expression and IsStored are unchanged), SQL Server has nothing to
// alter — the column's type is derived from the expression. The migration must complete
// without emitting `ALTER TABLE ... ALTER COLUMN`, which would fail with
// "Cannot alter column ... because it is 'COMPUTED'".
await Test(
builder => builder.Entity(
"People", x =>
{
x.Property<int>("Id");
x.Property<int>("Calc").HasComputedColumnSql("[Id] * 2");
}),
builder => builder.Entity(
"People", x =>
{
x.Property<int>("Id");
x.Property<long>("Calc").HasComputedColumnSql("[Id] * 2");
}),
model =>
{
var table = Assert.Single(model.Tables);
var column = Assert.Single(table.Columns, c => c.Name == "Calc");
Assert.Equal("([Id]*(2))", column.ComputedColumnSql);
});

AssertSql();
}

[ConditionalFact]
public virtual async Task Alter_computed_column_clr_type_only_change_does_not_rebuild_indexes()
{
// Regression guard for #33425 / Copilot review: a CLR-type-only change on a computed
// column must be a TRUE no-op. The "narrowed" code path can otherwise treat the type
// change as a column-narrowing and drop+recreate every index on the column, even though
// no ALTER COLUMN is emitted. That's needlessly expensive on large tables. Verify no
// DROP/CREATE INDEX SQL appears.
await Test(
builder => builder.Entity(
"People", x =>
{
x.Property<int>("Id");
x.Property<int>("Calc").HasComputedColumnSql("[Id] * 2");
x.HasIndex("Calc");
}),
builder => builder.Entity(
"People", x =>
{
x.Property<int>("Id");
x.Property<long>("Calc").HasComputedColumnSql("[Id] * 2");
x.HasIndex("Calc");
}),
model =>
{
var table = Assert.Single(model.Tables);
var column = Assert.Single(table.Columns, c => c.Name == "Calc");
Assert.Equal("([Id]*(2))", column.ComputedColumnSql);
Assert.Single(table.Indexes);
});

AssertSql();
}

public override async Task Add_column_with_required()
{
await base.Add_column_with_required();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,73 @@ public override void AddForeignKeyOperation_without_principal_columns()
""");
}

[ConditionalFact]
public virtual void AlterColumnOperation_computed_column_with_only_clr_type_change_is_noop()
{
// Regression test for #33425: when the CLR type of a property mapped to a computed column
// changes (e.g. int → long for a column with .HasComputedColumnSql("DATALENGTH(...)")) but
// the expression and IsStored are unchanged, no SQL should be emitted. SQL Server rejects
// ALTER COLUMN on computed columns with "Cannot alter column ... because it is 'COMPUTED'",
// and the underlying database column's type is derived from the expression — there is
// nothing to change.
Generate(
new AlterColumnOperation
{
Table = "Files",
Name = "FileSize",
ClrType = typeof(long),
ColumnType = "bigint",
IsNullable = false,
ComputedColumnSql = "DATALENGTH([FileContents])",
OldColumn = new AddColumnOperation
{
ClrType = typeof(int),
ColumnType = "int",
IsNullable = false,
ComputedColumnSql = "DATALENGTH([FileContents])"
}
});

AssertSql("");
}

[ConditionalFact]
public virtual void AlterColumnOperation_computed_column_with_changed_expression_drops_and_adds()
{
// Regression guard: when the computed column expression itself changes, SQL Server still
// cannot ALTER COLUMN — but a drop+add is required to apply the new expression. This path
// must remain intact.
Generate(
new AlterColumnOperation
{
Table = "Files",
Name = "FileSize",
ClrType = typeof(long),
ColumnType = "bigint",
IsNullable = false,
ComputedColumnSql = "LEN([FileContents])",
OldColumn = new AddColumnOperation
{
ClrType = typeof(int),
ColumnType = "int",
IsNullable = false,
ComputedColumnSql = "DATALENGTH([FileContents])"
}
});

AssertSql(
"""
DECLARE @var nvarchar(max);
SELECT @var = QUOTENAME([d].[name])
FROM [sys].[default_constraints] [d]
INNER JOIN [sys].[columns] [c] ON [d].[parent_column_id] = [c].[column_id] AND [d].[parent_object_id] = [c].[object_id]
WHERE ([d].[parent_object_id] = OBJECT_ID(N'[Files]') AND [c].[name] = N'FileSize');
IF @var IS NOT NULL EXEC(N'ALTER TABLE [Files] DROP CONSTRAINT ' + @var + ';');
ALTER TABLE [Files] DROP COLUMN [FileSize];
ALTER TABLE [Files] ADD [FileSize] AS LEN([FileContents]);
""");
}

[ConditionalFact]
public virtual void AlterColumnOperation_with_identity_legacy()
{
Expand Down
Loading