Skip to content

Complex json change tracking #36446

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

Merged
roji merged 1 commit into from
Jul 28, 2025
Merged

Complex json change tracking #36446

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
345 changes: 190 additions & 155 deletions ...uery/RelationalShapedQueryCompilingExpressionVisitor.ShaperProcessingExpressionVisitor.cs
View file
Open in desktop

Large diffs are not rendered by default.

21 changes: 21 additions & 0 deletions src/EFCore.Relational/Query/RelationalShapedQueryCompilingExpressionVisitor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -484,6 +484,27 @@ private static Expression CreateReaderColumnsExpression(
return result;
}

/// <summary>
/// Called after a structural type is materialized, but before it's handed off to the change tracker.
/// Here we inject the JSON shapers for any complex JSON properties the type has.
/// </summary>
/// <remarks>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
/// any release. You should only use it directly in your code with extreme caution and knowing that
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </remarks>
public override void AddStructuralTypeInitialization(
StructuralTypeShaperExpression shaper,
ParameterExpression instanceVariable,
List<ParameterExpression> variables,
List<Expression> expressions)
{
Check.DebugAssert(_currentShaperProcessor is not null);

_currentShaperProcessor.ProcessTopLevelComplexJsonProperties(shaper, instanceVariable, expressions);
}

private Expression CreateRelationalCommandResolverExpression(Expression queryExpression)
{
// In the regular case, we generate code that accesses the RelationalCommandCache (which invokes the 2nd part of the
Expand Down
2 changes: 1 addition & 1 deletion src/EFCore/Query/Internal/StructuralTypeMaterializerSource.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ IServiceProperty serviceProperty
=> serviceProperty.ParameterBinding.BindToParameter(bindingInfo),

IComplexProperty { IsCollection: true } complexProperty
=> Expression.Default(complexProperty.ClrType), // Initialize collections to null, they'll be populated separately
=> Default(complexProperty.ClrType), // Initialize collections to null, they'll be populated separately

IComplexProperty complexProperty
=> CreateMaterializeExpression(
Expand Down
2 changes: 1 addition & 1 deletion src/EFCore/Query/LiftableConstantExpressionHelpers.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ public static Expression BuildMemberAccessForEntityOrComplexType(
/// any release. You should only use it directly in your code with extreme caution and knowing that
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
public static Expression<Func<MaterializerLiftableConstantContext, object>> BuildMemberAccessLambdaForEntityOrComplexType(
public static Expression<Func<MaterializerLiftableConstantContext, object>> BuildMemberAccessLambdaForStructuralType(
ITypeBase type)
{
var prm = Parameter(typeof(MaterializerLiftableConstantContext));
Expand Down
63 changes: 39 additions & 24 deletions src/EFCore/Query/ShapedQueryCompilingExpressionVisitor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ protected ShapedQueryCompilingExpressionVisitor(

_structuralTypeMaterializerInjector =
new StructuralTypeMaterializerInjector(
this,
dependencies.EntityMaterializerSource,
dependencies.LiftableConstantFactory,
queryCompilationContext.QueryTrackingBehavior,
Expand Down Expand Up @@ -233,12 +234,12 @@ protected override Expression VisitConstant(ConstantExpression constantExpressio
{
{ Value: IEntityType entityTypeValue } => liftableConstantFactory.CreateLiftableConstant(
constantExpression.Value,
LiftableConstantExpressionHelpers.BuildMemberAccessLambdaForEntityOrComplexType(entityTypeValue),
LiftableConstantExpressionHelpers.BuildMemberAccessLambdaForStructuralType(entityTypeValue),
entityTypeValue.ShortName() + "EntityType",
constantExpression.Type),
{ Value: IComplexType complexTypeValue } => liftableConstantFactory.CreateLiftableConstant(
constantExpression.Value,
LiftableConstantExpressionHelpers.BuildMemberAccessLambdaForEntityOrComplexType(complexTypeValue),
LiftableConstantExpressionHelpers.BuildMemberAccessLambdaForStructuralType(complexTypeValue),
complexTypeValue.ShortName() + "ComplexType",
constantExpression.Type),
{ Value: IProperty propertyValue } => liftableConstantFactory.CreateLiftableConstant(
Expand Down Expand Up @@ -361,8 +362,20 @@ protected override Expression VisitExtension(Expression extensionExpression)
}
}

/// <summary>
/// Called after a structural type is materialized, but before it's handed off to the change tracker.
/// </summary>
public virtual void AddStructuralTypeInitialization(
StructuralTypeShaperExpression shaper,
ParameterExpression instanceVariable,
List<ParameterExpression> variables,
List<Expression> expressions)
{
}

private sealed class StructuralTypeMaterializerInjector(
IStructuralTypeMaterializerSource entityMaterializerSource,
ShapedQueryCompilingExpressionVisitor shapedQueryCompiler,
IStructuralTypeMaterializerSource materializerSource,
ILiftableConstantFactory liftableConstantFactory,
QueryTrackingBehavior queryTrackingBehavior,
bool supportsPrecompiledQuery)
Expand Down Expand Up @@ -426,10 +439,10 @@ bool ContainsOwner(IEntityType? owner)

protected override Expression VisitExtension(Expression extensionExpression)
=> extensionExpression is StructuralTypeShaperExpression shaper
? ProcessEntityShaper(shaper)
? ProcessStructuralTypeShaper(shaper)
: base.VisitExtension(extensionExpression);

private Expression ProcessEntityShaper(StructuralTypeShaperExpression shaper)
private Expression ProcessStructuralTypeShaper(StructuralTypeShaperExpression shaper)
{
_currentEntityIndex++;

Expand Down Expand Up @@ -565,7 +578,7 @@ private Expression ProcessEntityShaper(StructuralTypeShaperExpression shaper)
supportsPrecompiledQuery
? liftableConstantFactory.CreateLiftableConstant(
typeBase,
LiftableConstantExpressionHelpers.BuildMemberAccessLambdaForEntityOrComplexType(typeBase),
LiftableConstantExpressionHelpers.BuildMemberAccessLambdaForStructuralType(typeBase),
typeBase.Name + "EntityType",
typeof(IEntityType))
: Constant(typeBase),
Expand Down Expand Up @@ -606,7 +619,7 @@ private Expression MaterializeEntity(
ParameterExpression instanceVariable,
ParameterExpression? entryVariable)
{
var typeBase = shaper.StructuralType;
var structuralType = shaper.StructuralType;

var expressions = new List<Expression>();
var variables = new List<ParameterExpression>();
Expand All @@ -626,7 +639,7 @@ private Expression MaterializeEntity(
typeof(ISnapshot))
: Constant(Snapshot.Empty, typeof(ISnapshot))));

var returnType = typeBase.ClrType;
var returnType = structuralType.ClrType;
var valueBufferExpression = Call(materializationContextVariable, MaterializationContext.GetValueBufferMethod);

var materializationConditionBody = ReplacingExpressionVisitor.Replace(
Expand All @@ -637,23 +650,23 @@ private Expression MaterializeEntity(
var expressionContext = (returnType, materializationContextVariable, concreteEntityTypeVariable, shadowValuesVariable);
expressions.Add(Assign(concreteEntityTypeVariable, materializationConditionBody));

var (primaryKey, concreteEntityTypes) = typeBase is IEntityType entityType
var (primaryKey, concreteStructuralTypes) = structuralType is IEntityType entityType
? (entityType.FindPrimaryKey(), entityType.GetConcreteDerivedTypesInclusive().Cast<ITypeBase>().ToArray())
: (null, [typeBase]);
: (null, [structuralType]);

var switchCases = new SwitchCase[concreteEntityTypes.Length];
for (var i = 0; i < concreteEntityTypes.Length; i++)
var switchCases = new SwitchCase[concreteStructuralTypes.Length];
for (var i = 0; i < concreteStructuralTypes.Length; i++)
{
var concreteEntityType = concreteEntityTypes[i];
var concreteStructuralType = concreteStructuralTypes[i];
switchCases[i] = SwitchCase(
CreateFullMaterializeExpression(concreteEntityTypes[i], expressionContext),
CreateFullMaterializeExpression(concreteStructuralTypes[i], expressionContext),
supportsPrecompiledQuery
? liftableConstantFactory.CreateLiftableConstant(
concreteEntityTypes[i],
LiftableConstantExpressionHelpers.BuildMemberAccessLambdaForEntityOrComplexType(concreteEntityType),
concreteEntityType.ShortName() + (typeBase is IEntityType ? "EntityType" : "ComplexType"),
typeBase is IEntityType ? typeof(IEntityType) : typeof(IComplexType))
: Constant(concreteEntityTypes[i], typeBase is IEntityType ? typeof(IEntityType) : typeof(IComplexType)));
concreteStructuralTypes[i],
LiftableConstantExpressionHelpers.BuildMemberAccessLambdaForStructuralType(concreteStructuralType),
concreteStructuralType.ShortName() + (structuralType is IEntityType ? "EntityType" : "ComplexType"),
structuralType is IEntityType ? typeof(IEntityType) : typeof(IComplexType))
: Constant(concreteStructuralTypes[i], structuralType is IEntityType ? typeof(IEntityType) : typeof(IComplexType)));
}

var materializationExpression = Switch(
Expand All @@ -663,9 +676,11 @@ private Expression MaterializeEntity(

expressions.Add(Assign(instanceVariable, materializationExpression));

shapedQueryCompiler.AddStructuralTypeInitialization(shaper, instanceVariable, variables, expressions);

if (_queryStateManager && primaryKey is not null)
{
if (typeBase is IEntityType entityType2)
if (structuralType is IEntityType entityType2)
{
foreach (var et in entityType2.GetAllBaseTypes().Concat(entityType2.GetDerivedTypesInclusive()))
{
Expand Down Expand Up @@ -696,7 +711,7 @@ private Expression MaterializeEntity(
}

private BlockExpression CreateFullMaterializeExpression(
ITypeBase concreteTypeBase,
ITypeBase concreteStructuralType,
(Type ReturnType,
ParameterExpression MaterializationContextVariable,
ParameterExpression ConcreteEntityTypeVariable,
Expand All @@ -709,14 +724,14 @@ private BlockExpression CreateFullMaterializeExpression(

var blockExpressions = new List<Expression>(2);

var materializer = entityMaterializerSource
var materializer = materializerSource
.CreateMaterializeExpression(
new StructuralTypeMaterializerSourceParameters(
concreteTypeBase, "instance", queryTrackingBehavior), materializationContextVariable);
concreteStructuralType, "instance", queryTrackingBehavior), materializationContextVariable);

// TODO: Properly support shadow properties for complex types #35613
if (_queryStateManager
&& concreteTypeBase is IRuntimeEntityType { ShadowPropertyCount: > 0 } runtimeEntityType)
&& concreteStructuralType is IRuntimeEntityType { ShadowPropertyCount: > 0 } runtimeEntityType)
{
var valueBufferExpression = Call(
materializationContextVariable, MaterializationContext.GetValueBufferMethod);
Expand Down
23 changes: 8 additions & 15 deletions test/EFCore.Relational.Specification.Tests/Update/ComplexCollectionJsonUpdateTestBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ public abstract class ComplexCollectionJsonUpdateTestBase<TFixture>(TFixture fix
protected ComplexCollectionJsonContext CreateContext()
=> (ComplexCollectionJsonContext)Fixture.CreateContext();

[ConditionalFact(Skip = "Issue #36433")]
[ConditionalFact]
public virtual Task Add_element_to_complex_collection_mapped_to_json()
=> TestHelpers.ExecuteWithStrategyInTransactionAsync(
CreateContext,
Expand All @@ -26,17 +26,10 @@ public virtual Task Add_element_to_complex_collection_mapped_to_json()

company.Contacts!.Add(new Contact { Name = "New Contact", PhoneNumbers = ["555-0000"] });

Assert.Equal("""
CompanyWithComplexCollections {Id: 1} Unchanged
Id: 1 PK
Name: 'Test Company'
Contacts (Complex: List<Contact>)
Department (Complex: Department)
Budget: 10000.00
Copy link
Member Author

@roji roji Jul 27, 2025

Choose a reason for hiding this comment

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

This failed on SQLite because we get Budget: 10000.0.

Copy link
Member

@AndriySvyryd AndriySvyryd Jul 28, 2025

Choose a reason for hiding this comment

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

Add something like Assert.DoesNotContain("Exception");

Name: 'Initial Department'
Employees (Complex: List<Employee>)

""", context.ChangeTracker.DebugView.LongView);
Assert.Contains("Contacts (Complex: List<Contact>)", context.ChangeTracker.DebugView.LongView);
Assert.Contains("Department (Complex: Department)", context.ChangeTracker.DebugView.LongView);
Assert.Contains("Name: 'Initial Department'", context.ChangeTracker.DebugView.LongView);
Assert.Contains("Employees (Complex: List<Employee>)", context.ChangeTracker.DebugView.LongView);

ClearLog();
await context.SaveChangesAsync();
Expand Down Expand Up @@ -126,7 +119,7 @@ public virtual Task Move_elements_in_complex_collection_mapped_to_json()
}
});

[ConditionalFact(Skip = "Issue #36433")]
[ConditionalFact]
public virtual Task Change_complex_collection_mapped_to_json_to_null_and_to_empty()
=> TestHelpers.ExecuteWithStrategyInTransactionAsync(
CreateContext,
Expand Down Expand Up @@ -354,7 +347,7 @@ public virtual Task Modify_nested_complex_property_in_complex_collection_mapped_
}
});

[ConditionalFact(Skip = "Issue #36433")]
[ConditionalFact]
public virtual Task Set_complex_collection_to_null_mapped_to_json()
=> TestHelpers.ExecuteWithStrategyInTransactionAsync(
CreateContext,
Expand Down Expand Up @@ -499,7 +492,7 @@ public virtual Task Complex_collection_with_empty_nested_collections_mapped_to_j
}
});

[ConditionalFact(Skip = "Issue #36433")]
[ConditionalFact]
public virtual Task Set_complex_property_mapped_to_json_to_null()
=> TestHelpers.ExecuteWithStrategyInTransactionAsync(
CreateContext,
Expand Down
Loading