Fix to #30330 - Json: updating property with conversion from string t…

…o other type fails on sql server when producing update statement, when deciding whether we need CAST around the result of JSON_VALUE, we need to look if the property has converter, rather than just look at it's clr type. If the clr type is string, but it gets converted to int/bool etc we still need a CAST. Fixes #30330
dotnet · Mar 2, 2023 · f707d02 · f707d02
1 parent c4f0826
commit f707d02
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 6 deletions.
diff --git a/src/EFCore.SqlServer/Update/Internal/SqlServerUpdateSqlGenerator.cs b/src/EFCore.SqlServer/Update/Internal/SqlServerUpdateSqlGenerator.cs
@@ -154,8 +154,10 @@ protected override void AppendUpdateColumnValue(
 
             if (columnModification.Property != null)
             {
-                var needsTypeConversion = columnModification.Property.ClrType.IsNumeric()
-                    || columnModification.Property.ClrType == typeof(bool);
+                var propertyClrType = columnModification.Property.GetTypeMapping().Converter?.ProviderClrType
+                    ?? columnModification.Property.ClrType;
+
+                var needsTypeConversion = propertyClrType.IsNumeric() || propertyClrType == typeof(bool);
 
                 if (needsTypeConversion)
                 {

diff --git a/test/EFCore.SqlServer.FunctionalTests/Update/JsonUpdateSqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Update/JsonUpdateSqlServerTest.cs
@@ -1328,20 +1328,52 @@ FROM [JsonEntitiesConverters] AS [j]
 """);
     }
 
-    [ConditionalFact(Skip = "issue #30330")]
+    [ConditionalFact]
     public override async Task Edit_single_property_with_converter_string_True_False_to_bool()
     {
         await base.Edit_single_property_with_converter_string_True_False_to_bool();
 
-        AssertSql();
+        AssertSql(
+"""
+@p0='[false]' (Nullable = false) (Size = 7)
+@p1='1'
+
+SET IMPLICIT_TRANSACTIONS OFF;
+SET NOCOUNT ON;
+UPDATE [JsonEntitiesConverters] SET [Reference] = JSON_MODIFY([Reference], 'strict $.StringTrueFalseConvertedToBool', CAST(JSON_VALUE(@p0, '$[0]') AS bit))
+OUTPUT 1
+WHERE [Id] = @p1;
+""",
+                //
+                """
+SELECT TOP(2) [j].[Id], [j].[Reference]
+FROM [JsonEntitiesConverters] AS [j]
+WHERE [j].[Id] = 1
+""");
     }
 
-    [ConditionalFact(Skip = "issue #30330")]
+    [ConditionalFact]
     public override async Task Edit_single_property_with_converter_string_Y_N_to_bool()
     {
         await base.Edit_single_property_with_converter_string_Y_N_to_bool();
 
-        AssertSql();
+        AssertSql(
+"""
+@p0='[true]' (Nullable = false) (Size = 6)
+@p1='1'
+
+SET IMPLICIT_TRANSACTIONS OFF;
+SET NOCOUNT ON;
+UPDATE [JsonEntitiesConverters] SET [Reference] = JSON_MODIFY([Reference], 'strict $.StringYNConvertedToBool', CAST(JSON_VALUE(@p0, '$[0]') AS bit))
+OUTPUT 1
+WHERE [Id] = @p1;
+""",
+                //
+                """
+SELECT TOP(2) [j].[Id], [j].[Reference]
+FROM [JsonEntitiesConverters] AS [j]
+WHERE [j].[Id] = 1
+""");
     }
 
     protected override void ClearLog()