Fix JSON null partial updates

Use jsonb_set_lax for JSONB partial updates when targeting PostgreSQL 16 or later so SQL null values are written as JSON null rather than nulling the whole document. Keep older PostgreSQL versions on jsonb_set/json_set with a JSON null fallback for nullable update values.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: roji

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"),
+            typeof(object),
+            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')))
 """);
     }