From 01aba079f0f357ebeae128e527c19688c26bc4f3 Mon Sep 17 00:00:00 2001 From: Brice Lambson Date: Fri, 23 Oct 2020 12:12:00 -0700 Subject: [PATCH] SqlServer RevEng: Stop scaffolding included index columns 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. --- .../Internal/SqlServerDatabaseModelFactory.cs | 9 ----- .../MigrationsSqlServerTest.cs | 34 +++++-------------- .../SqlServerDatabaseModelFactoryTest.cs | 2 +- 3 files changed, 10 insertions(+), 35 deletions(-) diff --git a/src/EFCore.SqlServer/Scaffolding/Internal/SqlServerDatabaseModelFactory.cs b/src/EFCore.SqlServer/Scaffolding/Internal/SqlServerDatabaseModelFactory.cs index 82a578ee6a9..0c02c5a3a74 100644 --- a/src/EFCore.SqlServer/Scaffolding/Internal/SqlServerDatabaseModelFactory.cs +++ b/src/EFCore.SqlServer/Scaffolding/Internal/SqlServerDatabaseModelFactory.cs @@ -1005,8 +1005,6 @@ FROM [sys].[indexes] i index[SqlServerAnnotationNames.FillFactor] = indexGroup.Key.FillFactor; } - var includedColumns = new List(); - foreach (var dataRecord in indexGroup) { var columnName = dataRecord.GetValueOrDefault("column_name"); @@ -1014,8 +1012,6 @@ FROM [sys].[indexes] i var isIncludedColumn = dataRecord.GetValueOrDefault("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 index 264ac48c4b7..b54e54542ab 100644 --- 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)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)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)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)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)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)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)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)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)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 index aaba6ea11a2..b27735e0499 100644 --- 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)index[SqlServerAnnotationNames.Include]); + Assert.Null(index[SqlServerAnnotationNames.Include]); }, "DROP TABLE IncludeIndexTable;"); }