Support making computed columns non-computed (#25446)

Fixes #25445
dotnet · Aug 11, 2021 · 527e8b5 · 527e8b5
1 parent 4f93561
commit 527e8b5
Show file tree

Hide file tree

Showing 4 changed files with 104 additions and 25 deletions.
diff --git a/src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs b/src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs
@@ -243,7 +243,8 @@ protected override void Generate(
             var column = model?.GetRelationalModel().FindTable(operation.Table, operation.Schema)
                 ?.Columns.FirstOrDefault(c => c.Name == operation.Name);
 
-            if (operation.ComputedColumnSql != null)
+            if (operation.ComputedColumnSql != operation.OldColumn.ComputedColumnSql
+                || operation.IsStored != operation.OldColumn.IsStored)
             {
                 var dropColumnOperation = new DropColumnOperation
                 {

diff --git a/test/EFCore.Relational.Specification.Tests/Migrations/MigrationsTestBase.cs b/test/EFCore.Relational.Specification.Tests/Migrations/MigrationsTestBase.cs
@@ -802,6 +802,27 @@ public virtual Task Alter_column_change_computed_type()
                         Assert.True(sumColumn.IsStored);
                 });
 
+        [ConditionalFact]
+        public virtual Task Alter_column_make_non_computed()
+            => Test(
+                builder => builder.Entity(
+                    "People", e =>
+                    {
+                        e.Property<int>("Id");
+                        e.Property<int>("X");
+                        e.Property<int>("Y");
+                    }),
+                builder => builder.Entity("People").Property<int>("Sum")
+                    .HasComputedColumnSql($"{DelimitIdentifier("X")} + {DelimitIdentifier("Y")}"),
+                builder => builder.Entity("People").Property<int>("Sum"),
+                model =>
+                {
+                    var table = Assert.Single(model.Tables);
+                    var sumColumn = Assert.Single(table.Columns, c => c.Name == "Sum");
+                    Assert.Null(sumColumn.ComputedColumnSql);
+                    Assert.NotEqual(true, sumColumn.IsStored);
+                });
+
         [ConditionalFact]
         public virtual Task Alter_column_add_comment()
             => Test(
@@ -815,6 +836,24 @@ public virtual Task Alter_column_add_comment()
                         Assert.Equal("Some comment", column.Comment);
                 });
 
+        [ConditionalFact]
+        public virtual Task Alter_computed_column_add_comment()
+            => Test(
+                builder => builder.Entity("People", x =>
+                {
+                    x.Property<int>("Id");
+                    x.Property<int>("SomeColumn").HasComputedColumnSql("42");
+                }),
+                builder => { },
+                builder => builder.Entity("People").Property<int>("SomeColumn").HasComment("Some comment"),
+                model =>
+                {
+                    var table = Assert.Single(model.Tables);
+                    var column = Assert.Single(table.Columns.Where(c => c.Name == "SomeColumn"));
+                    if (AssertComments)
+                        Assert.Equal("Some comment", column.Comment);
+                });
+
         [ConditionalFact]
         public virtual Task Alter_column_change_comment()
             => Test(

diff --git a/test/EFCore.SqlServer.FunctionalTests/Migrations/MigrationsSqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Migrations/MigrationsSqlServerTest.cs
@@ -629,6 +629,21 @@ FROM [sys].[default_constraints] [d]
 ALTER TABLE [People] ADD [Sum] AS [X] + [Y] PERSISTED;");
         }
 
+        public override async Task Alter_column_make_non_computed()
+        {
+            await base.Alter_column_make_non_computed();
+
+            AssertSql(
+                @"DECLARE @var0 sysname;
+SELECT @var0 = [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'[People]') AND [c].[name] = N'Sum');
+IF @var0 IS NOT NULL EXEC(N'ALTER TABLE [People] DROP CONSTRAINT [' + @var0 + '];');
+ALTER TABLE [People] DROP COLUMN [Sum];
+ALTER TABLE [People] ADD [Sum] int NOT NULL;");
+        }
+
         [ConditionalFact]
         public override async Task Alter_column_add_comment()
         {
@@ -643,33 +658,12 @@ public override async Task Alter_column_add_comment()
         }
 
         [ConditionalFact]
-        public virtual async Task Alter_computed_column_add_comment()
+        public override async Task Alter_computed_column_add_comment()
         {
-            await Test(
-                builder => builder.Entity("People", x =>
-                {
-                    x.Property<int>("Id");
-                    x.Property<int>("SomeColumn").HasComputedColumnSql("42");
-                }),
-                builder => { },
-                builder => builder.Entity("People").Property<int>("SomeColumn").HasComment("Some comment"),
-                model =>
-                {
-                    var table = Assert.Single(model.Tables);
-                    var column = Assert.Single(table.Columns.Where(c => c.Name == "SomeColumn"));
-                    Assert.Equal("Some comment", column.Comment);
-                });
+            await base.Alter_computed_column_add_comment();
 
             AssertSql(
-                @"DECLARE @var0 sysname;
-SELECT @var0 = [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'[People]') AND [c].[name] = N'SomeColumn');
-IF @var0 IS NOT NULL EXEC(N'ALTER TABLE [People] DROP CONSTRAINT [' + @var0 + '];');
-ALTER TABLE [People] DROP COLUMN [SomeColumn];
-ALTER TABLE [People] ADD [SomeColumn] AS 42;
-DECLARE @defaultSchema AS sysname;
+                @"DECLARE @defaultSchema AS sysname;
 SET @defaultSchema = SCHEMA_NAME();
 DECLARE @description AS sql_variant;
 SET @description = N'Some comment';

diff --git a/test/EFCore.Sqlite.FunctionalTests/Migrations/MigrationsSqliteTest.cs b/test/EFCore.Sqlite.FunctionalTests/Migrations/MigrationsSqliteTest.cs
@@ -451,6 +451,26 @@ public override async Task Alter_column_change_computed_type()
                 @"PRAGMA foreign_keys = 1;");
         }
 
+        public override async Task Alter_column_make_non_computed()
+        {
+            await base.Alter_column_make_non_computed();
+
+            AssertSql(
+                @"CREATE TABLE ""ef_temp_People"" (
+    ""Id"" INTEGER NOT NULL,
+    ""Sum"" INTEGER NOT NULL,
+    ""X"" INTEGER NOT NULL,
+    ""Y"" INTEGER NOT NULL
+);",
+                @"INSERT INTO ""ef_temp_People"" (""Id"", ""Sum"", ""X"", ""Y"")
+SELECT ""Id"", ""Sum"", ""X"", ""Y""
+FROM ""People"";",
+                @"PRAGMA foreign_keys = 0;",
+                @"DROP TABLE ""People"";",
+                @"ALTER TABLE ""ef_temp_People"" RENAME TO ""People"";",
+                @"PRAGMA foreign_keys = 1;");
+        }
+
         public override async Task Alter_column_add_comment()
         {
             await base.Alter_column_add_comment();
@@ -469,6 +489,31 @@ public override async Task Alter_column_add_comment()
                 @"PRAGMA foreign_keys = 1;");
         }
 
+        public override async Task Alter_computed_column_add_comment()
+        {
+            await base.Alter_computed_column_add_comment();
+
+            AssertSql(
+                @"CREATE TABLE ""ef_temp_People"" (
+    ""Id"" INTEGER NOT NULL,
+
+    -- Some comment
+    ""SomeColumn"" AS (42)
+);",
+                //
+                @"INSERT INTO ""ef_temp_People"" (""Id"")
+SELECT ""Id""
+FROM ""People"";",
+                //
+                @"PRAGMA foreign_keys = 0;",
+                //
+                @"DROP TABLE ""People"";",
+                //
+                @"ALTER TABLE ""ef_temp_People"" RENAME TO ""People"";",
+                //
+                @"PRAGMA foreign_keys = 1;");
+        }
+
         public override async Task Alter_column_change_comment()
         {
             await base.Alter_column_change_comment();