Skip to content

Commit b931a3d

Browse files
authored
Use our new JSON serialization infra for partial updates (#31232)
In addition, when the database's partial update function (e.g. JSON_MODIFY) accepts a non-JSON relational value (e.g. int), simply send that value via the property's regular type mapping.
1 parent bb1fb3c commit b931a3d

File tree

7 files changed

+225
-213
lines changed

7 files changed

+225
-213
lines changed

src/EFCore.Relational/Update/ModificationCommand.cs

Lines changed: 79 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -325,11 +325,24 @@ private List<IColumnModification> GenerateColumnModifications()
325325
var jsonColumnTypeMapping = jsonColumn.StoreTypeMapping;
326326
var navigationValue = finalUpdatePathElement.ParentEntry.GetCurrentValue(navigation);
327327
var jsonPathString = string.Join(".", updateInfo.Path.Select(x => x.PropertyName + (x.Ordinal != null ? "[" + x.Ordinal + "]" : "")));
328-
object? value;
329-
if (updateInfo.Property is not null)
328+
if (updateInfo.Property is IProperty property)
330329
{
331-
value = GenerateValueForSinglePropertyUpdate(updateInfo.Property, updateInfo.PropertyValue);
332-
jsonPathString = jsonPathString + "." + updateInfo.Property.GetJsonPropertyName();
330+
var columnModificationParameters = new ColumnModificationParameters(
331+
jsonColumn.Name,
332+
value: updateInfo.PropertyValue,
333+
property: property,
334+
columnType: jsonColumnTypeMapping.StoreType,
335+
jsonColumnTypeMapping,
336+
jsonPath: jsonPathString + "." + updateInfo.Property.GetJsonPropertyName(),
337+
read: false,
338+
write: true,
339+
key: false,
340+
condition: false,
341+
_sensitiveLoggingEnabled) { GenerateParameterName = _generateParameterName };
342+
343+
ProcessSinglePropertyJsonUpdate(ref columnModificationParameters);
344+
345+
columnModifications.Add(new ColumnModification(columnModificationParameters));
333346
}
334347
else
335348
{
@@ -371,26 +384,24 @@ private List<IColumnModification> GenerateColumnModifications()
371384

372385
writer.Flush();
373386

374-
value = writer.BytesCommitted > 0
387+
var value = writer.BytesCommitted > 0
375388
? Encoding.UTF8.GetString(stream.ToArray())
376389
: null;
377-
}
378390

379-
var columnModificationParameters = new ColumnModificationParameters(
380-
jsonColumn.Name,
381-
value: value,
382-
property: updateInfo.Property,
383-
columnType: jsonColumnTypeMapping.StoreType,
384-
jsonColumnTypeMapping,
385-
jsonPath: jsonPathString,
386-
read: false,
387-
write: true,
388-
key: false,
389-
condition: false,
390-
_sensitiveLoggingEnabled)
391-
{ GenerateParameterName = _generateParameterName, };
392-
393-
columnModifications.Add(new ColumnModification(columnModificationParameters));
391+
columnModifications.Add(new ColumnModification(new ColumnModificationParameters(
392+
jsonColumn.Name,
393+
value: value,
394+
property: updateInfo.Property,
395+
columnType: jsonColumnTypeMapping.StoreType,
396+
jsonColumnTypeMapping,
397+
jsonPath: jsonPathString,
398+
read: false,
399+
write: true,
400+
key: false,
401+
condition: false,
402+
_sensitiveLoggingEnabled)
403+
{ GenerateParameterName = _generateParameterName }));
404+
}
394405
}
395406
}
396407

@@ -659,7 +670,7 @@ void HandleColumnModification(IColumnMappingBase columnMapping)
659670
if (modifiedMembers.Count == 1)
660671
{
661672
result.Property = modifiedMembers.Single().Metadata;
662-
result.PropertyValue = entry.GetCurrentProviderValue(result.Property);
673+
result.PropertyValue = entry.GetCurrentValue(result.Property);
663674
}
664675
else
665676
{
@@ -711,22 +722,56 @@ static JsonPartialUpdateInfo FindCommonJsonPartialUpdateInfo(
711722
}
712723

713724
/// <summary>
714-
/// Generates value to use for update in case a single property is being updated.
725+
/// Performs processing specifically needed for column modifications that correspond to single-property JSON updates.
715726
/// </summary>
716-
/// <param name="property">Property to be updated.</param>
717-
/// <param name="propertyValue">Value object that the property will be updated to.</param>
718-
/// <returns>Value that the property will be updated to.</returns>
719-
[EntityFrameworkInternal]
720-
protected virtual object? GenerateValueForSinglePropertyUpdate(IProperty property, object? propertyValue)
727+
/// <remarks>
728+
/// By default, strings, numeric types and bool and sent as a regular relational parameter, since database functions responsible for
729+
/// patching JSON documents support this. Other types get converted to JSON via the normal means and sent as a string parameter.
730+
/// </remarks>
731+
protected virtual void ProcessSinglePropertyJsonUpdate(ref ColumnModificationParameters parameters)
721732
{
733+
var property = parameters.Property!;
722734
var propertyProviderClrType = (property.GetTypeMapping().Converter?.ProviderClrType ?? property.ClrType).UnwrapNullableType();
723735

724-
return (propertyProviderClrType == typeof(DateTime)
725-
|| propertyProviderClrType == typeof(DateTimeOffset)
726-
|| propertyProviderClrType == typeof(TimeSpan)
727-
|| propertyProviderClrType == typeof(Guid))
728-
? JsonValue.Create(propertyValue)?.ToJsonString().Replace("\"", "")
729-
: propertyValue;
736+
// On most databases, the function which patches a JSON document (e.g. SQL Server JSON_MODIFY) accepts relational string, numeric
737+
// and bool types directly, without serializing it to a JSON string. So by default, for those cases simply return the value as-is,
738+
// with the property's type mapping which will take care of sending the parameter with the relational value.
739+
// Note that we haven't yet applied a value converter if one is configured, in order to allow for it to get applied later with
740+
// the regular parameter flow.
741+
if (propertyProviderClrType == typeof(string)
742+
|| propertyProviderClrType == typeof(bool)
743+
|| propertyProviderClrType.IsNumeric())
744+
{
745+
parameters = parameters with { TypeMapping = property.GetRelationalTypeMapping() };
746+
return;
747+
}
748+
749+
// Other, non-JSON-native types need to be serialized to a JSON string.
750+
751+
// First, apply value conversion to get the provider value
752+
var value = property.GetTypeMapping().Converter is ValueConverter converter
753+
? converter.ConvertToProvider(parameters.Value)
754+
: parameters.Value;
755+
756+
var stream = new MemoryStream();
757+
var writer = new Utf8JsonWriter(stream, new JsonWriterOptions { Indented = false });
758+
759+
if (value is null)
760+
{
761+
writer.WriteNullValue();
762+
}
763+
else
764+
{
765+
property.GetJsonValueReaderWriter()!.ToJson(writer, value);
766+
}
767+
768+
writer.Flush();
769+
770+
// The JSON string contains enclosing quotes (JSON string representation), remove these.
771+
parameters = parameters with
772+
{
773+
Value = Encoding.UTF8.GetString(stream.ToArray())[1..^1]
774+
};
730775
}
731776

732777
private void WriteJson(

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

Lines changed: 0 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -34,34 +34,4 @@ public SqlServerModificationCommand(in NonTrackedModificationCommandParameters m
3434
: base(modificationCommandParameters)
3535
{
3636
}
37-
38-
/// <summary>
39-
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
40-
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
41-
/// any release. You should only use it directly in your code with extreme caution and knowing that
42-
/// doing so can result in application failures when updating to a new Entity Framework Core release.
43-
/// </summary>
44-
protected override object? GenerateValueForSinglePropertyUpdate(IProperty property, object? propertyValue)
45-
{
46-
var propertyProviderClrType = (property.GetTypeMapping().Converter?.ProviderClrType ?? property.ClrType).UnwrapNullableType();
47-
48-
// when we generate SqlParameter when updating single property in JSON entity
49-
// we always use SqlServerJsonTypeMapping as type mapping for the parameter
50-
// (since we don't have dedicated type mapping for individual JSON properties)
51-
// later, when we generate DbParameter we assign the value and then the DbType from the type mapping.
52-
// in case of byte value, when we assign the value to the DbParameter it sets its type to Byte and its size to 1
53-
// then, we change DbType to String, but keep size as is
54-
// so, if value was, say, 15 we initially generate DbParameter of type Byte, value 25 and size 1
55-
// but when we change the type we end up with type String, value 25 and size 1, which effectively is "2"
56-
// to mitigate this, we convert the value to string, to guarantee the correct parameter size.
57-
// this can be removed when we have dedicated JSON type mapping for individual (leaf) properties
58-
if (propertyProviderClrType == typeof(byte))
59-
{
60-
return JsonValue.Create(propertyValue)?.ToJsonString().Replace("\"", "");
61-
}
62-
63-
#pragma warning disable EF1001 // Internal EF Core API usage.
64-
return base.GenerateValueForSinglePropertyUpdate(property, propertyValue);
65-
#pragma warning restore EF1001 // Internal EF Core API usage.
66-
}
6737
}

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

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -154,24 +154,7 @@ protected override void AppendUpdateColumnValue(
154154

155155
if (columnModification.Property != null)
156156
{
157-
var propertyClrType = columnModification.Property.GetTypeMapping().Converter?.ProviderClrType
158-
?? columnModification.Property.ClrType;
159-
160-
var needsTypeConversion = propertyClrType.IsNumeric() || propertyClrType == typeof(bool);
161-
162-
if (needsTypeConversion)
163-
{
164-
stringBuilder.Append("CAST(");
165-
}
166-
167157
base.AppendUpdateColumnValue(updateSqlGeneratorHelper, columnModification, stringBuilder, name, schema);
168-
169-
if (needsTypeConversion)
170-
{
171-
stringBuilder.Append(" AS ");
172-
stringBuilder.Append(columnModification.Property.GetRelationalTypeMapping().StoreType);
173-
stringBuilder.Append(")");
174-
}
175158
}
176159
else
177160
{

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

Lines changed: 24 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -39,22 +39,37 @@ public SqliteModificationCommand(in NonTrackedModificationCommandParameters modi
3939
/// any release. You should only use it directly in your code with extreme caution and knowing that
4040
/// doing so can result in application failures when updating to a new Entity Framework Core release.
4141
/// </summary>
42-
protected override object? GenerateValueForSinglePropertyUpdate(IProperty property, object? propertyValue)
42+
protected override void ProcessSinglePropertyJsonUpdate(ref ColumnModificationParameters parameters)
4343
{
44+
var property = parameters.Property!;
45+
4446
var propertyProviderClrType = (property.GetTypeMapping().Converter?.ProviderClrType ?? property.ClrType).UnwrapNullableType();
4547

46-
if (propertyProviderClrType == typeof(bool) && propertyValue is bool boolPropertyValue)
48+
// SQLite has no bool type, so if we simply sent the bool as-is, we'd get 1/0 in the JSON document.
49+
// To get an actual unquoted true/false value, we pass "true"/"false" string through the json() minifier, which does this.
50+
// See https://sqlite.org/forum/info/91d09974c3754ea6.
51+
// Here we convert the .NET bool to a "true"/"false" string, and SqliteUpdateSqlGenerator will add the enclosing json().
52+
if (propertyProviderClrType == typeof(bool))
4753
{
48-
// Sqlite converts true/false into native 0/1 when using json_extract
49-
// so we convert those values to strings so that they stay as true/false
50-
// which is what we want to store in json object in the end
51-
return boolPropertyValue
52-
? "true"
53-
: "false";
54+
var value = property.GetTypeMapping().Converter is ValueConverter converter
55+
? converter.ConvertToProvider(parameters.Value)
56+
: parameters.Value;
57+
58+
parameters = parameters with
59+
{
60+
Value = value switch
61+
{
62+
true => "true",
63+
false => "false",
64+
_ => throw new Exception("IMPOSSIBLE")
65+
}
66+
};
67+
68+
return;
5469
}
5570

5671
#pragma warning disable EF1001 // Internal EF Core API usage.
57-
return base.GenerateValueForSinglePropertyUpdate(property, propertyValue);
72+
base.ProcessSinglePropertyJsonUpdate(ref parameters);
5873
#pragma warning restore EF1001 // Internal EF Core API usage.
5974
}
6075
}

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

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -165,14 +165,13 @@ protected override void AppendUpdateColumnValue(
165165
var providerClrType = (columnModification.Property.GetTypeMapping().Converter?.ProviderClrType
166166
?? columnModification.Property.ClrType).UnwrapNullableType();
167167

168-
// special handling for bool
169-
// json_extract converts true/false into native 0/1 values,
170-
// but we want to store the values as true/false in JSON
171-
// in order to do that we modify the parameter value to "true"/"false"
172-
// and wrap json() function around it to avoid conversion to 0/1
168+
// SQLite has no bool type, so if we simply sent the bool as-is, we'd get 1/0 in the JSON document.
169+
// To get an actual unquoted true/false value, we pass "true"/"false" string through the json() minifier, which does this.
170+
// See https://sqlite.org/forum/info/91d09974c3754ea6.
171+
// SqliteModificationCommand converted the .NET bool to a "true"/"false" string, here we add the enclosing json().
173172
//
174-
// for decimal, sqlite generates string parameter for decimal values
175-
// but don't want to store the values as strings, we use json to "unwrap" it
173+
// For decimal, Microsoft.Data.Sqlite maps decimal to TEXT instead of to REAL, but we don't want decimals to be stored as
174+
// strings inside JSON. So we again use json() to "unwrap" it.
176175
if (providerClrType == typeof(bool) || providerClrType == typeof(decimal))
177176
{
178177
stringBuilder.Append("json(");

0 commit comments

Comments
 (0)