Fix JSON null partial updates

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

@@ -20,6 +20,7 @@ public class NpgsqlQueryableMethodTranslatingExpressionVisitor : RelationalQuery
     private readonly RelationalQueryCompilationContext _queryCompilationContext;
     private readonly NpgsqlTypeMappingSource _typeMappingSource;
     private readonly NpgsqlSqlExpressionFactory _sqlExpressionFactory;
+    private readonly Version _postgresVersion;
     private readonly bool _isRedshift;
     private RelationalTypeMapping? _ordinalityTypeMapping;
 
@@ -55,6 +56,7 @@ public NpgsqlQueryableMethodTranslatingExpressionVisitor(
         _queryCompilationContext = queryCompilationContext;
         _typeMappingSource = (NpgsqlTypeMappingSource)relationalDependencies.TypeMappingSource;
         _sqlExpressionFactory = (NpgsqlSqlExpressionFactory)relationalDependencies.SqlExpressionFactory;
+        _postgresVersion = npgsqlSingletonOptions.PostgresVersion;
         _isRedshift = npgsqlSingletonOptions.UseRedshift;
     }
 
@@ -70,6 +72,7 @@ protected NpgsqlQueryableMethodTranslatingExpressionVisitor(NpgsqlQueryableMetho
         _queryCompilationContext = parentVisitor._queryCompilationContext;
         _typeMappingSource = parentVisitor._typeMappingSource;
         _sqlExpressionFactory = parentVisitor._sqlExpressionFactory;
+        _postgresVersion = parentVisitor._postgresVersion;
         _isRedshift = parentVisitor._isRedshift;
     }
 
@@ -1274,6 +1277,14 @@ protected override bool TrySerializeScalarToJson(
     {
         var jsonTypeMapping = ((ColumnExpression)target.Json).TypeMapping!;
 
+        if (IsSqlNull(value))
+        {
+            jsonValue = JsonNull(jsonTypeMapping);
+            return true;
+        }
+
+        var coalesceToJsonNull = !SupportsJsonSetLax(jsonTypeMapping) && MayReturnNull(value);
+
         if (
             // The base implementation doesn't handle serializing arbitrary SQL expressions to JSON, since that's
             // database-specific. In PostgreSQL we simply do this by wrapping any expression in to_jsonb().
@@ -1286,7 +1297,7 @@ protected override bool TrySerializeScalarToJson(
             switch (value.TypeMapping!.StoreType)
             {
                 case "jsonb" or "json":
-                    jsonValue = value;
+                    jsonValue = coalesceToJsonNull ? CoalesceToJsonNull(value, jsonTypeMapping) : value;
                     return true;
 
                 case "bytea":
@@ -1319,24 +1330,39 @@ protected override bool TrySerializeScalarToJson(
                     jsonScalarValue.Type,
                     jsonTypeMapping,
                     jsonScalarValue.IsNullable);
+
+                if (coalesceToJsonNull)
+                {
+                    jsonValue = CoalesceToJsonNull(jsonValue, jsonTypeMapping);
+                }
+
                 return true;
             }
 
-            jsonValue = _sqlExpressionFactory.Function(
+            var valueForJsonScalar = NeedsJsonScalarTypeInference(value) ? CastForJsonScalarTypeInference(value, target) : value;
+            var toJson = _sqlExpressionFactory.Function(
                 jsonTypeMapping.StoreType switch
                 {
                     "jsonb" => "to_jsonb",
                     "json" => "to_json",
                     _ => throw new UnreachableException()
                 },
-                // Make sure PG interprets constant values correctly by adding explicit typing based on the target property's type mapping.
+                // Make sure PG interprets constant/nullable parameter values correctly by adding explicit typing based on the target property's
+                // type mapping.
                 // Note that we can only be here for scalar properties, for structural types we always already get a jsonb/json value
                 // and don't need to add to_jsonb/to_json.
-                [value is SqlConstantExpression ? _sqlExpressionFactory.Convert(value, target.Type, target.TypeMapping) : value],
+                [valueForJsonScalar],
                 nullable: true,
                 argumentsPropagateNullability: [true],
                 typeof(string),
                 jsonTypeMapping);
+
+            jsonValue = toJson;
+        }
+
+        if (coalesceToJsonNull)
+        {
+            jsonValue = CoalesceToJsonNull(jsonValue, jsonTypeMapping);
         }
 
         return true;
@@ -1362,26 +1388,46 @@ protected override bool TrySerializeScalarToJson(
             _ => throw new UnreachableException(),
         };
 
-        var jsonSet = _sqlExpressionFactory.Function(
-            jsonColumn.TypeMapping?.StoreType switch
-            {
-                "jsonb" => "jsonb_set",
-                "json" => "json_set",
-                _ => throw new UnreachableException()
-            },
-            arguments:
-            [
-                existingSetterValue ?? jsonColumn,
-                // Hack: Rendering of JSONPATH strings happens in value generation. We can have a special expression for modify to hold the
-                // IReadOnlyList<PathSegment> (just like Json{Scalar,Query}Expression), but instead we do the slight hack of packaging it
-                // as a constant argument; it will be unpacked and handled in SQL generation.
-                _sqlExpressionFactory.Constant(path, RelationalTypeMapping.NullMapping),
-                value
-            ],
-            nullable: true,
-            argumentsPropagateNullability: [true, true, true],
-            typeof(string),
-            jsonColumn.TypeMapping);
+        var jsonTypeMapping = jsonColumn.TypeMapping!;
+        var isNullConstant = IsJsonNull(value);
+        var isConstant = value is SqlConstantExpression;
+        var valueMayBeNull = !isNullConstant && MayReturnNull(value);
+        var supportsJsonSetLax = SupportsJsonSetLax(jsonTypeMapping);
+
+        value = isNullConstant
+            ? JsonNull(jsonTypeMapping)
+            : supportsJsonSetLax || isConstant || !valueMayBeNull
+                ? value
+                : CoalesceToJsonNull(value, jsonTypeMapping);
+
+        var useJsonSetLax = supportsJsonSetLax && (valueMayBeNull || isNullConstant);
+        var jsonSetTarget = existingSetterValue ?? jsonColumn;
+
+        // Hack: Rendering of JSONPATH strings happens in value generation. We can have a special expression for modify to hold the
+        // IReadOnlyList<PathSegment> (just like Json{Scalar,Query}Expression), but instead we do the slight hack of packaging it
+        // as a constant argument; it will be unpacked and handled in SQL generation.
+        var jsonPath = _sqlExpressionFactory.Constant(path, RelationalTypeMapping.NullMapping);
+
+        var jsonSet = useJsonSetLax
+            ? _sqlExpressionFactory.Function(
+                "jsonb_set_lax",
+                [jsonSetTarget, jsonPath, value],
+                nullable: true,
+                argumentsPropagateNullability: [true, true, false],
+                typeof(string),
+                jsonTypeMapping)
+            : _sqlExpressionFactory.Function(
+                jsonTypeMapping.StoreType switch
+                {
+                    "jsonb" => "jsonb_set",
+                    "json" => "json_set",
+                    _ => throw new UnreachableException()
+                },
+                [jsonSetTarget, jsonPath, value],
+                nullable: true,
+                argumentsPropagateNullability: [true, true, !isNullConstant],
+                typeof(string),
+                jsonTypeMapping);
 
         if (existingSetterValue is null)
         {
@@ -1394,6 +1440,54 @@ protected override bool TrySerializeScalarToJson(
         }
     }
 
+    private SqlExpression JsonNull(RelationalTypeMapping jsonTypeMapping)
+        => new SqlUnaryExpression(
+            ExpressionType.Convert,
+            _sqlExpressionFactory.Constant("null"),
+            jsonTypeMapping.ClrType,
+            jsonTypeMapping);
+
+    private SqlExpression CoalesceToJsonNull(SqlExpression value, RelationalTypeMapping jsonTypeMapping)
+        => _sqlExpressionFactory.Coalesce(value, JsonNull(jsonTypeMapping), jsonTypeMapping);
+
+    private bool SupportsJsonSetLax(RelationalTypeMapping jsonTypeMapping)
+        => jsonTypeMapping.StoreType == "jsonb" && !_isRedshift && _postgresVersion.AtLeast(16);
+
+    private static bool IsNullConstant(SqlExpression expression)
+        => expression is SqlConstantExpression { Value: null or DBNull };
+
+    private static bool IsSqlNull(SqlExpression expression)
+        => IsNullConstant(expression)
+            || expression is SqlFragmentExpression { Sql: "NULL" };
+
+    private static bool IsJsonNull(SqlExpression expression)
+        => IsSqlNull(expression)
+            || expression is SqlFunctionExpression
+            {
+                Name: "to_jsonb" or "to_json",
+                Arguments: [var argument]
+            } && IsSqlNull(argument);
+
+    private static bool NeedsJsonScalarTypeInference(SqlExpression expression)
+        => expression is SqlConstantExpression;
+
+    private static SqlExpression CastForJsonScalarTypeInference(SqlExpression expression, JsonScalarExpression target)
+        => new SqlUnaryExpression(ExpressionType.Convert, expression, typeof(object), target.TypeMapping ?? expression.TypeMapping);
+
+    private static bool MayReturnNull(SqlExpression expression)
+        => expression switch
+        {
+            SqlConstantExpression { Value: null or DBNull } => true,
+            SqlFragmentExpression { Sql: "NULL" } => true,
+            SqlParameterExpression { IsNullable: true } => true,
+            ColumnExpression { IsNullable: true } => true,
+            JsonScalarExpression { IsNullable: true } => true,
+            PgJsonTraversalExpression => true,
+            SqlFunctionExpression { IsNullable: true } => true,
+
+            _ => false
+        };
+
     #endregion ExecuteUpdate
 
     /// <summary>

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

@@ -1,3 +1,5 @@
+using Microsoft.EntityFrameworkCore.Query.Associations;
+
 namespace Microsoft.EntityFrameworkCore.Query.Associations.ComplexJson;
 
 public class ComplexJsonBulkUpdateNpgsqlTest(
@@ -47,7 +49,7 @@ public override async Task Update_property_inside_associate()
 @p='foo_updated'
 
 UPDATE "RootEntity" AS r
-SET "RequiredAssociate" = jsonb_set(r."RequiredAssociate", '{String}', to_jsonb(@p))
+SET "RequiredAssociate" = jsonb_set_lax(r."RequiredAssociate", '{String}', to_jsonb(@p))
 """);
     }
 
@@ -58,7 +60,7 @@ public override async Task Update_property_inside_associate_with_special_chars()
         AssertExecuteUpdateSql(
             """
 UPDATE "RootEntity" AS r
-SET "RequiredAssociate" = jsonb_set(r."RequiredAssociate", '{String}', to_jsonb('{ Some other/JSON:like text though it [isn''t]: ממש ממש לאéèéè }'::text))
+SET "RequiredAssociate" = jsonb_set_lax(r."RequiredAssociate", '{String}', to_jsonb('{ Some other/JSON:like text though it [isn''t]: ממש ממש לאéèéè }'::text))
 WHERE (r."RequiredAssociate" ->> 'String') = '{ this may/look:like JSON but it [isn''t]: ממש ממש לאéèéè }'
 """);
     }
@@ -72,7 +74,7 @@ public override async Task Update_property_inside_nested_associate()
 @p='foo_updated'
 
 UPDATE "RootEntity" AS r
-SET "RequiredAssociate" = jsonb_set(r."RequiredAssociate", '{RequiredNestedAssociate,String}', to_jsonb(@p))
+SET "RequiredAssociate" = jsonb_set_lax(r."RequiredAssociate", '{RequiredNestedAssociate,String}', to_jsonb(@p))
 """);
     }
 
@@ -85,7 +87,7 @@ public override async Task Update_property_on_projected_associate()
 @p='foo_updated'
 
 UPDATE "RootEntity" AS r
-SET "RequiredAssociate" = jsonb_set(r."RequiredAssociate", '{String}', to_jsonb(@p))
+SET "RequiredAssociate" = jsonb_set_lax(r."RequiredAssociate", '{String}', to_jsonb(@p))
 """);
     }
 
@@ -129,7 +131,7 @@ public override async Task Update_nested_associate_to_parameter()
 @complex_type_p='{"Id":1000,"Int":80,"Ints":[1,2,4],"Name":"Updated nested name","String":"Updated nested string"}' (DbType = Object)
 
 UPDATE "RootEntity" AS r
-SET "RequiredAssociate" = jsonb_set(r."RequiredAssociate", '{RequiredNestedAssociate}', @complex_type_p)
+SET "RequiredAssociate" = jsonb_set_lax(r."RequiredAssociate", '{RequiredNestedAssociate}', @complex_type_p)
 """);
     }
 
@@ -256,7 +258,7 @@ public override async Task Update_nested_collection_to_parameter()
 @complex_type_p='[{"Id":1000,"Int":80,"Ints":[1,2,4],"Name":"Updated nested name1","String":"Updated nested string1"},{"Id":1001,"Int":81,"Ints":[1,2,4],"Name":"Updated nested name2","String":"Updated nested string2"}]' (DbType = Object)
 
 UPDATE "RootEntity" AS r
-SET "RequiredAssociate" = jsonb_set(r."RequiredAssociate", '{NestedCollection}', @complex_type_p)
+SET "RequiredAssociate" = jsonb_set_lax(r."RequiredAssociate", '{NestedCollection}', @complex_type_p)
 """);
     }
 
@@ -278,7 +280,7 @@ public override async Task Update_nested_collection_to_another_nested_collection
         AssertExecuteUpdateSql(
             """
 UPDATE "RootEntity" AS r
-SET "RequiredAssociate" = jsonb_set(r."RequiredAssociate", '{NestedCollection}', r."OptionalAssociate" -> 'NestedCollection')
+SET "RequiredAssociate" = jsonb_set_lax(r."RequiredAssociate", '{NestedCollection}', r."OptionalAssociate" -> 'NestedCollection')
 WHERE (r."OptionalAssociate") IS NOT NULL
 """);
     }
@@ -321,7 +323,7 @@ public override async Task Update_primitive_collection_to_parameter()
 @ints='[1,2,4]' (DbType = Object)
 
 UPDATE "RootEntity" AS r
-SET "RequiredAssociate" = jsonb_set(r."RequiredAssociate", '{Ints}', @ints)
+SET "RequiredAssociate" = jsonb_set_lax(r."RequiredAssociate", '{Ints}', @ints)
 """);
     }
 
@@ -345,7 +347,7 @@ public override async Task Update_inside_primitive_collection()
 @p='99'
 
 UPDATE "RootEntity" AS r
-SET "RequiredAssociate" = jsonb_set(r."RequiredAssociate", '{Ints,1}', to_jsonb(@p))
+SET "RequiredAssociate" = jsonb_set_lax(r."RequiredAssociate", '{Ints,1}', to_jsonb(@p))
 WHERE jsonb_array_length(r."RequiredAssociate" -> 'Ints') >= 2
 """);
     }
@@ -364,7 +366,7 @@ public override async Task Update_multiple_properties_inside_same_associate()
 @p1='20'
 
 UPDATE "RootEntity" AS r
-SET "RequiredAssociate" = jsonb_set(jsonb_set(r."RequiredAssociate", '{String}', to_jsonb(@p)), '{Int}', to_jsonb(@p1))
+SET "RequiredAssociate" = jsonb_set_lax(jsonb_set_lax(r."RequiredAssociate", '{String}', to_jsonb(@p)), '{Int}', to_jsonb(@p1))
 """);
     }
 
@@ -378,8 +380,8 @@ public override async Task Update_multiple_properties_inside_associates_and_on_e
 
 UPDATE "RootEntity" AS r
 SET "Name" = r."Name" || 'Modified',
-    "RequiredAssociate" = jsonb_set(r."RequiredAssociate", '{String}', r."OptionalAssociate" -> 'String'),
-    "OptionalAssociate" = jsonb_set(r."OptionalAssociate", '{RequiredNestedAssociate,String}', to_jsonb(@p))
+    "RequiredAssociate" = jsonb_set_lax(r."RequiredAssociate", '{String}', r."OptionalAssociate" -> 'String'),
+    "OptionalAssociate" = jsonb_set_lax(r."OptionalAssociate", '{RequiredNestedAssociate,String}', to_jsonb(@p))
 WHERE (r."OptionalAssociate") IS NOT NULL
 """);
     }
@@ -393,8 +395,8 @@ public override async Task Update_multiple_projected_associates_via_anonymous_ty
 @p='foo_updated'
 
 UPDATE "RootEntity" AS r
-SET "RequiredAssociate" = jsonb_set(r."RequiredAssociate", '{String}', r."OptionalAssociate" -> 'String'),
-    "OptionalAssociate" = jsonb_set(r."OptionalAssociate", '{String}', to_jsonb(@p))
+SET "RequiredAssociate" = jsonb_set_lax(r."RequiredAssociate", '{String}', r."OptionalAssociate" -> 'String'),
+    "OptionalAssociate" = jsonb_set_lax(r."OptionalAssociate", '{String}', to_jsonb(@p))
 WHERE (r."OptionalAssociate") IS NOT NULL
 """);
     }

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

@@ -85,7 +85,7 @@ public override async Task ExecuteUpdate_within_json_to_parameter()
 @Fixture_OtherValue='False'
 
 UPDATE "JsonTypeEntity" AS j
-SET "JsonContainer" = jsonb_set(j."JsonContainer", '{Value}', to_jsonb(@Fixture_OtherValue))
+SET "JsonContainer" = jsonb_set_lax(j."JsonContainer", '{Value}', to_jsonb(@Fixture_OtherValue))
 """);
     }
 
@@ -96,7 +96,7 @@ public override async Task ExecuteUpdate_within_json_to_constant()
         AssertSql(
             """
 UPDATE "JsonTypeEntity" AS j
-SET "JsonContainer" = jsonb_set(j."JsonContainer", '{Value}', to_jsonb(FALSE::boolean))
+SET "JsonContainer" = jsonb_set_lax(j."JsonContainer", '{Value}', to_jsonb(FALSE::boolean))
 """);
     }
 
@@ -118,7 +118,7 @@ public override async Task ExecuteUpdate_within_json_to_nonjson_column()
         AssertSql(
             """
 UPDATE "JsonTypeEntity" AS j
-SET "JsonContainer" = jsonb_set(j."JsonContainer", '{Value}', to_jsonb(j."OtherValue"))
+SET "JsonContainer" = jsonb_set_lax(j."JsonContainer", '{Value}', to_jsonb(j."OtherValue"))
 """);
     }
 

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

@@ -85,7 +85,7 @@ public override async Task ExecuteUpdate_within_json_to_parameter()
 @Fixture_OtherValue='0x04050607'
 
 UPDATE "JsonTypeEntity" AS j
-SET "JsonContainer" = jsonb_set(j."JsonContainer", '{Value}', to_jsonb(encode(@Fixture_OtherValue, 'base64')))
+SET "JsonContainer" = jsonb_set_lax(j."JsonContainer", '{Value}', to_jsonb(encode(@Fixture_OtherValue, 'base64')))
 """);
     }
 
@@ -96,7 +96,7 @@ public override async Task ExecuteUpdate_within_json_to_constant()
         AssertSql(
             """
 UPDATE "JsonTypeEntity" AS j
-SET "JsonContainer" = jsonb_set(j."JsonContainer", '{Value}', to_jsonb(encode(BYTEA E'\\x04050607', 'base64')))
+SET "JsonContainer" = jsonb_set_lax(j."JsonContainer", '{Value}', to_jsonb(encode(BYTEA E'\\x04050607', 'base64')))
 """);
     }
 
@@ -107,7 +107,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(encode(decode(j."JsonContainer" ->> 'OtherValue', 'base64'), 'base64')))
+SET "JsonContainer" = jsonb_set_lax(j."JsonContainer", '{Value}', to_jsonb(encode(decode(j."JsonContainer" ->> 'OtherValue', 'base64'), 'base64')))
 """);
     }
 
@@ -118,7 +118,7 @@ public override async Task ExecuteUpdate_within_json_to_nonjson_column()
         AssertSql(
             """
 UPDATE "JsonTypeEntity" AS j
-SET "JsonContainer" = jsonb_set(j."JsonContainer", '{Value}', to_jsonb(encode(j."OtherValue", 'base64')))
+SET "JsonContainer" = jsonb_set_lax(j."JsonContainer", '{Value}', to_jsonb(encode(j."OtherValue", 'base64')))
 """);
     }