JSON: optimize update path for single property element - don't wrap the value in json array

[maumar]

Currently when updating single property inside json, we create parameter value as JSON array and put the value in it, then extract it in the update statement. This is because we want to guarantee the correct format of the value that we want to update (if we rely on database casting from a given type to string, we may end up in bad format and data gets corrupted. This only happens for some data types on sql server (guid, datetime, datetime offset) - so maybe we can special case. But need to look at Sqlite. Alternatively, if/when we have dedicated type mapping for json, we might be able to always generate parameter string value in correct format to be stored inside JSON.

[roji]

@maumar can you provide a bit more details on the bad format corruption?

I see that in ModificationCommand, for partial updates we simply do JsonValue.Create over the property value. For any types not natively supported by JSON (DateTime, Guid), can't we just call ToJsonString() on that JsonValue and sent that string as the parameter value to JSON_MODIFY? I think that doesn't require any array, JSON_VALUE or server-side casting.

(for natively-supported JSON types - int/bool/null - we should be able to just send a regular parameter with the right value, e.g. send bool as a BIT parameter, which JSON_MODIFY applies as true/false).

@ajcvickers @AndriySvyryd this is related to our recent conversations about how to generate a literal representation of something in JSON context; here the problem is what we do with parameters. In other words, if we want to send a DateTime/Guid into a JSON document (partial update), we need to be able to actually send a string with the appropriate timestamp format. We currently delegate this conversion to System.Text.Json in ModificationCommand, but this can/should probably be refactored into the same facility we'd use for literals.

[maumar]

by corruption I mean that when we try to ExtractJsonProperty<T> we get an error, because the JsonElement we read can't be deserialized into the requested type. Calling ToJsonString on those params should work, especially that now we have a provider specific hook (in case we need some special processing for Sqlite etc.) - will give it a try

edit: when generating ColumnModification, we use JSON column type mapping (the only one we have available, since individual JSON properties don't have their own mappings). Even if the value is, say, int = 1234, the parameter that gets generated is nvarchar (i.e. '1234'), so we still need to keep the convert before we pass the parameter value to JSON_MODIFY. But we can get rid of the array trick at least. And the convert will likely be able to go away when/if we get dedicated JSON type mapping.

[AndriySvyryd]

@roji Yes, as we've discussed this should be handled by RelationalTypeMapping.GenerateSqlLiteral (and that in turn could delegate this to a service)