Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix to #28845 - JSON: add support for partial update to update just a single scalar property, rather than entire entities #28862

Merged
merged 1 commit into from
Aug 25, 2022
Merged

Fix to #28845 - JSON: add support for partial update to update just a single scalar property, rather than entire entities #28862

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 10 additions & 10 deletions src/EFCore.Relational/Update/ColumnModificationParameters.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,14 +236,14 @@ public ColumnModificationParameters(
}

/// <summary>
/// Creates a new <see cref="ColumnModificationParameters" /> instance.
/// Creates a new <see cref="ColumnModificationParameters" /> instance specific for updating objects mapped to JSON column.
/// </summary>
/// <param name="columnName">The name of the column.</param>
/// <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="columnType">The database type of the column.</param>
/// <param name="columnName">The name of the JSON column.</param>
/// <param name="value">The current value of the JSON element located at the given JSON path.</param>
/// <param name="property">In case of JSON column single scalar property modification, the scalar property that is being modified, null otherwise.</param>
/// <param name="columnType">The database type of the JSON column.</param>
/// <param name="typeMapping">The relational type mapping to be used for the command parameter.</param>
/// <param name="jsonPath">The JSON path leading to the JSON element that needs to be updated.</param>
/// <param name="read">Indicates whether a value must be read from the database for the column.</param>
/// <param name="write">Indicates whether a value must be written to the database for the column.</param>
/// <param name="key">Indicates whether the column part of a primary or alternate key.</param>
Expand All @@ -252,8 +252,8 @@ public ColumnModificationParameters(
/// <param name="isNullable">A value indicating whether the value could be null.</param>
public ColumnModificationParameters(
string columnName,
object? originalValue,
object? value,
IProperty? property,
string? columnType,
RelationalTypeMapping? typeMapping,
string jsonPath,
Expand All @@ -266,12 +266,11 @@ public ColumnModificationParameters(
{
Column = null;
ColumnName = columnName;
OriginalValue = originalValue;
OriginalValue = null;
Value = value;
Property = null;
Property = property;
ColumnType = columnType;
TypeMapping = typeMapping;
JsonPath = jsonPath;
IsRead = read;
IsWrite = write;
IsKey = key;
Expand All @@ -281,5 +280,6 @@ public ColumnModificationParameters(

GenerateParameterName = null;
Entry = null;
JsonPath = jsonPath;
}
}
3 changes: 3 additions & 0 deletions src/EFCore.Relational/Update/IColumnModification.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ public interface IColumnModification
/// <summary>
/// The property that maps to the column.
/// </summary>
/// <remarks>
/// In case of JSON column single scalar property modification, the scalar property that is being modified.
/// </remarks>
public IProperty? Property { get; }

/// <summary>
Expand Down
126 changes: 76 additions & 50 deletions src/EFCore.Relational/Update/ModificationCommand.cs
View file
Open in desktop
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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
private sealed class JsonPartialUpdateInfo
private struct JsonPartialUpdateInfo

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm modifying Property and PropertyValue later and also can return null JsonPartialUpdateInfo from FindJsonPartialUpdateInfo, so I find it more convenient to keep as class. Will change to struct if you feel strongly about it tho.

{
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,19 +330,16 @@ 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 modifiedMembers = entry.ToEntityEntry().Properties.Where(m => 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
continue;
}

Expand All @@ -348,52 +353,60 @@ 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 JsonArray(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,
originalValue: null,
value: json?.ToJsonString(),
property: updateInfo.Property,
columnType: jsonColumnTypeMapping.StoreType,
jsonColumnTypeMapping,
jsonPath: jsonPathString,
Expand Down Expand Up @@ -582,9 +595,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 +630,20 @@ entry.EntityState is EntityState.Modified or EntityState.Added
currentEntry,
currentOwnership.GetNavigation(pointsToPrincipal: false)!);

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

var modifiedMembers = entry.ToEntityEntry().Properties.Where(m => m.IsModified).ToList();
if (modifiedMembers.Count == 1)
{
result.Property = modifiedMembers.Single().Metadata;
result.PropertyValue = entry.GetCurrentProviderValue(result.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);
}

// parent entity got deleted, no need to do any json-specific processing
Expand All @@ -629,36 +655,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
Open in desktop
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.Property != null)
{
var needsTypeConversion = columnModification.Property.ClrType.IsNumeric()
|| columnModification.Property.ClrType == typeof(bool);

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

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

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

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