Skip to content

Skip ALTER COLUMN for computed columns when only CLR type changes#38252

Open
m-x-shokhzod wants to merge 2 commits into
dotnet:mainfrom
m-x-shokhzod:fix/migrations-computed-column-clr-type-change
Open

Skip ALTER COLUMN for computed columns when only CLR type changes#38252
m-x-shokhzod wants to merge 2 commits into
dotnet:mainfrom
m-x-shokhzod:fix/migrations-computed-column-clr-type-change

Conversation

@m-x-shokhzod
Copy link
Copy Markdown
Contributor

@m-x-shokhzod m-x-shokhzod commented May 10, 2026

Migrations for properties mapped to a computed column failed at runtime when only the CLR type changed (e.g. intlong for a column with .HasComputedColumnSql("DATALENGTH(...)")). EF emitted ALTER TABLE ... ALTER COLUMN, which SQL Server rejected with:

Cannot alter column 'FileSize' because it is 'COMPUTED'.

A computed column's store type is derived from the expression, so a CLR-type-only change is metadata on the EF side and requires no database-side change.

Fix

In SqlServerMigrationsSqlGenerator.Generate(AlterColumnOperation), detect the no-op case once and use it to short-circuit both the index drop/recreate path and the ALTER COLUMN emission:

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

// 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)
{
    // ... narrowed = ... (skipped when computed is unchanged)
}

// later
if (computedColumnIsNoOp)
{
    alterStatementNeeded = false;
}

The flag is computed once and gates two sites:

  1. The narrowed block at the top — prevents GetIndexesToRebuild / DropIndexes / CreateIndexes from running for a column the DB won't see modified.
  2. The alterStatementNeeded reset — suppresses the actual ALTER COLUMN SQL.

The provider-independent MigrationsModelDiffer is untouched — the constraint is SQL Server-specific (PostgreSQL/SQLite generated-column rules differ, MySQL allows MODIFY COLUMN on generated columns), so the suppression belongs in the SQL Server generator.

Comment changes via sp_addextendedproperty further down still run — this is intentional and covered by the existing Alter_computed_column_add_comment regression test.

Behavior

Scenario Before After
Computed column, CLR type changes, same expression ALTER COLUMN → SQL Server rejects No SQL emitted, no index rebuild
Computed column, expression changes Drop+add path Unchanged
Non-computed column, type changes ALTER COLUMN Unchanged
Computed column, comment changes sp_addextendedproperty Unchanged

Tests

Generator-level (SqlServerMigrationsSqlGeneratorTest):

  • AlterColumnOperation_computed_column_with_only_clr_type_change_is_noop — asserts empty SQL for the bug case
  • AlterColumnOperation_computed_column_with_changed_expression_drops_and_adds — guards the existing drop+add path

End-to-end (MigrationsSqlServerTest):

  • Alter_computed_column_clr_type_only_change_is_noop — full migration against real SQL Server, zero SQL
  • Alter_computed_column_clr_type_only_change_does_not_rebuild_indexes — same scenario with an index on the computed column, asserts no DROP INDEX / CREATE INDEX (would have caught the original Copilot-flagged regression)

CI matrix (SqlServer 2019/2022/2025) covers end-to-end execution.

Fixes #33425

@m-x-shokhzod m-x-shokhzod requested a review from a team as a code owner May 10, 2026 05:43
@roji
Copy link
Copy Markdown
Member

roji commented May 10, 2026

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.

Have you verified whether this is a SQL Server-specific thing, or universal to all databases? I'm pretty sure it's the former, in which case you're introducing a SQL Server-specific change into ModelDiffer, which is provider-independent.

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 dotnet#33425
@m-x-shokhzod m-x-shokhzod force-pushed the fix/migrations-computed-column-clr-type-change branch from ad662f8 to cdc292a Compare May 11, 2026 00:17
@m-x-shokhzod
Copy link
Copy Markdown
Contributor Author

m-x-shokhzod commented May 11, 2026

Good point — you're right. The constraint is SQL Server specific: PostgreSQL and SQLite have similar but narrower restrictions on generated columns, and MySQL handles type changes on generated columns fine via MODIFY COLUMN.

Reworked (force-pushed cdc292aff8):

  • MigrationsModelDiffer.cs reverted to upstream main — provider-independent again.
  • The fix now lives in SqlServerMigrationsSqlGenerator.Generate(AlterColumnOperation): when both source and target are computed, alterStatementNeeded is set to false. This suppresses only the ALTER COLUMN emission; the comment block via sp_addextendedproperty further down still runs (which the unit test approach in my first version missed — caught by the Alter_computed_column_add_comment regression).
  • Added an end-to-end test Alter_computed_column_clr_type_only_change_is_noop in MigrationsSqlServerTest that exercises the customer-reported scenario end-to-end. Verified locally against real SQL Server (Azure SQL Edge on Apple Silicon) — passes. Will also run on the SqlServer 2019/2022/2025 CI matrix.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses SQL Server migrations failing when EF scaffolds an ALTER COLUMN for a computed column whose CLR type changed (e.g. intlong) while the computed expression is unchanged. Since SQL Server rejects ALTER COLUMN on computed columns, the generator is updated to avoid emitting that SQL and new regression tests are added.

Changes:

  • Suppress ALTER TABLE ... ALTER COLUMN SQL generation for computed columns when both old/new are computed (leaving expression-change behavior intact via the existing drop+add path).
  • Add SQL generator regression coverage for “CLR type only” changes (no SQL) vs expression changes (drop+add).
  • Add end-to-end migrations functional coverage ensuring the migration completes without emitting SQL for the CLR-type-only scenario.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs Avoids emitting ALTER COLUMN for computed columns (when both old/new are computed).
test/EFCore.SqlServer.FunctionalTests/Migrations/SqlServerMigrationsSqlGeneratorTest.cs Adds generator-level regression tests for computed columns (no-op vs drop+add).
test/EFCore.SqlServer.FunctionalTests/Migrations/MigrationsSqlServerTest.cs Adds functional regression test ensuring no SQL is emitted for CLR-type-only computed column changes.

Comment on lines +360 to +370
// SQL Server cannot ALTER COLUMN on a computed column (the type is derived from the
// expression, not user-configurable). When source and target are both computed with the
// same expression and persistence (the drop+add path above didn't trigger), suppress
// the ALTER statement — type/precision/scale/nullability/collation/annotation diffs
// either don't apply or cannot be applied this way; emitting ALTER COLUMN would fail
// with "Cannot alter column ... because it is 'COMPUTED'". Comment changes are handled
// separately via sp_addextendedproperty below and still apply. See #33425.
if (operation.ComputedColumnSql != null && operation.OldColumn.ComputedColumnSql != null)
{
alterStatementNeeded = false;
}
Comment on lines +360 to +366
// SQL Server cannot ALTER COLUMN on a computed column (the type is derived from the
// expression, not user-configurable). When source and target are both computed with the
// same expression and persistence (the drop+add path above didn't trigger), suppress
// the ALTER statement — type/precision/scale/nullability/collation/annotation diffs
// either don't apply or cannot be applied this way; emitting ALTER COLUMN would fail
// with "Cannot alter column ... because it is 'COMPUTED'". Comment changes are handled
// separately via sp_addextendedproperty below and still apply. See #33425.
Addresses Copilot review on dotnet#38252: the previous fix suppressed
`ALTER COLUMN` for CLR-type-only changes on computed columns, but the
`narrowed` path still dropped and recreated every index on the column
because `columnType != oldType`. Hoist the computed-column-no-op
detection earlier and reuse it to gate both the index rebuild and the
`ALTER COLUMN` emission.

Add an end-to-end regression test that creates an index on the computed
column, changes only the CLR type, and asserts no SQL is emitted —
would have caught the original issue.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Migration tries to alter column type of computed column if CLR type changes.

4 participants