Fix to #30410 - JSON: optimize update path for single property elemen…

…t - don't wrap the value in json array

Rather than generate a JSON parameter and extract correct value from it, we generate the parameter value directly. Special casing is handled on the provider level by *ModificationCommand and *UpdateSqlGenerator

Fixes #30410

src/EFCore.Relational/Update/ModificationCommand.cs

@@ -327,9 +327,10 @@ private List<IColumnModification> GenerateColumnModifications()
                 var jsonPathString = string.Join(
                     ".", updateInfo.Path.Select(x => x.PropertyName + (x.Ordinal != null ? "[" + x.Ordinal + "]" : "")));
 
+                object? singlePropertyValue = default;
                 if (updateInfo.Property != null)
                 {
-                    json = new JsonArray(GenerateJsonForSinglePropertyUpdate(updateInfo.Property, updateInfo.PropertyValue));
+                    singlePropertyValue = GenerateValueForSinglePropertyUpdate(updateInfo.Property, updateInfo.PropertyValue);
                     jsonPathString = jsonPathString + "." + updateInfo.Property.GetJsonPropertyName();
                 }
                 else
@@ -367,7 +368,7 @@ private List<IColumnModification> GenerateColumnModifications()
 
                 var columnModificationParameters = new ColumnModificationParameters(
                     jsonColumn.Name,
-                    value: json?.ToJsonString(),
+                    value: updateInfo.Property != null ? singlePropertyValue : json?.ToJsonString(),
                     property: updateInfo.Property,
                     columnType: jsonColumnTypeMapping.StoreType,
                     jsonColumnTypeMapping,
@@ -699,14 +700,23 @@ static JsonPartialUpdateInfo FindCommonJsonPartialUpdateInfo(
     }
 
     /// <summary>
-    ///     Generates <see cref="JsonNode" /> representing the value to use for update in case a single property is being updated.
+    ///     Generates value to use for update in case a single property is being updated.
     /// </summary>
     /// <param name="property">Property to be updated.</param>
     /// <param name="propertyValue">Value object that the property will be updated to.</param>
-    /// <returns><see cref="JsonNode" /> representing the value that the property will be updated to.</returns>
+    /// <returns>Value that the property will be updated to.</returns>
     [EntityFrameworkInternal]
-    protected virtual JsonNode? GenerateJsonForSinglePropertyUpdate(IProperty property, object? propertyValue)
-        => JsonValue.Create(propertyValue);
+    protected virtual object? GenerateValueForSinglePropertyUpdate(IProperty property, object? propertyValue)
+    {
+        var propertyProviderClrType = (property.GetTypeMapping().Converter?.ProviderClrType ?? property.ClrType).UnwrapNullableType();
+
+        return (propertyProviderClrType == typeof(DateTime)
+            || propertyProviderClrType == typeof(DateTimeOffset)
+            || propertyProviderClrType == typeof(TimeSpan)
+            || propertyProviderClrType == typeof(Guid))
+            ? JsonValue.Create(propertyValue)?.ToJsonString().Replace("\"", "")
+            : propertyValue;
+    }
 
     private JsonNode? CreateJson(object? navigationValue, IUpdateEntry parentEntry, IEntityType entityType, int? ordinal, bool isCollection)
     {

src/EFCore.SqlServer/Extensions/SqlServerServiceCollectionExtensions.cs

@@ -106,6 +106,7 @@ public static IServiceCollection AddEntityFrameworkSqlServer(this IServiceCollec
             .TryAdd<IEvaluatableExpressionFilter, SqlServerEvaluatableExpressionFilter>()
             .TryAdd<IRelationalTransactionFactory, SqlServerTransactionFactory>()
             .TryAdd<IModificationCommandBatchFactory, SqlServerModificationCommandBatchFactory>()
+            .TryAdd<IModificationCommandFactory, SqlServerModificationCommandFactory>()
             .TryAdd<IValueGeneratorSelector, SqlServerValueGeneratorSelector>()
             .TryAdd<IRelationalConnection>(p => p.GetRequiredService<ISqlServerConnection>())
             .TryAdd<IMigrationsSqlGenerator, SqlServerMigrationsSqlGenerator>()

src/EFCore.SqlServer/Update/Internal/SqlServerModificationCommand.cs

@@ -0,0 +1,67 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System.Text.Json.Nodes;
+
+namespace Microsoft.EntityFrameworkCore.SqlServer.Update.Internal;
+
+/// <summary>
+///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
+///     the same compatibility standards as public APIs. It may be changed or removed without notice in
+///     any release. You should only use it directly in your code with extreme caution and knowing that
+///     doing so can result in application failures when updating to a new Entity Framework Core release.
+/// </summary>
+public class SqlServerModificationCommand : ModificationCommand
+{
+    /// <summary>
+    ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
+    ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
+    ///     any release. You should only use it directly in your code with extreme caution and knowing that
+    ///     doing so can result in application failures when updating to a new Entity Framework Core release.
+    /// </summary>
+    public SqlServerModificationCommand(in ModificationCommandParameters modificationCommandParameters)
+        : base(modificationCommandParameters)
+    {
+    }
+
+    /// <summary>
+    ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
+    ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
+    ///     any release. You should only use it directly in your code with extreme caution and knowing that
+    ///     doing so can result in application failures when updating to a new Entity Framework Core release.
+    /// </summary>
+    public SqlServerModificationCommand(in NonTrackedModificationCommandParameters modificationCommandParameters)
+        : base(modificationCommandParameters)
+    {
+    }
+
+    /// <summary>
+    ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
+    ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
+    ///     any release. You should only use it directly in your code with extreme caution and knowing that
+    ///     doing so can result in application failures when updating to a new Entity Framework Core release.
+    /// </summary>
+    protected override object? GenerateValueForSinglePropertyUpdate(IProperty property, object? propertyValue)
+    {
+        var propertyProviderClrType = (property.GetTypeMapping().Converter?.ProviderClrType ?? property.ClrType).UnwrapNullableType();
+
+        // when we generate SqlParameter when updating single property in JSON entity
+        // we always use SqlServerJsonTypeMapping as type mapping for the parameter
+        // (since we don't have dedicated type mapping for individual JSON properties)
+        // later, when we generate DbParameter we assign the value and then the DbType from the type mapping.
+        // in case of byte value, when we assign the value to the DbParameter it sets its type to Byte and its size to 1
+        // then, we change DbType to String, but keep size as is
+        // so, if value was, say, 15 we initially generate DbParameter of type Byte, value 25 and size 1
+        // but when we change the type we end up with type String, value 25 and size 1, which effectively is "2"
+        // to mitigate this, we convert the value to string, to guarantee the correct parameter size.
+        // this can be removed when we have dedicated JSON type mapping for individual (leaf) properties
+        if (propertyProviderClrType == typeof(byte))
+        {
+            return JsonValue.Create(propertyValue)?.ToJsonString().Replace("\"", "");
+        }
+
+#pragma warning disable EF1001 // Internal EF Core API usage.
+        return base.GenerateValueForSinglePropertyUpdate(property, propertyValue);
+#pragma warning restore EF1001 // Internal EF Core API usage.
+    }
+}

src/EFCore.SqlServer/Update/Internal/SqlServerModificationCommandFactory.cs

@@ -0,0 +1,33 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+namespace Microsoft.EntityFrameworkCore.SqlServer.Update.Internal;
+
+/// <summary>
+///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
+///     the same compatibility standards as public APIs. It may be changed or removed without notice in
+///     any release. You should only use it directly in your code with extreme caution and knowing that
+///     doing so can result in application failures when updating to a new Entity Framework Core release.
+/// </summary>
+public class SqlServerModificationCommandFactory : IModificationCommandFactory
+{
+    /// <summary>
+    ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
+    ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
+    ///     any release. You should only use it directly in your code with extreme caution and knowing that
+    ///     doing so can result in application failures when updating to a new Entity Framework Core release.
+    /// </summary>
+    public virtual IModificationCommand CreateModificationCommand(
+        in ModificationCommandParameters modificationCommandParameters)
+        => new SqlServerModificationCommand(modificationCommandParameters);
+
+    /// <summary>
+    ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
+    ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
+    ///     any release. You should only use it directly in your code with extreme caution and knowing that
+    ///     doing so can result in application failures when updating to a new Entity Framework Core release.
+    /// </summary>
+    public virtual INonTrackedModificationCommand CreateNonTrackedModificationCommand(
+        in NonTrackedModificationCommandParameters modificationCommandParameters)
+        => new SqlServerModificationCommand(modificationCommandParameters);
+}

src/EFCore.SqlServer/Update/Internal/SqlServerUpdateSqlGenerator.cs

@@ -164,9 +164,7 @@ protected override void AppendUpdateColumnValue(
                     stringBuilder.Append("CAST(");
                 }
 
-                stringBuilder.Append("JSON_VALUE(");
                 base.AppendUpdateColumnValue(updateSqlGeneratorHelper, columnModification, stringBuilder, name, schema);
-                stringBuilder.Append(", '$[0]')");
 
                 if (needsTypeConversion)
                 {

src/EFCore.Sqlite.Core/Update/Internal/SqliteModificationCommand.cs

@@ -1,8 +1,6 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
-using System.Text.Json.Nodes;
-
 namespace Microsoft.EntityFrameworkCore.Sqlite.Update.Internal;
 
 /// <summary>
@@ -41,25 +39,22 @@ public SqliteModificationCommand(in NonTrackedModificationCommandParameters modi
     ///     any release. You should only use it directly in your code with extreme caution and knowing that
     ///     doing so can result in application failures when updating to a new Entity Framework Core release.
     /// </summary>
-    protected override JsonNode? GenerateJsonForSinglePropertyUpdate(IProperty property, object? propertyValue)
+    protected override object? GenerateValueForSinglePropertyUpdate(IProperty property, object? propertyValue)
     {
-        if (propertyValue is bool boolPropertyValue
-            && (property.GetTypeMapping().Converter?.ProviderClrType ?? property.ClrType).UnwrapNullableType() == typeof(bool))
+        var propertyProviderClrType = (property.GetTypeMapping().Converter?.ProviderClrType ?? property.ClrType).UnwrapNullableType();
+
+        if (propertyProviderClrType == typeof(bool) && propertyValue is bool boolPropertyValue)
         {
             // Sqlite converts true/false into native 0/1 when using json_extract
             // so we convert those values to strings so that they stay as true/false
             // which is what we want to store in json object in the end
-            var modifiedPropertyValue = boolPropertyValue
+            return boolPropertyValue
                 ? "true"
                 : "false";
-
-#pragma warning disable EF1001 // Internal EF Core API usage.
-            return base.GenerateJsonForSinglePropertyUpdate(property, modifiedPropertyValue);
-#pragma warning restore EF1001 // Internal EF Core API usage.
         }
 
 #pragma warning disable EF1001 // Internal EF Core API usage.
-        return base.GenerateJsonForSinglePropertyUpdate(property, propertyValue);
+        return base.GenerateValueForSinglePropertyUpdate(property, propertyValue);
 #pragma warning restore EF1001 // Internal EF Core API usage.
     }
 }

src/EFCore.Sqlite.Core/Update/Internal/SqliteUpdateSqlGenerator.cs

@@ -162,24 +162,25 @@ protected override void AppendUpdateColumnValue(
 
             if (columnModification.Property != null)
             {
+                var providerClrType = (columnModification.Property.GetTypeMapping().Converter?.ProviderClrType
+                    ?? columnModification.Property.ClrType).UnwrapNullableType();
+
                 // special handling for bool
                 // json_extract converts true/false into native 0/1 values,
                 // but we want to store the values as true/false in JSON
                 // in order to do that we modify the parameter value to "true"/"false"
                 // and wrap json() function around it to avoid conversion to 0/1
-                var boolProviderType = (columnModification.Property.GetTypeMapping().Converter?.ProviderClrType
-                    ?? columnModification.Property.ClrType).UnwrapNullableType() == typeof(bool);
-
-                if (boolProviderType)
+                //
+                // for decimal, sqlite generates string parameter for decimal values
+                // but don't want to store the values as strings, we use json to "unwrap" it
+                if (providerClrType == typeof(bool) || providerClrType == typeof(decimal))
                 {
                     stringBuilder.Append("json(");
                 }
 
-                stringBuilder.Append("json_extract(");
                 base.AppendUpdateColumnValue(updateSqlGeneratorHelper, columnModification, stringBuilder, name, schema);
-                stringBuilder.Append(", '$[0]')");
 
-                if (boolProviderType)
+                if (providerClrType == typeof(bool) || providerClrType == typeof(decimal))
                 {
                     stringBuilder.Append(")");
                 }