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.

However, this uncovered some issue with NoTrackingWithIdentityResolution - depending on the order in which entities are processed during materialization, they could cause data corruption (wrong order) when materializing JSON collections. Also, using queryable operators on JSON collections may cause errors or data corruption - we don't propagate key values of those queries to the materializer, so those entities end up with null keys.
Adding a validator that makes sure that entities are visited in the correct order and issues exception instructing what to do, if the order is wrong. Also we disable usage of queryable operators, due to the issue mentioned above, and cases where parameters are being used to access collection element in the navigatio chain. For cases with parameters, we can't tell if the value is the same or different so can't properly validate those cases.
Two different parameters can have the same value, leading to the same entity being materialized, but when we analyze their JSON path, those paths look different.

Fixes #33073
  • Loading branch information
maumar committed Mar 6, 2024
1 parent 8173631 commit 250e99c
Show file tree
Hide file tree
Showing 8 changed files with 1,490 additions and 49 deletions.
24 changes: 24 additions & 0 deletions src/EFCore.Relational/Properties/RelationalStrings.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 9 additions & 0 deletions src/EFCore.Relational/Properties/RelationalStrings.resx
Expand Up @@ -559,6 +559,15 @@
<data name="JsonNodeMustBeHandledByProviderSpecificVisitor" xml:space="preserve">
<value>This node should be handled by provider-specific sql generator.</value>
</data>
<data name="JsonProjectingCollectionElementAccessedUsingParmeterNoTrackingWithIdentityResolution" xml:space="preserve">
<value>Using parameter to access the element of a JSON collection '{entityTypeName}' is not supported for '{asNoTrackingWithIdentityResolution}'. Use constant, or project the entire JSON entity collection instead.</value>
</data>
<data name="JsonProjectingEntitiesIncorrectOrderNoTrackingWithIdentityResolution" xml:space="preserve">
<value>When using '{asNoTrackingWithIdentityResolution}' entities mapped to JSON must be projected in a particular order. Project entire collection of entities '{entityTypeName}' before its individual elements.</value>
</data>
<data name="JsonProjectingQueryableOperationNoTrackingWithIdentityResolution" xml:space="preserve">
<value>Projecting queryable operations on JSON collection is not supported for '{asNoTrackingWithIdentityResolution}'.</value>
</data>
<data name="JsonPropertyNameShouldBeConfiguredOnNestedNavigation" xml:space="preserve">
<value>The JSON property name should only be configured on nested owned navigations.</value>
</data>
Expand Down

Large diffs are not rendered by default.

8 changes: 4 additions & 4 deletions src/EFCore/Query/QueryContext.cs
Expand Up @@ -125,10 +125,10 @@ public virtual void InitializeStateManager(bool standAlone = false)
/// </summary>
[EntityFrameworkInternal]
public virtual InternalEntityEntry? TryGetEntry(
IKey key,
object[] keyValues,
bool throwOnNullKey,
out bool hasNullKey)
IKey key,
object[] keyValues,
bool throwOnNullKey,
out bool hasNullKey)
// InitializeStateManager will populate the field before calling here
=> _stateManager!.TryGetEntry(key, keyValues, throwOnNullKey, out hasNullKey);

Expand Down
911 changes: 904 additions & 7 deletions test/EFCore.Relational.Specification.Tests/Query/JsonQueryTestBase.cs

Large diffs are not rendered by default.

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

0 comments on commit 250e99c

Please sign in to comment.