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.

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

Fix to #33073 - JSON columns throws Invalid token type: 'StartObject' exception with AsNoTrackingWithIdentityResolution() #33101

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

maumar
Copy link
Contributor

@maumar maumar commented Feb 14, 2024

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

@maumar
Copy link
Contributor Author

maumar commented Feb 14, 2024

there is a problem with this fix, some tests regress into data corruption (from invalid token now), need to investigate in-depth.

failing tests for NTWIR

Json_branch_collection_distinct_and_other_collection - NRE
Json_collection_distinct_in_projection - NRE
Json_collection_filter_in_projection - NRE
Json_collection_SelectMany - unable to track
Json_collection_skip_take_in_projection - NRE
Json_multiple_collection_projections - NRE
Json_nested_collection_anonymous_projection_in_projection - DATA CORRUPTION
Json_nested_collection_filter_in_projection - NRE
Json_nested_collection_SelectMany - unable to track
Json_projection_deduplication_with_collection_in_original_and_collection_indexer_in_target - DATA CORRUPTION
Json_projection_deduplication_with_collection_indexer_in_target - DATA CORRUPTION

@maumar
Copy link
Contributor Author

maumar commented Mar 5, 2024

@roji @ajcvickers new version up

… 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
@aldrashan
Copy link

Semi-related: the same error gets thrown when it tries to deserialize a json object as a list of objects.

 public class Engravings
 {
     public Guid UidProduction { get; set; }
     public List<Engraving_Files>? Files { get; set; }
 }
modelBuilder.Entity<Engravings>(entity =>
 {
     entity.HasKey(x => x.UidProduction);
     entity.OwnsMany(x => x.Files, opt => { opt.ToJson(); });
 });

-> Database is expected to return something like the following for the Files property: [ {"id":"1"}, {"id":"2"} ]
If the database value is actually not an array, because of manual db manipulations, e.g. {"id":"3"}, the exception gets thrown.

Meanwhile, if you would try to deserialize that incorrect object as a List via Newtonsoft (haven't tried with System.Text.Json), you'd get a much clearer Exception message.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

JSON columns throws Invalid token type: 'StartObject' exception with AsNoTrackingWithIdentityResolution()
3 participants