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 5, 2024
1 parent 5d3dfe1 commit 8fcb54c
Show file tree
Hide file tree
Showing 8 changed files with 1,543 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
894 changes: 887 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
Expand Up @@ -2414,4 +2414,92 @@ ELSE NULL
END = N'COUNTRY'
""");
}


[ConditionalFact]
public void Test_NTWIR_owned_type()
{
using (var ctx = new MyContext())
{
ctx.Database.EnsureDeleted();
ctx.Database.EnsureCreated();
var e = new MyEntity
{
Owned = new OwnedRoot
{
Number = 10,
Nested = new List<OwnedBranch>
{
new OwnedBranch { Foo = "f1", InnerNested = new List<OwnedLeaf> { new OwnedLeaf { Bar = 55 } } },
new OwnedBranch { Foo = "f2", InnerNested = new List<OwnedLeaf> { new OwnedLeaf { Bar = 266 }, new OwnedLeaf { Bar = 277 } } }

//new OwnedBranch { Foo = "f1" },
//new OwnedBranch { Foo = "f2" }
}
}
};
ctx.Entities.Add(e);
ctx.SaveChanges();
}

using (var ctx = new MyContext())
{
var result = ctx.Entities
.OrderBy(x => x.Id)
.Select(
x => new
{
First = x.Owned.Nested.ToList(),
})
.AsNoTrackingWithIdentityResolution().ToList();
}
}

public class MyContext : DbContext
{
public DbSet<MyEntity> Entities { get; set; }


protected override void OnModelCreating(ModelBuilder modelBuilder)
{
modelBuilder.Entity<MyEntity>().OwnsOne(x => x.Owned, b =>
{
b.ToJson();
b.OwnsMany(xx => xx.Nested, bb => bb.OwnsMany(xxx => xxx.InnerNested));
//b.OwnsMany(xx => xx.Nested);
});
}

protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
{
optionsBuilder.UseSqlServer(@"Server=(localdb)\mssqllocaldb;Database=Repro;Trusted_Connection=True;MultipleActiveResultSets=true");
}
}

public class MyEntity
{
public int Id { get; set; }
public string Name { get; set; }

public OwnedRoot Owned { get; set; }
}

public class OwnedRoot
{
public int Number { get; set; }

public List<OwnedBranch> Nested { get; set; }
}

public class OwnedBranch
{
public string Foo { get; set; }

public List<OwnedLeaf> InnerNested { get; set; }
}

public class OwnedLeaf
{
public int Bar { get; set; }
}
}

0 comments on commit 8fcb54c

Please sign in to comment.