Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Json: updating property with conversion from string to other type fails on sql server #30330

Closed
maumar opened this issue Feb 23, 2023 · 1 comment · Fixed by #30380
Closed
Assignees
Labels
area-json closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported Servicing-approved type-bug
Milestone

Comments

@maumar
Copy link
Contributor

maumar commented Feb 23, 2023

when updating a "normal" (non-string) property, we produce the following sql:

@p0='[true]' (Nullable = false) (Size = 6)
@p1='[false]' (Nullable = false) (Size = 7)
@p2='1'

SET IMPLICIT_TRANSACTIONS OFF;
SET NOCOUNT ON;
UPDATE [JsonEntitiesAllTypes] SET [Collection] = JSON_MODIFY([Collection], 'strict $[0].TestBoolean', CAST(JSON_VALUE(@p0, '$[0]') AS bit)), [Reference] = JSON_MODIFY([Reference], 'strict $.TestBoolean', CAST(JSON_VALUE(@p1, '$[0]') AS bit))
OUTPUT 1
WHERE [Id] = @p2;

however if we update property that is string on client side but (say) bool on the server we issue the following:

@p0='[false]' (Nullable = false) (Size = 7)
@p1='1'

SET IMPLICIT_TRANSACTIONS OFF;
SET NOCOUNT ON;
UPDATE [JsonEntitiesConverters] SET [Reference] = JSON_MODIFY([Reference], ''strict $.StringTrueFalseConvertedToBool'', JSON_VALUE(@p0, ''$[0]''))
OUTPUT 1
WHERE [Id] = @p1;

i.e. we don't wrap the value in cast to a provider type, which then results in us storing it as string and causes problems when querying the values. We should look if the property has a converter and check provider type when determining if we need CAST or not.

@maumar maumar self-assigned this Feb 23, 2023
@maumar maumar changed the title Query/Json: updating property with conversion from string to other type fails on sql server Json: updating property with conversion from string to other type fails on sql server Feb 23, 2023
maumar added a commit that referenced this issue Mar 2, 2023
…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
@maumar maumar added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Mar 2, 2023
@maumar maumar added this to the 8.0.0 milestone Mar 2, 2023
maumar added a commit that referenced this issue Mar 2, 2023
…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
maumar added a commit that referenced this issue Mar 2, 2023
…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
@ghost ghost closed this as completed in #30380 Mar 2, 2023
ghost pushed a commit that referenced this issue Mar 2, 2023
…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
@ajcvickers ajcvickers modified the milestones: 8.0.0, 8.0.0-preview3 Mar 13, 2023
@maumar maumar reopened this Mar 31, 2023
@maumar maumar modified the milestones: 8.0.0-preview3, 7.0.x Mar 31, 2023
@maumar maumar closed this as completed Mar 31, 2023
@maumar maumar modified the milestones: 7.0.x, 8.0.0-preview3 Mar 31, 2023
@maumar
Copy link
Contributor Author

maumar commented Apr 3, 2023

reopening for servicing given the #30598 customer report (relatively common scenario)

@maumar maumar reopened this Apr 3, 2023
@maumar maumar modified the milestones: 8.0.0-preview3, 7.0.x Apr 3, 2023
maumar added a commit that referenced this issue Apr 3, 2023
…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
ajcvickers pushed a commit that referenced this issue Apr 5, 2023
… from string to other type fails on sql server (#30615)

Fixes #30330
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-json closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported Servicing-approved type-bug
Projects
None yet
2 participants