Skip to content

Commit

Permalink
Fix to #30330 - Json: updating property with conversion from string t…
Browse files Browse the repository at this point in the history
…o other type fails on sql server (#30380)

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
  • Loading branch information
maumar committed Mar 2, 2023
1 parent c4f0826 commit 590d63c
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -154,8 +154,10 @@ protected virtual int MergeIntoMinimumThreshold

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)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -558,7 +558,7 @@ public override async Task Edit_single_property_char()

SET IMPLICIT_TRANSACTIONS OFF;
SET NOCOUNT ON;
UPDATE [JsonEntitiesAllTypes] SET [Reference] = JSON_MODIFY([Reference], 'strict $.TestCharacter', CAST(JSON_VALUE(@p0, '$[0]') AS nvarchar(1)))
UPDATE [JsonEntitiesAllTypes] SET [Reference] = JSON_MODIFY([Reference], 'strict $.TestCharacter', JSON_VALUE(@p0, '$[0]'))
OUTPUT 1
WHERE [Id] = @p1;
""",
Expand Down Expand Up @@ -1270,7 +1270,7 @@ public override async Task Edit_single_property_with_converter_bool_to_string_Tr

SET IMPLICIT_TRANSACTIONS OFF;
SET NOCOUNT ON;
UPDATE [JsonEntitiesConverters] SET [Reference] = JSON_MODIFY([Reference], 'strict $.BoolConvertedToStringTrueFalse', CAST(JSON_VALUE(@p0, '$[0]') AS nvarchar(5)))
UPDATE [JsonEntitiesConverters] SET [Reference] = JSON_MODIFY([Reference], 'strict $.BoolConvertedToStringTrueFalse', JSON_VALUE(@p0, '$[0]'))
OUTPUT 1
WHERE [Id] = @p1;
""",
Expand All @@ -1293,7 +1293,7 @@ public override async Task Edit_single_property_with_converter_bool_to_string_Y_

SET IMPLICIT_TRANSACTIONS OFF;
SET NOCOUNT ON;
UPDATE [JsonEntitiesConverters] SET [Reference] = JSON_MODIFY([Reference], 'strict $.BoolConvertedToStringYN', CAST(JSON_VALUE(@p0, '$[0]') AS nvarchar(1)))
UPDATE [JsonEntitiesConverters] SET [Reference] = JSON_MODIFY([Reference], 'strict $.BoolConvertedToStringYN', JSON_VALUE(@p0, '$[0]'))
OUTPUT 1
WHERE [Id] = @p1;
""",
Expand Down Expand Up @@ -1328,20 +1328,52 @@ OUTPUT 1
""");
}

[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()
Expand Down

0 comments on commit 590d63c

Please sign in to comment.