From ea042f90ae78a2f06b4bd58243da928ceee47909 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 27 Aug 2025 01:26:35 +0000 Subject: [PATCH 1/7] Initial plan From 81b6abc5eb75b663ca192163cd4181c984eb7b81 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 27 Aug 2025 01:50:33 +0000 Subject: [PATCH 2/7] Fix null reference exception in migrations for invalid store types Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com> --- .../Migrations/MigrationsSqlGenerator.cs | 8 +- .../InvalidColumnTypeIntegrationTest.cs | 71 ++++++++++++++++ .../SqlServerMigrationsSqlGeneratorTest.cs | 80 +++++++++++++++++++ 3 files changed, 158 insertions(+), 1 deletion(-) create mode 100644 test/EFCore.SqlServer.Tests/Migrations/InvalidColumnTypeIntegrationTest.cs create mode 100644 test/EFCore.SqlServer.Tests/Migrations/SqlServerMigrationsSqlGeneratorTest.cs diff --git a/src/EFCore.Relational/Migrations/MigrationsSqlGenerator.cs b/src/EFCore.Relational/Migrations/MigrationsSqlGenerator.cs index b8c70fc1195..e72c03a282a 100644 --- a/src/EFCore.Relational/Migrations/MigrationsSqlGenerator.cs +++ b/src/EFCore.Relational/Migrations/MigrationsSqlGenerator.cs @@ -1356,7 +1356,13 @@ protected virtual void ColumnDefinition( return; } - var columnType = operation.ColumnType ?? GetColumnType(schema, table, name, operation, model)!; + var columnType = operation.ColumnType ?? GetColumnType(schema, table, name, operation, model); + if (columnType == null) + { + throw new InvalidOperationException( + RelationalStrings.UnsupportedType(operation.ClrType?.Name ?? "unknown")); + } + builder .Append(Dependencies.SqlGenerationHelper.DelimitIdentifier(name)) .Append(" ") diff --git a/test/EFCore.SqlServer.Tests/Migrations/InvalidColumnTypeIntegrationTest.cs b/test/EFCore.SqlServer.Tests/Migrations/InvalidColumnTypeIntegrationTest.cs new file mode 100644 index 00000000000..7da848e7f3a --- /dev/null +++ b/test/EFCore.SqlServer.Tests/Migrations/InvalidColumnTypeIntegrationTest.cs @@ -0,0 +1,71 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Microsoft.EntityFrameworkCore.TestUtilities; +using System.ComponentModel.DataAnnotations.Schema; + +namespace Microsoft.EntityFrameworkCore.SqlServer.Tests.Migrations; + +/// +/// Integration test that simulates the user's original issue scenario more closely +/// +public class InvalidColumnTypeIntegrationTest +{ + [ConditionalFact] + public void Creating_migration_with_invalid_column_type_should_not_throw_null_reference() + { + var options = new DbContextOptionsBuilder() + .UseSqlServer("Server=(localdb)\\mssqllocaldb;Database=TestInvalidColumnType;Trusted_Connection=true;MultipleActiveResultSets=true") + .Options; + + using var context = new InvalidColumnTypeContext(options); + + // This simulates the scenario where a user has invalid Column attributes + // Before the fix, this would throw a NullReferenceException + // After the fix, this should throw a more helpful InvalidOperationException + var ex = Assert.ThrowsAny(() => + { + context.Database.EnsureDeleted(); + context.Database.EnsureCreated(); + }); + + // We should NOT get a NullReferenceException + Assert.IsNotType(ex); + + // We should get some form of meaningful error instead + Assert.True(ex is InvalidOperationException || ex is ArgumentException || ex is NotSupportedException); + } + + public class InvalidColumnTypeContext : DbContext + { + public InvalidColumnTypeContext(DbContextOptions options) : base(options) + { + } + + public DbSet People { get; set; } + public DbSet Contracts { get; set; } + } + + public class Person + { + public int Id { get; set; } + + [Column(TypeName = "decimal(18,2)")] // Invalid for string - this is what the user had + public string FirstName { get; set; } = string.Empty; + + [Column(TypeName = "decimal(18,2)")] // Invalid for string - this is what the user had + public string LastName { get; set; } = string.Empty; + + public int Age { get; set; } + public List Contracts { get; set; } = new(); + } + + public class Contract + { + public int Id { get; set; } + public string ContractLabel { get; set; } = string.Empty; + public DateTime DateSign { get; set; } = DateTime.Now; + public int? PersonId { get; set; } + public Person Person { get; set; } + } +} \ No newline at end of file diff --git a/test/EFCore.SqlServer.Tests/Migrations/SqlServerMigrationsSqlGeneratorTest.cs b/test/EFCore.SqlServer.Tests/Migrations/SqlServerMigrationsSqlGeneratorTest.cs new file mode 100644 index 00000000000..02b5ae54637 --- /dev/null +++ b/test/EFCore.SqlServer.Tests/Migrations/SqlServerMigrationsSqlGeneratorTest.cs @@ -0,0 +1,80 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Microsoft.EntityFrameworkCore.Migrations.Operations; + +namespace Microsoft.EntityFrameworkCore.SqlServer.Tests.Migrations; + +public class SqlServerMigrationsSqlGeneratorNullReferenceTest : MigrationsSqlGeneratorTestBase +{ + public SqlServerMigrationsSqlGeneratorNullReferenceTest() + : base(SqlServerTestHelpers.Instance) + { + } + + protected override string GetGeometryCollectionStoreType() + => "geography"; + + [ConditionalFact] + public void CreateTableOperation_with_invalid_string_column_store_type_should_not_throw_null_reference() + { + // This should NOT throw a null reference exception when an invalid store type is specified for a string column + Generate( + new CreateTableOperation + { + Name = "People", + Columns = + { + new AddColumnOperation + { + Name = "FirstName", + Table = "People", + ClrType = typeof(string), + ColumnType = "decimal(18,2)", // Invalid store type for string + IsNullable = false + } + } + }); + + // Let's see what SQL is generated + Assert.NotEmpty(Sql); + // If we reach here, no null reference exception was thrown, which is correct + } + + [ConditionalFact] + public void Reproduce_null_reference_with_invalid_column_type_during_migration_add() + { + // This test directly tests the fix by showing that + // when GetColumnType would return null, we now get a proper exception + // instead of a null reference exception + + var operation = new AddColumnOperation + { + Name = "TestColumn", + Table = "TestTable", + ClrType = typeof(System.IO.FileStream), // This should definitely not have a mapping + ColumnType = null, // Force it to call GetColumnType + IsNullable = false + }; + + // Let's see what SQL gets generated instead of what we expect + // This test validates that we don't get a NullReferenceException anymore + try + { + Generate(operation); + // If we get here without exception, the type mapping source found something unexpected + Assert.NotNull(Sql); + } + catch (NullReferenceException) + { + // This should NOT happen anymore with our fix + Assert.Fail("NullReferenceException should not be thrown - this indicates the fix didn't work"); + } + catch (InvalidOperationException ex) + { + // This is the expected behavior with our fix + Assert.Contains("type mapping", ex.Message); + } + // Any other exception is also acceptable as long as it's not NullReferenceException + } +} \ No newline at end of file From fee12168d1ed21ac3d2a481e5de150e19a2abc3f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 27 Aug 2025 20:56:18 +0000 Subject: [PATCH 3/7] Changes before error encountered Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com> --- .../Migrations/MigrationsSqlGenerator.cs | 29 ++++++-- .../Properties/RelationalStrings.Designer.cs | 8 +++ .../Properties/RelationalStrings.resx | 3 + .../SqlServerMigrationsSqlGenerator.cs | 2 +- .../SqliteMigrationsSqlGenerator.cs | 2 +- .../SqlServerMigrationsSqlGeneratorTest.cs | 63 ++++++++++++++++ .../InvalidColumnTypeIntegrationTest.cs | 71 ------------------- .../SqlServerMigrationsSqlGeneratorTest.cs | 63 ---------------- 8 files changed, 98 insertions(+), 143 deletions(-) delete mode 100644 test/EFCore.SqlServer.Tests/Migrations/InvalidColumnTypeIntegrationTest.cs diff --git a/src/EFCore.Relational/Migrations/MigrationsSqlGenerator.cs b/src/EFCore.Relational/Migrations/MigrationsSqlGenerator.cs index e72c03a282a..c4cfa1db4cc 100644 --- a/src/EFCore.Relational/Migrations/MigrationsSqlGenerator.cs +++ b/src/EFCore.Relational/Migrations/MigrationsSqlGenerator.cs @@ -1357,11 +1357,6 @@ protected virtual void ColumnDefinition( } var columnType = operation.ColumnType ?? GetColumnType(schema, table, name, operation, model); - if (columnType == null) - { - throw new InvalidOperationException( - RelationalStrings.UnsupportedType(operation.ClrType?.Name ?? "unknown")); - } builder .Append(Dependencies.SqlGenerationHelper.DelimitIdentifier(name)) @@ -1407,7 +1402,7 @@ protected virtual void ComputedColumnDefinition( /// The column metadata. /// The target model which may be if the operations exist without a model. /// The database/store type for the column. - protected virtual string? GetColumnType( + protected virtual string GetColumnType( string? schema, string tableName, string name, @@ -1435,7 +1430,7 @@ protected virtual void ComputedColumnDefinition( || table.Indexes.Any(u => u.Columns.Contains(column)); } - return Dependencies.TypeMappingSource.FindMapping( + var storeType = Dependencies.TypeMappingSource.FindMapping( operation.ClrType, null, keyOrIndex, @@ -1446,6 +1441,26 @@ protected virtual void ComputedColumnDefinition( operation.Precision, operation.Scale) ?.StoreType; + + if (storeType != null) + { + return storeType; + } + + // Try getting the type from the default value if available + if (operation.DefaultValue != null) + { + var defaultValueMapping = Dependencies.TypeMappingSource.GetMappingForValue(operation.DefaultValue); + if (defaultValueMapping?.StoreType != null) + { + return defaultValueMapping.StoreType; + } + } + + // If no mapping found, throw with detailed information + var fullTableName = schema != null ? $"{schema}.{tableName}" : tableName; + throw new InvalidOperationException( + RelationalStrings.UnsupportedTypeForColumn(fullTableName, name, operation.ClrType?.Name ?? "unknown")); } /// diff --git a/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs b/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs index 1248246cde2..c482e874ef2 100644 --- a/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs +++ b/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs @@ -2241,6 +2241,14 @@ public static string UnsupportedType(object? clrType) GetString("UnsupportedType", nameof(clrType)), clrType); + /// + /// Unable to find a store type mapping for column '{table}.{column}' with CLR type '{clrType}'. + /// + public static string UnsupportedTypeForColumn(object? table, object? column, object? clrType) + => string.Format( + GetString("UnsupportedTypeForColumn", nameof(table), nameof(column), nameof(clrType)), + table, column, clrType); + /// /// The database operation was expected to affect {expectedRows} row(s), but actually affected {actualRows} row(s); data may have been modified or deleted since entities were loaded. See https://go.microsoft.com/fwlink/?LinkId=527962 for information on understanding and handling optimistic concurrency exceptions. /// diff --git a/src/EFCore.Relational/Properties/RelationalStrings.resx b/src/EFCore.Relational/Properties/RelationalStrings.resx index c600ef49f15..6c91cc2ebeb 100644 --- a/src/EFCore.Relational/Properties/RelationalStrings.resx +++ b/src/EFCore.Relational/Properties/RelationalStrings.resx @@ -1306,6 +1306,9 @@ The current provider doesn't have a store type mapping for properties of type '{clrType}'. + + Unable to find a store type mapping for column '{table}.{column}' with CLR type '{clrType}'. + The database operation was expected to affect {expectedRows} row(s), but actually affected {actualRows} row(s); data may have been modified or deleted since entities were loaded. See https://go.microsoft.com/fwlink/?LinkId=527962 for information on understanding and handling optimistic concurrency exceptions. diff --git a/src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs b/src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs index d57bfa6b187..7d004a0b050 100644 --- a/src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs +++ b/src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs @@ -1640,7 +1640,7 @@ protected override void ColumnDefinition( return; } - var columnType = operation.ColumnType ?? GetColumnType(schema, table, name, operation, model)!; + var columnType = operation.ColumnType ?? GetColumnType(schema, table, name, operation, model); builder .Append(Dependencies.SqlGenerationHelper.DelimitIdentifier(name)) .Append(" ") diff --git a/src/EFCore.Sqlite.Core/Migrations/SqliteMigrationsSqlGenerator.cs b/src/EFCore.Sqlite.Core/Migrations/SqliteMigrationsSqlGenerator.cs index d1d6e7e23ff..e43c0a88572 100644 --- a/src/EFCore.Sqlite.Core/Migrations/SqliteMigrationsSqlGenerator.cs +++ b/src/EFCore.Sqlite.Core/Migrations/SqliteMigrationsSqlGenerator.cs @@ -58,7 +58,7 @@ private bool IsSpatialiteColumn(AddColumnOperation operation, IModel? model) operation.Table, operation.Name, operation, - model)!); + model)); private IReadOnlyList RewriteOperations( IReadOnlyList migrationOperations, diff --git a/test/EFCore.SqlServer.FunctionalTests/Migrations/SqlServerMigrationsSqlGeneratorTest.cs b/test/EFCore.SqlServer.FunctionalTests/Migrations/SqlServerMigrationsSqlGeneratorTest.cs index f8d9cb99007..e15d3ee9401 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Migrations/SqlServerMigrationsSqlGeneratorTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Migrations/SqlServerMigrationsSqlGeneratorTest.cs @@ -1267,6 +1267,69 @@ FROM [sys].[default_constraints] [d] """); } + [ConditionalFact] + public void Invalid_column_type_throws_meaningful_exception() + { + var ex = Assert.Throws(() => + Generate( + new CreateTableOperation + { + Name = "People", + Columns = + { + new AddColumnOperation + { + Name = "FirstName", + Table = "People", + ClrType = typeof(string), + ColumnType = "decimal(18,2)", // Invalid store type for string + IsNullable = false + } + } + })); + + Assert.Contains("People.FirstName", ex.Message); + Assert.Contains("String", ex.Message); + } + + [ConditionalFact] + public void Invalid_column_type_for_unmappable_clr_type_throws_meaningful_exception() + { + var ex = Assert.Throws(() => + Generate( + new AddColumnOperation + { + Name = "TestColumn", + Table = "TestTable", + ClrType = typeof(System.IO.FileStream), // Unmappable CLR type + ColumnType = null, + IsNullable = false + })); + + Assert.Contains("TestTable.TestColumn", ex.Message); + Assert.Contains("FileStream", ex.Message); + } + + [ConditionalFact] + public void Column_type_found_from_default_value_when_clr_type_unmappable() + { + Generate( + new AddColumnOperation + { + Name = "TestColumn", + Table = "TestTable", + ClrType = typeof(System.IO.FileStream), // Unmappable CLR type + ColumnType = null, + DefaultValue = "test string", // String default value should give us nvarchar mapping + IsNullable = false + }); + + AssertSql( + """ +ALTER TABLE [TestTable] ADD [TestColumn] nvarchar(max) NOT NULL DEFAULT N'test string'; +"""); + } + private static void CreateGotModel(ModelBuilder b) => b.HasDefaultSchema("dbo").Entity( "Person", pb => diff --git a/test/EFCore.SqlServer.Tests/Migrations/InvalidColumnTypeIntegrationTest.cs b/test/EFCore.SqlServer.Tests/Migrations/InvalidColumnTypeIntegrationTest.cs deleted file mode 100644 index 7da848e7f3a..00000000000 --- a/test/EFCore.SqlServer.Tests/Migrations/InvalidColumnTypeIntegrationTest.cs +++ /dev/null @@ -1,71 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using Microsoft.EntityFrameworkCore.TestUtilities; -using System.ComponentModel.DataAnnotations.Schema; - -namespace Microsoft.EntityFrameworkCore.SqlServer.Tests.Migrations; - -/// -/// Integration test that simulates the user's original issue scenario more closely -/// -public class InvalidColumnTypeIntegrationTest -{ - [ConditionalFact] - public void Creating_migration_with_invalid_column_type_should_not_throw_null_reference() - { - var options = new DbContextOptionsBuilder() - .UseSqlServer("Server=(localdb)\\mssqllocaldb;Database=TestInvalidColumnType;Trusted_Connection=true;MultipleActiveResultSets=true") - .Options; - - using var context = new InvalidColumnTypeContext(options); - - // This simulates the scenario where a user has invalid Column attributes - // Before the fix, this would throw a NullReferenceException - // After the fix, this should throw a more helpful InvalidOperationException - var ex = Assert.ThrowsAny(() => - { - context.Database.EnsureDeleted(); - context.Database.EnsureCreated(); - }); - - // We should NOT get a NullReferenceException - Assert.IsNotType(ex); - - // We should get some form of meaningful error instead - Assert.True(ex is InvalidOperationException || ex is ArgumentException || ex is NotSupportedException); - } - - public class InvalidColumnTypeContext : DbContext - { - public InvalidColumnTypeContext(DbContextOptions options) : base(options) - { - } - - public DbSet People { get; set; } - public DbSet Contracts { get; set; } - } - - public class Person - { - public int Id { get; set; } - - [Column(TypeName = "decimal(18,2)")] // Invalid for string - this is what the user had - public string FirstName { get; set; } = string.Empty; - - [Column(TypeName = "decimal(18,2)")] // Invalid for string - this is what the user had - public string LastName { get; set; } = string.Empty; - - public int Age { get; set; } - public List Contracts { get; set; } = new(); - } - - public class Contract - { - public int Id { get; set; } - public string ContractLabel { get; set; } = string.Empty; - public DateTime DateSign { get; set; } = DateTime.Now; - public int? PersonId { get; set; } - public Person Person { get; set; } - } -} \ No newline at end of file diff --git a/test/EFCore.SqlServer.Tests/Migrations/SqlServerMigrationsSqlGeneratorTest.cs b/test/EFCore.SqlServer.Tests/Migrations/SqlServerMigrationsSqlGeneratorTest.cs index 02b5ae54637..6117ebaecc0 100644 --- a/test/EFCore.SqlServer.Tests/Migrations/SqlServerMigrationsSqlGeneratorTest.cs +++ b/test/EFCore.SqlServer.Tests/Migrations/SqlServerMigrationsSqlGeneratorTest.cs @@ -14,67 +14,4 @@ public SqlServerMigrationsSqlGeneratorNullReferenceTest() protected override string GetGeometryCollectionStoreType() => "geography"; - - [ConditionalFact] - public void CreateTableOperation_with_invalid_string_column_store_type_should_not_throw_null_reference() - { - // This should NOT throw a null reference exception when an invalid store type is specified for a string column - Generate( - new CreateTableOperation - { - Name = "People", - Columns = - { - new AddColumnOperation - { - Name = "FirstName", - Table = "People", - ClrType = typeof(string), - ColumnType = "decimal(18,2)", // Invalid store type for string - IsNullable = false - } - } - }); - - // Let's see what SQL is generated - Assert.NotEmpty(Sql); - // If we reach here, no null reference exception was thrown, which is correct - } - - [ConditionalFact] - public void Reproduce_null_reference_with_invalid_column_type_during_migration_add() - { - // This test directly tests the fix by showing that - // when GetColumnType would return null, we now get a proper exception - // instead of a null reference exception - - var operation = new AddColumnOperation - { - Name = "TestColumn", - Table = "TestTable", - ClrType = typeof(System.IO.FileStream), // This should definitely not have a mapping - ColumnType = null, // Force it to call GetColumnType - IsNullable = false - }; - - // Let's see what SQL gets generated instead of what we expect - // This test validates that we don't get a NullReferenceException anymore - try - { - Generate(operation); - // If we get here without exception, the type mapping source found something unexpected - Assert.NotNull(Sql); - } - catch (NullReferenceException) - { - // This should NOT happen anymore with our fix - Assert.Fail("NullReferenceException should not be thrown - this indicates the fix didn't work"); - } - catch (InvalidOperationException ex) - { - // This is the expected behavior with our fix - Assert.Contains("type mapping", ex.Message); - } - // Any other exception is also acceptable as long as it's not NullReferenceException - } } \ No newline at end of file From cd9ba810101ef7711842dca6e1b47857360bf74f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 27 Aug 2025 22:16:38 +0000 Subject: [PATCH 4/7] Remove empty test class and finalize implementation Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com> --- .../SqlServerMigrationsSqlGeneratorTest.cs | 17 ----------------- 1 file changed, 17 deletions(-) delete mode 100644 test/EFCore.SqlServer.Tests/Migrations/SqlServerMigrationsSqlGeneratorTest.cs diff --git a/test/EFCore.SqlServer.Tests/Migrations/SqlServerMigrationsSqlGeneratorTest.cs b/test/EFCore.SqlServer.Tests/Migrations/SqlServerMigrationsSqlGeneratorTest.cs deleted file mode 100644 index 6117ebaecc0..00000000000 --- a/test/EFCore.SqlServer.Tests/Migrations/SqlServerMigrationsSqlGeneratorTest.cs +++ /dev/null @@ -1,17 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using Microsoft.EntityFrameworkCore.Migrations.Operations; - -namespace Microsoft.EntityFrameworkCore.SqlServer.Tests.Migrations; - -public class SqlServerMigrationsSqlGeneratorNullReferenceTest : MigrationsSqlGeneratorTestBase -{ - public SqlServerMigrationsSqlGeneratorNullReferenceTest() - : base(SqlServerTestHelpers.Instance) - { - } - - protected override string GetGeometryCollectionStoreType() - => "geography"; -} \ No newline at end of file From 0ba8316f960f1d7b2c7f790204d862330877f4eb Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 27 Aug 2025 23:25:17 +0000 Subject: [PATCH 5/7] Update tests to verify exception message equals RelationalStrings.UnsupportedTypeForColumn Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com> --- .../Migrations/SqlServerMigrationsSqlGeneratorTest.cs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/test/EFCore.SqlServer.FunctionalTests/Migrations/SqlServerMigrationsSqlGeneratorTest.cs b/test/EFCore.SqlServer.FunctionalTests/Migrations/SqlServerMigrationsSqlGeneratorTest.cs index e15d3ee9401..0211d351db8 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Migrations/SqlServerMigrationsSqlGeneratorTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Migrations/SqlServerMigrationsSqlGeneratorTest.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using Microsoft.EntityFrameworkCore.Diagnostics; using Microsoft.EntityFrameworkCore.Metadata.Internal; using Microsoft.EntityFrameworkCore.SqlServer.Internal; using Microsoft.EntityFrameworkCore.SqlServer.Metadata.Internal; @@ -1288,8 +1289,7 @@ public void Invalid_column_type_throws_meaningful_exception() } })); - Assert.Contains("People.FirstName", ex.Message); - Assert.Contains("String", ex.Message); + Assert.Equal(RelationalStrings.UnsupportedTypeForColumn("People", "FirstName", "String"), ex.Message); } [ConditionalFact] @@ -1306,8 +1306,7 @@ public void Invalid_column_type_for_unmappable_clr_type_throws_meaningful_except IsNullable = false })); - Assert.Contains("TestTable.TestColumn", ex.Message); - Assert.Contains("FileStream", ex.Message); + Assert.Equal(RelationalStrings.UnsupportedTypeForColumn("TestTable", "TestColumn", "FileStream"), ex.Message); } [ConditionalFact] From 7ee5f5455f3c382e4ffff4db6f540c42e3e496c0 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 28 Aug 2025 00:33:39 +0000 Subject: [PATCH 6/7] Remove invalid column type test that doesn't represent affected scenario Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com> --- .../SqlServerMigrationsSqlGeneratorTest.cs | 22 ------------------- 1 file changed, 22 deletions(-) diff --git a/test/EFCore.SqlServer.FunctionalTests/Migrations/SqlServerMigrationsSqlGeneratorTest.cs b/test/EFCore.SqlServer.FunctionalTests/Migrations/SqlServerMigrationsSqlGeneratorTest.cs index 0211d351db8..b9908faeddf 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Migrations/SqlServerMigrationsSqlGeneratorTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Migrations/SqlServerMigrationsSqlGeneratorTest.cs @@ -1268,29 +1268,7 @@ FROM [sys].[default_constraints] [d] """); } - [ConditionalFact] - public void Invalid_column_type_throws_meaningful_exception() - { - var ex = Assert.Throws(() => - Generate( - new CreateTableOperation - { - Name = "People", - Columns = - { - new AddColumnOperation - { - Name = "FirstName", - Table = "People", - ClrType = typeof(string), - ColumnType = "decimal(18,2)", // Invalid store type for string - IsNullable = false - } - } - })); - Assert.Equal(RelationalStrings.UnsupportedTypeForColumn("People", "FirstName", "String"), ex.Message); - } [ConditionalFact] public void Invalid_column_type_for_unmappable_clr_type_throws_meaningful_exception() From 6b592170933e3ab093f044579ccd503eb65709e9 Mon Sep 17 00:00:00 2001 From: Andriy Svyryd Date: Thu, 28 Aug 2025 17:20:21 -0700 Subject: [PATCH 7/7] Remove default value fallback --- .../Migrations/MigrationsSqlGenerator.cs | 11 ---------- .../SqlServerMigrationsSqlGenerator.cs | 4 +--- .../SqlServerMigrationsSqlGeneratorTest.cs | 20 ------------------- 3 files changed, 1 insertion(+), 34 deletions(-) diff --git a/src/EFCore.Relational/Migrations/MigrationsSqlGenerator.cs b/src/EFCore.Relational/Migrations/MigrationsSqlGenerator.cs index c4cfa1db4cc..69897e0742a 100644 --- a/src/EFCore.Relational/Migrations/MigrationsSqlGenerator.cs +++ b/src/EFCore.Relational/Migrations/MigrationsSqlGenerator.cs @@ -1447,17 +1447,6 @@ protected virtual string GetColumnType( return storeType; } - // Try getting the type from the default value if available - if (operation.DefaultValue != null) - { - var defaultValueMapping = Dependencies.TypeMappingSource.GetMappingForValue(operation.DefaultValue); - if (defaultValueMapping?.StoreType != null) - { - return defaultValueMapping.StoreType; - } - } - - // If no mapping found, throw with detailed information var fullTableName = schema != null ? $"{schema}.{tableName}" : tableName; throw new InvalidOperationException( RelationalStrings.UnsupportedTypeForColumn(fullTableName, name, operation.ClrType?.Name ?? "unknown")); diff --git a/src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs b/src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs index 7d004a0b050..a59c7413f9d 100644 --- a/src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs +++ b/src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs @@ -383,9 +383,7 @@ protected override void Generate( { Check.DebugAssert(operation.DefaultValue is not null); - var typeMapping = (columnType != null - ? Dependencies.TypeMappingSource.FindMapping(operation.DefaultValue.GetType(), columnType) - : null) + var typeMapping = Dependencies.TypeMappingSource.FindMapping(operation.DefaultValue.GetType(), columnType) ?? Dependencies.TypeMappingSource.GetMappingForValue(operation.DefaultValue); defaultValueSql = typeMapping.GenerateSqlLiteral(operation.DefaultValue); diff --git a/test/EFCore.SqlServer.FunctionalTests/Migrations/SqlServerMigrationsSqlGeneratorTest.cs b/test/EFCore.SqlServer.FunctionalTests/Migrations/SqlServerMigrationsSqlGeneratorTest.cs index b9908faeddf..22ddb64c6bf 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Migrations/SqlServerMigrationsSqlGeneratorTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Migrations/SqlServerMigrationsSqlGeneratorTest.cs @@ -1287,26 +1287,6 @@ public void Invalid_column_type_for_unmappable_clr_type_throws_meaningful_except Assert.Equal(RelationalStrings.UnsupportedTypeForColumn("TestTable", "TestColumn", "FileStream"), ex.Message); } - [ConditionalFact] - public void Column_type_found_from_default_value_when_clr_type_unmappable() - { - Generate( - new AddColumnOperation - { - Name = "TestColumn", - Table = "TestTable", - ClrType = typeof(System.IO.FileStream), // Unmappable CLR type - ColumnType = null, - DefaultValue = "test string", // String default value should give us nvarchar mapping - IsNullable = false - }); - - AssertSql( - """ -ALTER TABLE [TestTable] ADD [TestColumn] nvarchar(max) NOT NULL DEFAULT N'test string'; -"""); - } - private static void CreateGotModel(ModelBuilder b) => b.HasDefaultSchema("dbo").Entity( "Person", pb =>