Skip to content

Commit

Permalink
Fix to #33073 - JSON columns throws Invalid token type: 'StartObject'…
Browse files Browse the repository at this point in the history 
… exception with AsNoTrackingWithIdentityResolution()

Problem was that we were treating NoTrackingWithIdentityResolution queries as regular NoTracking in the context of JSON queries. However due to how JSON is materialized (streaming + nested includes are part of the parent materialization, rather than each entity materialized separately) we have special provisions when ChangeTracker is being used.
In NoTrackingWithIdentityResolution, ChangeTracker is being used but the materializer didn't adjust to that, which lead to errors.

Fix is to generate JSON materializer code based on whether query uses Change Tracker rather than if it's a Tracking/NoTracking query.

Fixes #33073
  • Loading branch information
@maumar
maumar committed Feb 14, 2024
1 parent 42e6cfb commit d08bc46
Show file tree
Hide file tree
Showing 5 changed files with 404 additions and 28 deletions.
22 changes: 15 additions & 7 deletions ...uery/RelationalShapedQueryCompilingExpressionVisitor.ShaperProcessingExpressionVisitor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ private static readonly MethodInfo EnumParseMethodInfo
private readonly RelationalShapedQueryCompilingExpressionVisitor _parentVisitor;
private readonly ISet<string>? _tags;
private readonly bool _isTracking;
private readonly bool _queryStateManager;
private readonly bool _isAsync;
private readonly bool _splitQuery;
private readonly bool _detailedErrorsEnabled;
Expand Down Expand Up @@ -186,6 +187,8 @@ public ShaperProcessingExpressionVisitor(
_generateCommandCache = true;
_detailedErrorsEnabled = parentVisitor._detailedErrorsEnabled;
_isTracking = parentVisitor.QueryCompilationContext.QueryTrackingBehavior == QueryTrackingBehavior.TrackAll;
_queryStateManager = parentVisitor.QueryCompilationContext.QueryTrackingBehavior == QueryTrackingBehavior.TrackAll
|| parentVisitor.QueryCompilationContext.QueryTrackingBehavior == QueryTrackingBehavior.NoTrackingWithIdentityResolution;
_isAsync = parentVisitor.QueryCompilationContext.IsAsync;
_splitQuery = splitQuery;

Expand All @@ -212,6 +215,8 @@ private ShaperProcessingExpressionVisitor(
_generateCommandCache = false;
_detailedErrorsEnabled = parentVisitor._detailedErrorsEnabled;
_isTracking = parentVisitor.QueryCompilationContext.QueryTrackingBehavior == QueryTrackingBehavior.TrackAll;
_queryStateManager = parentVisitor.QueryCompilationContext.QueryTrackingBehavior == QueryTrackingBehavior.TrackAll
|| parentVisitor.QueryCompilationContext.QueryTrackingBehavior == QueryTrackingBehavior.NoTrackingWithIdentityResolution;
_isAsync = parentVisitor.QueryCompilationContext.IsAsync;
_splitQuery = false;
}
Expand Down Expand Up @@ -241,6 +246,8 @@ private ShaperProcessingExpressionVisitor(
_generateCommandCache = true;
_detailedErrorsEnabled = parentVisitor._detailedErrorsEnabled;
_isTracking = parentVisitor.QueryCompilationContext.QueryTrackingBehavior == QueryTrackingBehavior.TrackAll;
_queryStateManager = parentVisitor.QueryCompilationContext.QueryTrackingBehavior == QueryTrackingBehavior.TrackAll
|| parentVisitor.QueryCompilationContext.QueryTrackingBehavior == QueryTrackingBehavior.NoTrackingWithIdentityResolution;
_isAsync = parentVisitor.QueryCompilationContext.IsAsync;
_splitQuery = true;

Expand Down Expand Up @@ -1390,7 +1397,7 @@ private Expression CreateJsonShapers(

var rewrittenEntityShaperMaterializer = new JsonEntityMaterializerRewriter(
entityType,
_isTracking,
_queryStateManager,
jsonReaderDataShaperLambdaParameter,
innerShapersMap,
innerFixupMap,
Expand Down Expand Up @@ -1512,7 +1519,7 @@ private Expression CreateJsonShapers(
private sealed class JsonEntityMaterializerRewriter : ExpressionVisitor
{
private readonly IEntityType _entityType;
private readonly bool _isTracking;
private readonly bool _queryStateManager;
private readonly ParameterExpression _jsonReaderDataParameter;
private readonly IDictionary<string, Expression> _innerShapersMap;
private readonly IDictionary<string, LambdaExpression> _innerFixupMap;
Expand All @@ -1528,15 +1535,15 @@ private static readonly PropertyInfo JsonEncodedTextEncodedUtf8BytesProperty

public JsonEntityMaterializerRewriter(
IEntityType entityType,
bool isTracking,
bool queryStateManager,
ParameterExpression jsonReaderDataParameter,
IDictionary<string, Expression> innerShapersMap,
IDictionary<string, LambdaExpression> innerFixupMap,
IDictionary<string, LambdaExpression> trackingInnerFixupMap,
IDiagnosticsLogger<DbLoggerCategory.Query> queryLogger)
{
_entityType = entityType;
_isTracking = isTracking;
_queryStateManager = queryStateManager;
_jsonReaderDataParameter = jsonReaderDataParameter;
_innerShapersMap = innerShapersMap;
_innerFixupMap = innerFixupMap;
Expand Down Expand Up @@ -1700,9 +1707,9 @@ protected override Expression VisitSwitch(SwitchExpression switchExpression)
finalBlockExpressions.Add(propertyAssignmentReplacer.Visit(jsonEntityTypeInitializerBlockExpression));
}

// Fixup is only needed for non-tracking queries, in case of tracking - ChangeTracker does the job
// Fixup is only needed for non-tracking queries, in case of tracking (or NoTrackingWithIdentityResolution) - ChangeTracker does the job
// or for empty/null collections of a tracking queries.
if (_isTracking)
if (_queryStateManager)
{
ProcessFixup(_trackingInnerFixupMap);
}
Expand Down Expand Up @@ -1879,7 +1886,8 @@ protected override Expression VisitConditional(ConditionalExpression conditional
// the code here re-arranges the existing materializer so that even if we find parent in the change tracker
// we still process all the child navigations, it's just that we use the parent instance from change tracker, rather than create new one
#pragma warning disable EF1001 // Internal EF Core API usage.
if (_isTracking
//if (_isTracking
if (_queryStateManager
&& visited is ConditionalExpression
{
Test: BinaryExpression
Expand Down
123 changes: 117 additions & 6 deletions test/EFCore.Relational.Specification.Tests/Query/JsonQueryTestBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,13 @@ public virtual Task Basic_json_projection_owner_entity_NoTracking(bool async)
async,
ss => ss.Set<JsonEntityBasic>().AsNoTracking());

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Basic_json_projection_owner_entity_NoTrackingWithIdentityResolution(bool async)
=> AssertQuery(
async,
ss => ss.Set<JsonEntityBasic>().AsNoTrackingWithIdentityResolution());

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Basic_json_projection_owner_entity_duplicated(bool async)
Expand All @@ -53,6 +60,19 @@ public virtual Task Basic_json_projection_owner_entity_duplicated_NoTracking(boo
AssertEqual(e.Second, a.Second);
});

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Basic_json_projection_owner_entity_duplicated_NoTrackingWithIdentityResolution(bool async)
=> AssertQuery(
async,
ss => ss.Set<JsonEntitySingleOwned>().Select(x => new { First = x, Second = x }).AsNoTrackingWithIdentityResolution(),
elementSorter: e => e.First.Id,
elementAsserter: (e, a) =>
{
AssertEqual(e.First, a.First);
AssertEqual(e.Second, a.Second);
});

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Basic_json_projection_owner_entity_twice(bool async)
Expand All @@ -79,6 +99,19 @@ public virtual Task Basic_json_projection_owner_entity_twice_NoTracking(bool asy
AssertEqual(e.Second, a.Second);
});

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Basic_json_projection_owner_entity_twice_NoTrackingWithIdentityResolution(bool async)
=> AssertQuery(
async,
ss => ss.Set<JsonEntityBasic>().Select(x => new { First = x, Second = x }).AsNoTrackingWithIdentityResolution(),
elementSorter: e => e.First.Id,
elementAsserter: (e, a) =>
{
AssertEqual(e.First, a.First);
AssertEqual(e.Second, a.Second);
});

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual async Task Project_json_reference_in_tracking_query_fails(bool async)
Expand Down Expand Up @@ -137,6 +170,61 @@ public virtual Task Basic_json_projection_owned_reference_root(bool async)
async,
ss => ss.Set<JsonEntityBasic>().Select(x => x.OwnedReferenceRoot).AsNoTracking());

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Basic_json_projection_owned_reference_root_NoTrackingWithIdentityResolution(bool async)
=> AssertQuery(
async,
ss => ss.Set<JsonEntityBasic>().Select(x => x.OwnedReferenceRoot).AsNoTrackingWithIdentityResolution());

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Basic_json_projection_owned_reference_duplicated(bool async)
=> AssertQuery(
async,
ss => ss.Set<JsonEntityBasic>()
.OrderBy(x => x.Id)
.Select(
x => new
{
Root1 = x.OwnedReferenceRoot,
Branch1 = x.OwnedReferenceRoot.OwnedReferenceBranch,
Root2 = x.OwnedReferenceRoot,
Branch2 = x.OwnedReferenceRoot.OwnedReferenceBranch,
}).AsNoTracking(),
assertOrder: true,
elementAsserter: (e, a) =>
{
AssertEqual(e.Root1, a.Root1);
AssertEqual(e.Root2, a.Root2);
AssertEqual(e.Branch1, a.Branch1);
AssertEqual(e.Branch2, a.Branch2);
});

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Basic_json_projection_owned_reference_duplicated_NoTrackingWithIdentityResolution(bool async)
=> AssertQuery(
async,
ss => ss.Set<JsonEntityBasic>()
.OrderBy(x => x.Id)
.Select(
x => new
{
Root1 = x.OwnedReferenceRoot,
Branch1 = x.OwnedReferenceRoot.OwnedReferenceBranch,
Root2 = x.OwnedReferenceRoot,
Branch2 = x.OwnedReferenceRoot.OwnedReferenceBranch,
}).AsNoTrackingWithIdentityResolution(),
assertOrder: true,
elementAsserter: (e, a) =>
{
AssertEqual(e.Root1, a.Root1);
AssertEqual(e.Root2, a.Root2);
AssertEqual(e.Branch1, a.Branch1);
AssertEqual(e.Branch2, a.Branch2);
});

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Basic_json_projection_owned_reference_duplicated2(bool async)
Expand All @@ -163,7 +251,7 @@ public virtual Task Basic_json_projection_owned_reference_duplicated2(bool async

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Basic_json_projection_owned_reference_duplicated(bool async)
public virtual Task Basic_json_projection_owned_reference_duplicated2_NoTrackingWithIdentityResolution(bool async)
=> AssertQuery(
async,
ss => ss.Set<JsonEntityBasic>()
Expand All @@ -172,17 +260,17 @@ public virtual Task Basic_json_projection_owned_reference_duplicated(bool async)
x => new
{
Root1 = x.OwnedReferenceRoot,
Branch1 = x.OwnedReferenceRoot.OwnedReferenceBranch,
Leaf1 = x.OwnedReferenceRoot.OwnedReferenceBranch.OwnedReferenceLeaf,
Root2 = x.OwnedReferenceRoot,
Branch2 = x.OwnedReferenceRoot.OwnedReferenceBranch,
}).AsNoTracking(),
Leaf2 = x.OwnedReferenceRoot.OwnedReferenceBranch.OwnedReferenceLeaf,
}).AsNoTrackingWithIdentityResolution(),
assertOrder: true,
elementAsserter: (e, a) =>
{
AssertEqual(e.Root1, a.Root1);
AssertEqual(e.Root2, a.Root2);
AssertEqual(e.Branch1, a.Branch1);
AssertEqual(e.Branch2, a.Branch2);
AssertEqual(e.Leaf1, a.Leaf1);
AssertEqual(e.Leaf2, a.Leaf2);
});

[ConditionalTheory]
Expand All @@ -193,13 +281,28 @@ public virtual Task Basic_json_projection_owned_collection_root(bool async)
ss => ss.Set<JsonEntityBasic>().Select(x => x.OwnedCollectionRoot).AsNoTracking(),
elementAsserter: (e, a) => AssertCollection(e, a, ordered: true));

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Basic_json_projection_owned_collection_root_NoTrackingWithIdentityResolution(bool async)
=> AssertQuery(
async,
ss => ss.Set<JsonEntityBasic>().Select(x => x.OwnedCollectionRoot).AsNoTrackingWithIdentityResolution(),
elementAsserter: (e, a) => AssertCollection(e, a, ordered: true));

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Basic_json_projection_owned_reference_branch(bool async)
=> AssertQuery(
async,
ss => ss.Set<JsonEntityBasic>().Select(x => x.OwnedReferenceRoot.OwnedReferenceBranch).AsNoTracking());

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Basic_json_projection_owned_reference_branch_NoTrackingWithIdentityResolution(bool async)
=> AssertQuery(
async,
ss => ss.Set<JsonEntityBasic>().Select(x => x.OwnedReferenceRoot.OwnedReferenceBranch).AsNoTrackingWithIdentityResolution());

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Basic_json_projection_owned_collection_branch(bool async)
Expand All @@ -208,6 +311,14 @@ public virtual Task Basic_json_projection_owned_collection_branch(bool async)
ss => ss.Set<JsonEntityBasic>().Select(x => x.OwnedReferenceRoot.OwnedCollectionBranch).AsNoTracking(),
elementAsserter: (e, a) => AssertCollection(e, a, ordered: true));

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Basic_json_projection_owned_collection_branch_NoTrackingWithIdentityResolution(bool async)
=> AssertQuery(
async,
ss => ss.Set<JsonEntityBasic>().Select(x => x.OwnedReferenceRoot.OwnedCollectionBranch).AsNoTrackingWithIdentityResolution(),
elementAsserter: (e, a) => AssertCollection(e, a, ordered: true));

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Basic_json_projection_owned_reference_leaf(bool async)
Expand Down
24 changes: 12 additions & 12 deletions test/EFCore.Specification.Tests/Query/AdHocMiscellaneousQueryTestBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -160,20 +160,20 @@ protected override void OnModelCreating(ModelBuilder modelBuilder)

public DbSet<Customer> Customers { get; set; }
public DbSet<Postcode> Postcodes { get; set; }
}

public class Customer
{
public int CustomerID { get; set; }
public string CustomerName { get; set; }
public int? PostcodeID { get; set; }
}
public class Customer
{
public int CustomerID { get; set; }
public string CustomerName { get; set; }
public int? PostcodeID { get; set; }
}

public class Postcode
{
public int PostcodeID { get; set; }
public string PostcodeValue { get; set; }
public string TownName { get; set; }
public class Postcode
{
public int PostcodeID { get; set; }
public string PostcodeValue { get; set; }
public string TownName { get; set; }
}
}

#endregion
Expand Down
Loading

0 comments on commit d08bc46

Please sign in to comment.