Simplify SQL when copying JSON scalar properties across documents (#3627)

Continues #3608

Author: roji

src/EFCore.PG/Query/Internal/NpgsqlQuerySqlGenerator.cs

@@ -1092,8 +1092,10 @@ protected override Expression VisitJsonScalar(JsonScalarExpression jsonScalarExp
                 GenerateJsonPath(jsonScalarExpression.Json, returnsText: false, path);
                 break;
 
-            // Scalar collection mapped to JSON (either because it's nested within a JSON document, or because the user explicitly
-            // opted for this rather than the default PG array mapping).
+            // A scalar to be extracted out as JSON (and not as e.g. text/int).
+            // This happens for scalar collection mapped to JSON (either because it's nested within a JSON document,
+            // or because the user explicitly opted for this rather than the default PG array mapping), or when we copy a scalar JSON
+            // property from one document to another via ExecuteUpdate.
             case NpgsqlJsonTypeMapping typeMapping:
                 Check.DebugAssert(typeMapping.ElementTypeMapping is not null);
                 GenerateJsonPath(jsonScalarExpression.Json, returnsText: false, path);

src/EFCore.PG/Query/Internal/NpgsqlQueryableMethodTranslatingExpressionVisitor.cs

@@ -1301,6 +1301,27 @@ protected override bool TrySerializeScalarToJson(
                     break;
             }
 
+            // We now have a scalar value expression that needs to be passed to jsonb_set(), but jsonb_set() requires a json/jsonb
+            // argument, not e.g. text or int. So we need to wrap the argument in to_jsonb/to_json.
+            // Note that for structural types we always already get a jsonb/json value and have already exited above (no need for
+            // to_jsonb/to_json).
+
+            // One exception is if the value expression happens to be a JsonScalarExpression (e.g. copy scalar property from within
+            // one JSON document into another). For that case, rather than do to_jsonb(x.JsonbDoc ->> 'SomeProperty') - which extracts
+            // a jsonb property as text only to reconvert it back to jsonb - we just change the type mapping on the JsonScalarExpression
+            // to json/jsonb, in order to generate x.JsonbDoc -> 'SomeProperty' (no text extraction).
+            if (value is JsonScalarExpression jsonScalarValue
+                && jsonScalarValue.Json.TypeMapping?.StoreType == jsonTypeMapping.StoreType)
+            {
+                jsonValue = new JsonScalarExpression(
+                    jsonScalarValue.Json,
+                    jsonScalarValue.Path,
+                    jsonScalarValue.Type,
+                    jsonTypeMapping,
+                    jsonScalarValue.IsNullable);
+                return true;
+            }
+
             jsonValue = _sqlExpressionFactory.Function(
                 jsonTypeMapping.StoreType switch
                 {

src/EFCore.PG/Storage/Internal/Mapping/NpgsqlJsonTypeMapping.cs

@@ -5,9 +5,11 @@
 namespace Npgsql.EntityFrameworkCore.PostgreSQL.Storage.Internal.Mapping;
 
 /// <summary>
-///     Supports the older Npgsql-specific JSON mapping, allowing mapping json/jsonb to text, to e.g.
+///     Represents scalars within a JSON document that maintain their json/jsonb type (rather than being extracted out as e.g. text/int).
+///     Also supports the older Npgsql-specific JSON mapping, allowing mapping json/jsonb to text, to e.g.
 ///     <see cref="JsonElement" /> (weakly-typed mapping) or to arbitrary POCOs (but without them being modeled).
-///     For the standard EF JSON support, which relies on owned entity modeling, see <see cref="NpgsqlStructuralJsonTypeMapping" />.
+///     Note that for structural types mapped via the standard EF complex/owned mapping, we use
+///     <see cref="NpgsqlStructuralJsonTypeMapping" />.
 /// </summary>
 public class NpgsqlJsonTypeMapping : NpgsqlTypeMapping
 {

test/EFCore.PG.FunctionalTests/Query/Associations/ComplexJson/ComplexJsonBulkUpdateNpgsqlTest.cs

@@ -364,7 +364,7 @@ public override async Task Update_multiple_properties_inside_associations_and_on
 
 UPDATE "RootEntity" AS r
 SET "Name" = r."Name" || 'Modified',
-    "RequiredRelated" = jsonb_set(r."RequiredRelated", '{String}', to_jsonb(r."OptionalRelated" ->> 'String')),
+    "RequiredRelated" = jsonb_set(r."RequiredRelated", '{String}', r."OptionalRelated" -> 'String'),
     "OptionalRelated" = jsonb_set(r."OptionalRelated", '{RequiredNested,String}', to_jsonb(@p))
 WHERE (r."OptionalRelated") IS NOT NULL
 """);
@@ -379,7 +379,7 @@ public override async Task Update_multiple_projected_associations_via_anonymous_
 @p='?'
 
 UPDATE "RootEntity" AS r
-SET "RequiredRelated" = jsonb_set(r."RequiredRelated", '{String}', to_jsonb(r."OptionalRelated" ->> 'String')),
+SET "RequiredRelated" = jsonb_set(r."RequiredRelated", '{String}', r."OptionalRelated" -> 'String'),
     "OptionalRelated" = jsonb_set(r."OptionalRelated", '{String}', to_jsonb(@p))
 WHERE (r."OptionalRelated") IS NOT NULL
 """);

test/EFCore.PG.FunctionalTests/Types/Miscellaneous/NpgsqlBoolTypeTest.cs

@@ -80,7 +80,7 @@ public override async Task ExecuteUpdate_within_json_to_another_json_property()
         AssertSql(
             """
 UPDATE "JsonTypeEntity" AS j
-SET "JsonContainer" = jsonb_set(j."JsonContainer", '{Value}', to_jsonb(CAST(j."JsonContainer" ->> 'OtherValue' AS boolean)))
+SET "JsonContainer" = jsonb_set(j."JsonContainer", '{Value}', j."JsonContainer" -> 'OtherValue')
 """);
     }
 

test/EFCore.PG.FunctionalTests/Types/Miscellaneous/NpgsqlGuidTypeTest.cs

@@ -80,7 +80,7 @@ public override async Task ExecuteUpdate_within_json_to_another_json_property()
         AssertSql(
             """
 UPDATE "JsonTypeEntity" AS j
-SET "JsonContainer" = jsonb_set(j."JsonContainer", '{Value}', to_jsonb(CAST(j."JsonContainer" ->> 'OtherValue' AS uuid)))
+SET "JsonContainer" = jsonb_set(j."JsonContainer", '{Value}', j."JsonContainer" -> 'OtherValue')
 """);
     }
 

test/EFCore.PG.FunctionalTests/Types/Miscellaneous/NpgsqlStringTypeTest.cs

@@ -80,7 +80,7 @@ public override async Task ExecuteUpdate_within_json_to_another_json_property()
         AssertSql(
             """
 UPDATE "JsonTypeEntity" AS j
-SET "JsonContainer" = jsonb_set(j."JsonContainer", '{Value}', to_jsonb(j."JsonContainer" ->> 'OtherValue'))
+SET "JsonContainer" = jsonb_set(j."JsonContainer", '{Value}', j."JsonContainer" -> 'OtherValue')
 """);
     }
 

test/EFCore.PG.FunctionalTests/Types/Networking/NpgsqlInetTypeTest.cs

@@ -82,7 +82,7 @@ public override async Task ExecuteUpdate_within_json_to_another_json_property()
         AssertSql(
             """
 UPDATE "JsonTypeEntity" AS j
-SET "JsonContainer" = jsonb_set(j."JsonContainer", '{Value}', to_jsonb(CAST(j."JsonContainer" ->> 'OtherValue' AS inet)))
+SET "JsonContainer" = jsonb_set(j."JsonContainer", '{Value}', j."JsonContainer" -> 'OtherValue')
 """);
     }
 

test/EFCore.PG.FunctionalTests/Types/Networking/NpgsqlMacaddrTypeTest.cs

@@ -82,7 +82,7 @@ public override async Task ExecuteUpdate_within_json_to_another_json_property()
         AssertSql(
             """
 UPDATE "JsonTypeEntity" AS j
-SET "JsonContainer" = jsonb_set(j."JsonContainer", '{Value}', to_jsonb(CAST(j."JsonContainer" ->> 'OtherValue' AS macaddr)))
+SET "JsonContainer" = jsonb_set(j."JsonContainer", '{Value}', j."JsonContainer" -> 'OtherValue')
 """);
     }
 

test/EFCore.PG.FunctionalTests/Types/Numeric/NpgsqlDecimalTypeTest.cs

@@ -80,7 +80,7 @@ public override async Task ExecuteUpdate_within_json_to_another_json_property()
         AssertSql(
             """
 UPDATE "JsonTypeEntity" AS j
-SET "JsonContainer" = jsonb_set(j."JsonContainer", '{Value}', to_jsonb(CAST(j."JsonContainer" ->> 'OtherValue' AS numeric)))
+SET "JsonContainer" = jsonb_set(j."JsonContainer", '{Value}', j."JsonContainer" -> 'OtherValue')
 """);
     }