From 818447e12b7d14cad9663d12915f5d9550bf0243 Mon Sep 17 00:00:00 2001 From: David Wimmer Date: Wed, 9 Nov 2022 14:29:50 +0100 Subject: [PATCH 1/3] Consider non-default column collation in table variable - Add property annotation RelationalAnnotationNames.Collation to runtime model - Consider column collation annotation when creating DELCARE TABLE statement - Fixed unit test Fixes #7172 --- .../RelationalCSharpRuntimeAnnotationCodeGenerator.cs | 1 - .../Extensions/RelationalPropertyExtensions.cs | 9 +-------- .../Conventions/RelationalRuntimeModelConvention.cs | 1 - .../Update/Internal/SqlServerUpdateSqlGenerator.cs | 5 ++++- .../Internal/CSharpRuntimeModelCodeGeneratorTest.cs | 6 ++---- 5 files changed, 7 insertions(+), 15 deletions(-) diff --git a/src/EFCore.Relational/Design/Internal/RelationalCSharpRuntimeAnnotationCodeGenerator.cs b/src/EFCore.Relational/Design/Internal/RelationalCSharpRuntimeAnnotationCodeGenerator.cs index 9c5063a632e..3e950f11e98 100644 --- a/src/EFCore.Relational/Design/Internal/RelationalCSharpRuntimeAnnotationCodeGenerator.cs +++ b/src/EFCore.Relational/Design/Internal/RelationalCSharpRuntimeAnnotationCodeGenerator.cs @@ -570,7 +570,6 @@ public override void Generate(IProperty property, CSharpRuntimeAnnotationCodeGen { annotations.Remove(RelationalAnnotationNames.ColumnOrder); annotations.Remove(RelationalAnnotationNames.Comment); - annotations.Remove(RelationalAnnotationNames.Collation); if (annotations.TryGetAndRemove( RelationalAnnotationNames.RelationalOverrides, diff --git a/src/EFCore.Relational/Extensions/RelationalPropertyExtensions.cs b/src/EFCore.Relational/Extensions/RelationalPropertyExtensions.cs index 3ae8d586c88..e7ac71135f9 100644 --- a/src/EFCore.Relational/Extensions/RelationalPropertyExtensions.cs +++ b/src/EFCore.Relational/Extensions/RelationalPropertyExtensions.cs @@ -1250,9 +1250,7 @@ public static void SetComment(this IMutableProperty property, string? comment) /// The property. /// The collation for the column this property is mapped to. public static string? GetCollation(this IReadOnlyProperty property) - => (property is RuntimeProperty) - ? throw new InvalidOperationException(CoreStrings.RuntimeModelMissingData) - : (string?)property.FindAnnotation(RelationalAnnotationNames.Collation)?.Value; + => (string?)property.FindAnnotation(RelationalAnnotationNames.Collation)?.Value; /// /// Returns the collation to be used for the column. @@ -1262,11 +1260,6 @@ public static void SetComment(this IMutableProperty property, string? comment) /// The collation for the column this property is mapped to. public static string? GetCollation(this IReadOnlyProperty property, in StoreObjectIdentifier storeObject) { - if (property is RuntimeProperty) - { - throw new InvalidOperationException(CoreStrings.RuntimeModelMissingData); - } - var annotation = property.FindAnnotation(RelationalAnnotationNames.Collation); return annotation != null ? (string?)annotation.Value diff --git a/src/EFCore.Relational/Metadata/Conventions/RelationalRuntimeModelConvention.cs b/src/EFCore.Relational/Metadata/Conventions/RelationalRuntimeModelConvention.cs index d42c8594e8a..619e0770d72 100644 --- a/src/EFCore.Relational/Metadata/Conventions/RelationalRuntimeModelConvention.cs +++ b/src/EFCore.Relational/Metadata/Conventions/RelationalRuntimeModelConvention.cs @@ -354,7 +354,6 @@ protected override void ProcessPropertyAnnotations( { annotations.Remove(RelationalAnnotationNames.ColumnOrder); annotations.Remove(RelationalAnnotationNames.Comment); - annotations.Remove(RelationalAnnotationNames.Collation); if (annotations.TryGetValue(RelationalAnnotationNames.RelationalOverrides, out var relationalOverrides)) { diff --git a/src/EFCore.SqlServer/Update/Internal/SqlServerUpdateSqlGenerator.cs b/src/EFCore.SqlServer/Update/Internal/SqlServerUpdateSqlGenerator.cs index bf6827a2482..4c7ecc27a19 100644 --- a/src/EFCore.SqlServer/Update/Internal/SqlServerUpdateSqlGenerator.cs +++ b/src/EFCore.SqlServer/Update/Internal/SqlServerUpdateSqlGenerator.cs @@ -767,12 +767,15 @@ private void AppendDeclareTable( private static string GetTypeNameForCopy(IProperty property) { var typeName = property.GetColumnType(); + var collation = property[RelationalAnnotationNames.Collation]?.ToString(); - return property.ClrType == typeof(byte[]) + typeName = property.ClrType == typeof(byte[]) && (typeName.Equals("rowversion", StringComparison.OrdinalIgnoreCase) || typeName.Equals("timestamp", StringComparison.OrdinalIgnoreCase)) ? property.IsNullable ? "varbinary(8)" : "binary(8)" : typeName; + + return collation is not null ? $"{typeName} COLLATE {collation}" : typeName; } /// diff --git a/test/EFCore.Design.Tests/Scaffolding/Internal/CSharpRuntimeModelCodeGeneratorTest.cs b/test/EFCore.Design.Tests/Scaffolding/Internal/CSharpRuntimeModelCodeGeneratorTest.cs index a6f25ac4bea..8ae45bedadb 100644 --- a/test/EFCore.Design.Tests/Scaffolding/Internal/CSharpRuntimeModelCodeGeneratorTest.cs +++ b/test/EFCore.Design.Tests/Scaffolding/Internal/CSharpRuntimeModelCodeGeneratorTest.cs @@ -1290,6 +1290,7 @@ public static RuntimeEntityType Create(RuntimeModel model, RuntimeEntityType? ba overrides0.Add(StoreObjectIdentifier.Table(""Details"", null), detailsDetails); details.AddAnnotation(""Relational:RelationalOverrides"", overrides0); + details.AddAnnotation(""Relational:Collation"", ""Latin1_General_CI_AI""); details.AddAnnotation(""SqlServer:ValueGenerationStrategy"", SqlServerValueGenerationStrategy.None); var number = runtimeEntityType.AddProperty( @@ -1903,10 +1904,7 @@ public static void CreateAnnotations(RuntimeEntityType runtimeEntityType) Assert.Equal( CoreStrings.RuntimeModelMissingData, Assert.Throws(() => detailsProperty.IsSparse()).Message); - Assert.Null(detailsProperty[RelationalAnnotationNames.Collation]); - Assert.Equal( - CoreStrings.RuntimeModelMissingData, - Assert.Throws(() => detailsProperty.GetCollation()).Message); + Assert.Equal("Latin1_General_CI_AI", detailsProperty.GetCollation()); var ownedFragment = referenceOwnedType.GetMappingFragments().Single(); Assert.Equal(nameof(OwnedType.Details), detailsProperty.GetColumnName(ownedFragment.StoreObject)); From e25b3e4c1a8508626520023a850566589e451cb3 Mon Sep 17 00:00:00 2001 From: David Wimmer <45233164+david-wimmer@users.noreply.github.com> Date: Fri, 11 Nov 2022 08:54:52 +0100 Subject: [PATCH 2/3] Avoid double negation Co-authored-by: Shay Rojansky --- .../Update/Internal/SqlServerUpdateSqlGenerator.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/EFCore.SqlServer/Update/Internal/SqlServerUpdateSqlGenerator.cs b/src/EFCore.SqlServer/Update/Internal/SqlServerUpdateSqlGenerator.cs index 4c7ecc27a19..ef69e7d663d 100644 --- a/src/EFCore.SqlServer/Update/Internal/SqlServerUpdateSqlGenerator.cs +++ b/src/EFCore.SqlServer/Update/Internal/SqlServerUpdateSqlGenerator.cs @@ -775,7 +775,7 @@ private static string GetTypeNameForCopy(IProperty property) ? property.IsNullable ? "varbinary(8)" : "binary(8)" : typeName; - return collation is not null ? $"{typeName} COLLATE {collation}" : typeName; + return collation is null ? typeName : $"{typeName} COLLATE {collation}"; } /// From bcb9e428747edebf25cf7a4f9a41c159a271df28 Mon Sep 17 00:00:00 2001 From: David Wimmer Date: Mon, 14 Nov 2022 13:03:23 +0100 Subject: [PATCH 3/3] Add collation annotation for SQL Server only --- .../RelationalCSharpRuntimeAnnotationCodeGenerator.cs | 1 + .../Metadata/Conventions/RelationalRuntimeModelConvention.cs | 1 + .../SqlServerCSharpRuntimeAnnotationCodeGenerator.cs | 5 +++++ .../Metadata/Conventions/SqlServerRuntimeModelConvention.cs | 5 +++++ .../Internal/CSharpRuntimeModelCodeGeneratorTest.cs | 3 +-- 5 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/EFCore.Relational/Design/Internal/RelationalCSharpRuntimeAnnotationCodeGenerator.cs b/src/EFCore.Relational/Design/Internal/RelationalCSharpRuntimeAnnotationCodeGenerator.cs index 3e950f11e98..9c5063a632e 100644 --- a/src/EFCore.Relational/Design/Internal/RelationalCSharpRuntimeAnnotationCodeGenerator.cs +++ b/src/EFCore.Relational/Design/Internal/RelationalCSharpRuntimeAnnotationCodeGenerator.cs @@ -570,6 +570,7 @@ public override void Generate(IProperty property, CSharpRuntimeAnnotationCodeGen { annotations.Remove(RelationalAnnotationNames.ColumnOrder); annotations.Remove(RelationalAnnotationNames.Comment); + annotations.Remove(RelationalAnnotationNames.Collation); if (annotations.TryGetAndRemove( RelationalAnnotationNames.RelationalOverrides, diff --git a/src/EFCore.Relational/Metadata/Conventions/RelationalRuntimeModelConvention.cs b/src/EFCore.Relational/Metadata/Conventions/RelationalRuntimeModelConvention.cs index 619e0770d72..d42c8594e8a 100644 --- a/src/EFCore.Relational/Metadata/Conventions/RelationalRuntimeModelConvention.cs +++ b/src/EFCore.Relational/Metadata/Conventions/RelationalRuntimeModelConvention.cs @@ -354,6 +354,7 @@ protected override void ProcessPropertyAnnotations( { annotations.Remove(RelationalAnnotationNames.ColumnOrder); annotations.Remove(RelationalAnnotationNames.Comment); + annotations.Remove(RelationalAnnotationNames.Collation); if (annotations.TryGetValue(RelationalAnnotationNames.RelationalOverrides, out var relationalOverrides)) { diff --git a/src/EFCore.SqlServer/Design/Internal/SqlServerCSharpRuntimeAnnotationCodeGenerator.cs b/src/EFCore.SqlServer/Design/Internal/SqlServerCSharpRuntimeAnnotationCodeGenerator.cs index 5a5466ced33..ecbe55311b0 100644 --- a/src/EFCore.SqlServer/Design/Internal/SqlServerCSharpRuntimeAnnotationCodeGenerator.cs +++ b/src/EFCore.SqlServer/Design/Internal/SqlServerCSharpRuntimeAnnotationCodeGenerator.cs @@ -58,6 +58,11 @@ public override void Generate(IProperty property, CSharpRuntimeAnnotationCodeGen { annotations[SqlServerAnnotationNames.ValueGenerationStrategy] = property.GetValueGenerationStrategy(); } + + if (!annotations.ContainsKey(RelationalAnnotationNames.Collation)) + { + annotations[RelationalAnnotationNames.Collation] = property.GetCollation(); + } } base.Generate(property, parameters); diff --git a/src/EFCore.SqlServer/Metadata/Conventions/SqlServerRuntimeModelConvention.cs b/src/EFCore.SqlServer/Metadata/Conventions/SqlServerRuntimeModelConvention.cs index e9118e97711..1818de4e7d3 100644 --- a/src/EFCore.SqlServer/Metadata/Conventions/SqlServerRuntimeModelConvention.cs +++ b/src/EFCore.SqlServer/Metadata/Conventions/SqlServerRuntimeModelConvention.cs @@ -67,6 +67,11 @@ protected override void ProcessPropertyAnnotations( { annotations[SqlServerAnnotationNames.ValueGenerationStrategy] = property.GetValueGenerationStrategy(); } + + if (!annotations.ContainsKey(RelationalAnnotationNames.Collation)) + { + annotations[RelationalAnnotationNames.Collation] = property.GetCollation(); + } } } diff --git a/test/EFCore.Design.Tests/Scaffolding/Internal/CSharpRuntimeModelCodeGeneratorTest.cs b/test/EFCore.Design.Tests/Scaffolding/Internal/CSharpRuntimeModelCodeGeneratorTest.cs index 8ae45bedadb..5dd1120df7e 100644 --- a/test/EFCore.Design.Tests/Scaffolding/Internal/CSharpRuntimeModelCodeGeneratorTest.cs +++ b/test/EFCore.Design.Tests/Scaffolding/Internal/CSharpRuntimeModelCodeGeneratorTest.cs @@ -1290,7 +1290,6 @@ public static RuntimeEntityType Create(RuntimeModel model, RuntimeEntityType? ba overrides0.Add(StoreObjectIdentifier.Table(""Details"", null), detailsDetails); details.AddAnnotation(""Relational:RelationalOverrides"", overrides0); - details.AddAnnotation(""Relational:Collation"", ""Latin1_General_CI_AI""); details.AddAnnotation(""SqlServer:ValueGenerationStrategy"", SqlServerValueGenerationStrategy.None); var number = runtimeEntityType.AddProperty( @@ -1904,7 +1903,7 @@ public static void CreateAnnotations(RuntimeEntityType runtimeEntityType) Assert.Equal( CoreStrings.RuntimeModelMissingData, Assert.Throws(() => detailsProperty.IsSparse()).Message); - Assert.Equal("Latin1_General_CI_AI", detailsProperty.GetCollation()); + Assert.Null(detailsProperty[RelationalAnnotationNames.Collation]); var ownedFragment = referenceOwnedType.GetMappingFragments().Single(); Assert.Equal(nameof(OwnedType.Details), detailsProperty.GetColumnName(ownedFragment.StoreObject));