SqlServer RevEng: Stop scaffolding included index columns (#23083)

More people are hitting #22150 than we anticipated. Give that this configuration isn't needed when mapping to an existing column, we've decided to stop scaffolding them entirely until we can fix that particular issue.
dotnet · Oct 24, 2020 · d812063 · d812063
1 parent 73566d1
commit d812063
Show file tree

Hide file tree

Showing 3 changed files with 10 additions and 35 deletions.
diff --git a/src/EFCore.SqlServer/Scaffolding/Internal/SqlServerDatabaseModelFactory.cs b/src/EFCore.SqlServer/Scaffolding/Internal/SqlServerDatabaseModelFactory.cs
@@ -1005,17 +1005,13 @@ FROM [sys].[indexes] i
                         index[SqlServerAnnotationNames.FillFactor] = indexGroup.Key.FillFactor;
                     }
 
-                    var includedColumns = new List<string>();
-
                     foreach (var dataRecord in indexGroup)
                     {
                         var columnName = dataRecord.GetValueOrDefault<string>("column_name");
 
                         var isIncludedColumn = dataRecord.GetValueOrDefault<bool>("is_included_column");
                         if (isIncludedColumn)
                         {
-                            includedColumns.Add(columnName!);
-
                             continue;
                         }
 
@@ -1027,11 +1023,6 @@ FROM [sys].[indexes] i
                         index.Columns.Add(column);
                     }
 
-                    if (includedColumns.Count != 0)
-                    {
-                        index[SqlServerAnnotationNames.Include] = includedColumns.ToArray();
-                    }
-
                     table.Indexes.Add(index);
                 }
             }

diff --git a/test/EFCore.SqlServer.FunctionalTests/MigrationsSqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/MigrationsSqlServerTest.cs
@@ -748,8 +748,7 @@ await Test(
                     Assert.Equal(1, index.Columns.Count);
                     Assert.Contains(table.Columns.Single(c => c.Name == "SomeColumn"), index.Columns);
                     var includedColumns = (IReadOnlyList<string>)index[SqlServerAnnotationNames.Include];
-                    Assert.Equal(1, includedColumns.Count);
-                    Assert.Contains("SomeOtherColumn", includedColumns);
+                    Assert.Null(includedColumns);
                 });
 
             AssertSql(
@@ -853,8 +852,7 @@ await Test(
                     Assert.Contains(table.Columns.Single(c => c.Name == "FirstName"), index.Columns);
                     Assert.Contains(table.Columns.Single(c => c.Name == "LastName"), index.Columns);
                     var includedColumns = (IReadOnlyList<string>)index[SqlServerAnnotationNames.Include];
-                    Assert.Equal(1, includedColumns.Count);
-                    Assert.Contains("Name", includedColumns);
+                    Assert.Null(includedColumns);
                 });
 
             AssertSql(
@@ -1135,9 +1133,7 @@ await Test(
                     Assert.Equal(1, index.Columns.Count);
                     Assert.Contains(table.Columns.Single(c => c.Name == "Name"), index.Columns);
                     var includedColumns = (IReadOnlyList<string>)index[SqlServerAnnotationNames.Include];
-                    Assert.Equal(2, includedColumns.Count);
-                    Assert.Contains("FirstName", includedColumns);
-                    Assert.Contains("LastName", includedColumns);
+                    Assert.Null(includedColumns);
                 });
 
             AssertSql(
@@ -1176,9 +1172,7 @@ await Test(
                     Assert.Equal(1, index.Columns.Count);
                     Assert.Contains(table.Columns.Single(c => c.Name == "Name"), index.Columns);
                     var includedColumns = (IReadOnlyList<string>)index[SqlServerAnnotationNames.Include];
-                    Assert.Equal(2, includedColumns.Count);
-                    Assert.Contains("FirstName", includedColumns);
-                    Assert.Contains("LastName", includedColumns);
+                    Assert.Null(includedColumns);
                 });
 
             AssertSql(
@@ -1217,9 +1211,7 @@ await Test(
                     Assert.Equal(1, index.Columns.Count);
                     Assert.Contains(table.Columns.Single(c => c.Name == "Name"), index.Columns);
                     var includedColumns = (IReadOnlyList<string>)index[SqlServerAnnotationNames.Include];
-                    Assert.Equal(2, includedColumns.Count);
-                    Assert.Contains("FirstName", includedColumns);
-                    Assert.Contains("LastName", includedColumns);
+                    Assert.Null(includedColumns);
                 });
 
             AssertSql(
@@ -1260,9 +1252,7 @@ await Test(
                     Assert.Equal(1, index.Columns.Count);
                     Assert.Contains(table.Columns.Single(c => c.Name == "Name"), index.Columns);
                     var includedColumns = (IReadOnlyList<string>)index[SqlServerAnnotationNames.Include];
-                    Assert.Equal(2, includedColumns.Count);
-                    Assert.Contains("FirstName", includedColumns);
-                    Assert.Contains("LastName", includedColumns);
+                    Assert.Null(includedColumns);
                 });
 
             AssertSql(
@@ -1305,9 +1295,7 @@ await Test(
                     Assert.Equal(1, index.Columns.Count);
                     Assert.Contains(table.Columns.Single(c => c.Name == "Name"), index.Columns);
                     var includedColumns = (IReadOnlyList<string>)index[SqlServerAnnotationNames.Include];
-                    Assert.Equal(2, includedColumns.Count);
-                    Assert.Contains("FirstName", includedColumns);
-                    Assert.Contains("LastName", includedColumns);
+                    Assert.Null(includedColumns);
                     // TODO: Online index not scaffolded?
                 });
 
@@ -1352,9 +1340,7 @@ await Test(
                     Assert.Equal(1, index.Columns.Count);
                     Assert.Contains(table.Columns.Single(c => c.Name == "Name"), index.Columns);
                     var includedColumns = (IReadOnlyList<string>)index[SqlServerAnnotationNames.Include];
-                    Assert.Equal(2, includedColumns.Count);
-                    Assert.Contains("FirstName", includedColumns);
-                    Assert.Contains("LastName", includedColumns);
+                    Assert.Null(includedColumns);
                     // TODO: Online index not scaffolded?
                 });
 
@@ -1397,9 +1383,7 @@ await Test(
                     Assert.Equal(1, index.Columns.Count);
                     Assert.Contains(table.Columns.Single(c => c.Name == "Name"), index.Columns);
                     var includedColumns = (IReadOnlyList<string>)index[SqlServerAnnotationNames.Include];
-                    Assert.Equal(2, includedColumns.Count);
-                    Assert.Contains("FirstName", includedColumns);
-                    Assert.Contains("LastName", includedColumns);
+                    Assert.Null(includedColumns);
                     // TODO: Online index not scaffolded?
                 });
 

diff --git a/test/EFCore.SqlServer.FunctionalTests/Scaffolding/SqlServerDatabaseModelFactoryTest.cs b/test/EFCore.SqlServer.FunctionalTests/Scaffolding/SqlServerDatabaseModelFactoryTest.cs
@@ -1929,7 +1929,7 @@ IncludeProperty int
                 {
                     var index = Assert.Single(dbModel.Tables.Single().Indexes);
                     Assert.Equal(new[] { "IndexProperty" }, index.Columns.Select(ic => ic.Name).ToList());
-                    Assert.Equal(new[] { "IncludeProperty" }, (IReadOnlyList<string>)index[SqlServerAnnotationNames.Include]);
+                    Assert.Null(index[SqlServerAnnotationNames.Include]);
                 },
                 "DROP TABLE IncludeIndexTable;");
         }