Skip to content

Commit

Permalink
Fix to #28845 - JSON: add support for partial update to update just a…
Browse files Browse the repository at this point in the history 
… single scalar property, rather than entire entities

Optimization for partial updates - when only one property is modified on the entity we can change it directly, rather than rewriting the JSON element for the entire entity.
To avoid string formatting issues we construct faux JSON element and insert the value that is to be the replacement, and then in the update statement we extract the value using JSON_VALUE.
In case of numeric or bool we need an additional convert statement around the value.

Fixes #28845
  • Loading branch information
@maumar
maumar committed Aug 24, 2022
1 parent bbde5fe commit b526cb0
Show file tree
Hide file tree
Showing 15 changed files with 1,130 additions and 56 deletions.
4 changes: 4 additions & 0 deletions src/EFCore.Relational/Update/ColumnModification.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ public ColumnModification(in ColumnModificationParameters columnModificationPara
_generateParameterName = columnModificationParameters.GenerateParameterName;
Entry = columnModificationParameters.Entry;
JsonPath = columnModificationParameters.JsonPath;
JsonScalarProperty = columnModificationParameters.JsonScalarProperty;

UseParameter = _generateParameterName != null;
}
Expand Down Expand Up @@ -176,6 +177,9 @@ public virtual object? Value
/// <inheritdoc />
public virtual string? JsonPath { get; }

/// <inheritdoc />
public virtual IProperty? JsonScalarProperty { get; }

/// <inheritdoc />
public virtual void AddSharedColumnModification(IColumnModification modification)
{
Expand Down
13 changes: 12 additions & 1 deletion src/EFCore.Relational/Update/ColumnModificationParameters.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,11 @@ public readonly record struct ColumnModificationParameters
/// </summary>
public string? JsonPath { get; init; }

/// <summary>
/// In case of JSON column single scalar property modification, the scalar property that is being modified.
/// </summary>
public IProperty? JsonScalarProperty { get; init; }

/// <summary>
/// Creates a new <see cref="ColumnModificationParameters" /> instance.
/// </summary>
Expand Down Expand Up @@ -143,6 +148,7 @@ public ColumnModificationParameters(
GenerateParameterName = null;
Entry = null;
JsonPath = null;
JsonScalarProperty = null;
}

/// <summary>
Expand Down Expand Up @@ -189,6 +195,7 @@ public ColumnModificationParameters(
GenerateParameterName = null;
Entry = null;
JsonPath = null;
JsonScalarProperty = null;
}

/// <summary>
Expand Down Expand Up @@ -233,6 +240,7 @@ public ColumnModificationParameters(
GenerateParameterName = generateParameterName;
Entry = entry;
JsonPath = null;
JsonScalarProperty = null;
}

/// <summary>
Expand All @@ -242,6 +250,7 @@ public ColumnModificationParameters(
/// <param name="originalValue">The original value of the property mapped to this column.</param>
/// <param name="value">The current value of the property mapped to this column.</param>
/// <param name="jsonPath">The JSON path leading to the JSON element that needs to be updated.</param>
/// <param name="jsonScalarProperty">In case of JSON column single scalar property modification, the scalar property that is being modified.</param>
/// <param name="columnType">The database type of the column.</param>
/// <param name="typeMapping">The relational type mapping to be used for the command parameter.</param>
/// <param name="read">Indicates whether a value must be read from the database for the column.</param>
Expand All @@ -257,6 +266,7 @@ public ColumnModificationParameters(
string? columnType,
RelationalTypeMapping? typeMapping,
string jsonPath,
IProperty? jsonScalarProperty,
bool read,
bool write,
bool key,
Expand All @@ -271,7 +281,6 @@ public ColumnModificationParameters(
Property = null;
ColumnType = columnType;
TypeMapping = typeMapping;
JsonPath = jsonPath;
IsRead = read;
IsWrite = write;
IsKey = key;
Expand All @@ -281,5 +290,7 @@ public ColumnModificationParameters(

GenerateParameterName = null;
Entry = null;
JsonPath = jsonPath;
JsonScalarProperty = jsonScalarProperty;
}
}
5 changes: 5 additions & 0 deletions src/EFCore.Relational/Update/IColumnModification.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,11 @@ public interface IColumnModification
/// </summary>
public string? JsonPath { get; }

/// <summary>
/// In case of JSON column single scalar property modification, the scalar property that is being modified.
/// </summary>
public IProperty? JsonScalarProperty { get; }

/// <summary>
/// Adds a modification affecting the same database value.
/// </summary>
Expand Down
121 changes: 75 additions & 46 deletions src/EFCore.Relational/Update/ModificationCommand.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System.Collections;
using System.Data;
using System.Text.Json;
using System.Text.Json.Nodes;
using Microsoft.EntityFrameworkCore.ChangeTracking.Internal;
using Microsoft.EntityFrameworkCore.Internal;
Expand Down Expand Up @@ -251,6 +252,13 @@ public virtual IColumnModification AddColumnModification(in ColumnModificationPa
protected virtual IColumnModification CreateColumnModification(in ColumnModificationParameters columnModificationParameters)
=> new ColumnModification(columnModificationParameters);

private sealed class JsonPartialUpdateInfo
{
public List<JsonPartialUpdatePathEntry> Path { get; } = new();
public IProperty? Property { get; set; }
public object? PropertyValue { get; set; }
}

private record struct JsonPartialUpdatePathEntry
{
public JsonPartialUpdatePathEntry(
Expand Down Expand Up @@ -322,22 +330,33 @@ private List<IColumnModification> GenerateColumnModifications()

if (jsonEntry)
{
var jsonColumnsUpdateMap = new Dictionary<string, List<JsonPartialUpdatePathEntry>>();
var jsonColumnsUpdateMap = new Dictionary<string, JsonPartialUpdateInfo>();
var processedEntries = new List<IUpdateEntry>();
foreach (var entry in _entries.Where(e => e.EntityType.IsMappedToJson()))
{
var modifiedMembers = entry.ToEntityEntry().Members.Where(m => m is not NavigationEntry && m.IsModified).ToList();
var jsonColumn = entry.EntityType.GetContainerColumnName()!;
var jsonPartialUpdateInfo = FindJsonPartialUpdateInfo(entry, processedEntries);
processedEntries.Add(entry);

if (jsonPartialUpdateInfo == null)
{
// this entry is a subtree of an entry that we already processed
// so we already need to update the parent - no need to have extra entry for the subtree
processedEntries.Add(entry);
continue;
}

if (modifiedMembers.Count == 1)
{
jsonPartialUpdateInfo.Property = (IProperty)modifiedMembers.Single().Metadata;
jsonPartialUpdateInfo.PropertyValue = entry.GetCurrentProviderValue(jsonPartialUpdateInfo.Property);
}
else
{
// only add to processed entries list if we are planning to update the entire entity (rather than just a single property on that entity)
processedEntries.Add(entry);
}

if (jsonColumnsUpdateMap.TryGetValue(jsonColumn, out var currentJsonPartialUpdateInfo))
{
jsonPartialUpdateInfo = FindCommonJsonPartialUpdateInfo(
Expand All @@ -348,47 +367,56 @@ private List<IColumnModification> GenerateColumnModifications()
jsonColumnsUpdateMap[jsonColumn] = jsonPartialUpdateInfo;
}

foreach (var (jsonColumnName, updatePath) in jsonColumnsUpdateMap)
foreach (var (jsonColumnName, updateInfo) in jsonColumnsUpdateMap)
{
var finalUpdatePathElement = updatePath.Last();
var finalUpdatePathElement = updateInfo.Path.Last();
var navigation = finalUpdatePathElement.Navigation;

var jsonColumnTypeMapping = navigation.TargetEntityType.GetContainerColumnTypeMapping()!;
var navigationValue = finalUpdatePathElement.ParentEntry.GetCurrentValue(navigation);

var json = default(JsonNode?);
if (finalUpdatePathElement.Ordinal != null && navigationValue != null)
var jsonPathString = string.Join(
".", updateInfo.Path.Select(x => x.PropertyName + (x.Ordinal != null ? "[" + x.Ordinal + "]" : "")));

if (updateInfo.Property != null)
{
int i = 0;
foreach (var navigationValueElement in (IEnumerable)navigationValue)
json = new JsonObject();
json["v"] = JsonValue.Create(updateInfo.PropertyValue);
jsonPathString = jsonPathString + "." + updateInfo.Property.GetJsonPropertyName();
}
else
{
if (finalUpdatePathElement.Ordinal != null && navigationValue != null)
{
if (i == finalUpdatePathElement.Ordinal)
var i = 0;
foreach (var navigationValueElement in (IEnumerable)navigationValue)
{
json = CreateJson(
navigationValueElement,
finalUpdatePathElement.ParentEntry,
navigation.TargetEntityType,
ordinal: null,
isCollection: false);

break;
if (i == finalUpdatePathElement.Ordinal)
{
json = CreateJson(
navigationValueElement,
finalUpdatePathElement.ParentEntry,
navigation.TargetEntityType,
ordinal: null,
isCollection: false);

break;
}

i++;
}

i++;
}
else
{
json = CreateJson(
navigationValue,
finalUpdatePathElement.ParentEntry,
navigation.TargetEntityType,
ordinal: null,
isCollection: navigation.IsCollection);
}
}
else
{
json = CreateJson(
navigationValue,
finalUpdatePathElement.ParentEntry,
navigation.TargetEntityType,
ordinal: null,
isCollection: navigation.IsCollection);
}

var jsonPathString = string.Join(
".", updatePath.Select(x => x.PropertyName + (x.Ordinal != null ? "[" + x.Ordinal + "]" : "")));

var columnModificationParameters = new ColumnModificationParameters(
jsonColumnName,
Expand All @@ -397,6 +425,7 @@ private List<IColumnModification> GenerateColumnModifications()
columnType: jsonColumnTypeMapping.StoreType,
jsonColumnTypeMapping,
jsonPath: jsonPathString,
jsonScalarProperty: updateInfo.Property,
read: false,
write: true,
key: false,
Expand Down Expand Up @@ -582,9 +611,9 @@ entry.EntityState is EntityState.Modified or EntityState.Added

return columnModifications;

static List<JsonPartialUpdatePathEntry>? FindJsonPartialUpdateInfo(IUpdateEntry entry, List<IUpdateEntry> processedEntries)
static JsonPartialUpdateInfo? FindJsonPartialUpdateInfo(IUpdateEntry entry, List<IUpdateEntry> processedEntries)
{
var result = new List<JsonPartialUpdatePathEntry>();
var result = new JsonPartialUpdateInfo();
var currentEntry = entry;
var currentOwnership = currentEntry.EntityType.FindOwnership()!;

Expand Down Expand Up @@ -617,7 +646,7 @@ entry.EntityState is EntityState.Modified or EntityState.Added
currentEntry,
currentOwnership.GetNavigation(pointsToPrincipal: false)!);

result.Insert(0, pathEntry);
result.Path.Insert(0, pathEntry);
}

// parent entity got deleted, no need to do any json-specific processing
Expand All @@ -629,36 +658,36 @@ entry.EntityState is EntityState.Modified or EntityState.Added
return result;
}

static List<JsonPartialUpdatePathEntry> FindCommonJsonPartialUpdateInfo(
List<JsonPartialUpdatePathEntry> first,
List<JsonPartialUpdatePathEntry> second)
static JsonPartialUpdateInfo FindCommonJsonPartialUpdateInfo(
JsonPartialUpdateInfo first,
JsonPartialUpdateInfo second)
{
var result = new List<JsonPartialUpdatePathEntry>();
for (var i = 0; i < Math.Min(first.Count, second.Count); i++)
var result = new JsonPartialUpdateInfo();
for (var i = 0; i < Math.Min(first.Path.Count, second.Path.Count); i++)
{
if (first[i].PropertyName == second[i].PropertyName)
if (first.Path[i].PropertyName == second.Path[i].PropertyName)
{
if (first[i].Ordinal == second[i].Ordinal)
if (first.Path[i].Ordinal == second.Path[i].Ordinal)
{
result.Add(first[i]);
result.Path.Add(first.Path[i]);
continue;
}
else
{
var common = new JsonPartialUpdatePathEntry(
first[i].PropertyName,
first.Path[i].PropertyName,
null,
first[i].ParentEntry,
first[i].Navigation);
first.Path[i].ParentEntry,
first.Path[i].Navigation);

result.Add(common);
result.Path.Add(common);
}

break;
}
}

Debug.Assert(result.Count > 0, "Common denominator should always have at least one node - the root.");
Debug.Assert(result.Path.Count > 0, "Common denominator should always have at least one node - the root.");

return result;
}
Expand Down
35 changes: 32 additions & 3 deletions src/EFCore.SqlServer/Update/Internal/SqlServerUpdateSqlGenerator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System.Data;
using System.Globalization;
using System.Text;
using Microsoft.Extensions.Primitives;

namespace Microsoft.EntityFrameworkCore.SqlServer.Update.Internal;

Expand Down Expand Up @@ -152,9 +153,37 @@ protected override void AppendUpdateColumnValue(
// using strict so that we don't remove json elements when they are assigned NULL value
stringBuilder.Append(", 'strict ");
stringBuilder.Append(columnModification.JsonPath);
stringBuilder.Append("', JSON_QUERY(");
base.AppendUpdateColumnValue(updateSqlGeneratorHelper, columnModification, stringBuilder, name, schema);
stringBuilder.Append("))");
stringBuilder.Append("', ");

if (columnModification.JsonScalarProperty != null)
{
var needsTypeConversion = columnModification.JsonScalarProperty.ClrType.IsNumeric()
|| columnModification.JsonScalarProperty.ClrType == typeof(bool);

if (needsTypeConversion)
{
stringBuilder.Append("CAST(");
}

stringBuilder.Append("JSON_VALUE(");
base.AppendUpdateColumnValue(updateSqlGeneratorHelper, columnModification, stringBuilder, name, schema);
stringBuilder.Append(", '$.v')");

if (needsTypeConversion)
{
stringBuilder.Append(" AS ");
stringBuilder.Append(columnModification.JsonScalarProperty.GetRelationalTypeMapping().StoreType);
stringBuilder.Append(")");
}
}
else
{
stringBuilder.Append("JSON_QUERY(");
base.AppendUpdateColumnValue(updateSqlGeneratorHelper, columnModification, stringBuilder, name, schema);
stringBuilder.Append(")");
}

stringBuilder.Append(")");
}
else
{
Expand Down
Loading

0 comments on commit b526cb0

Please sign in to comment.