-
Notifications
You must be signed in to change notification settings - Fork 226
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
Change Tracking on Jsonb #1117
Comments
@sitepodmatt you're absolutely right, thanks for filing this - I did not include any change tracking support for this initial implementation. I'll try to look at this, hopefully for 3.1.0. Note that instead of cloning your entire Dictionary, you can simply explicitly tell EF that the value has changed: context.ChangeTracker.Entries<MyEntityType>().SingleOrDefault(e => ...)?.State = EntityState.Modified; PS Just to make sure I understand, are you mapping a |
Just experimenting at the moment, we basically have a jsonb column for aspects (think EAV style) for multiple aggregates, seems to work in principal. Since the graph can be deep I'll stick with the clone for now (our write load is small) than manually changing the entity entry state but thanks for the suggestion. The other quirk, but expected when you think about it is, when we do setValues on a matched entity (if exists) similar to https://docs.microsoft.com/en-us/ef/core/saving/disconnected-entities#mix-of-new-and-existing-entities then we need to ensure the dictionaries have structural equality by override equals etc... to avoid the state changing to Modified when its really Unchanged. Side thought: It got me thinking it would be rather cool if there was an EF 4.x built on the immutable collections. It would make change tracking dramatically faster, memory footprint smaller, but the change in how people approach entities would be huge as it wouldn't just be internal.. I wonder if F# has something.. |
Yeah, that's basically the main thing in this issue - looking into value comparers that would do structural equality. However, thinking about all this some more, it's not 100% clear this is the right thing to do. With big/deep data structures, structural equality can be exceedingly expensive, so it may be better to leave it to the user to manually flag changes (via the manual API I showed above). IIRC that's what EF Core does for byte arrays - changing a specific index in the array would not be detected by EF Core. It's likely that the same holds for JSON here - I'll look into it.
Do you mean mapping immutable collections to columns (e.g. such PostgreSQL database arrays)? Or to represent a collection navigation? Note that can easily disable change tracking (which seems to make sense for immutable stuff) or manage it yourself (via the manual API I showed above). |
Not too sure on the manually flagging the change, it would become a burden on the user to do and would need to expose the EF internals beyond a repository facade, also tricky on a deep graph (although the jsonb property is generally small and swallow), and duplicate of work on disconnected scenarios (where you would need to detect the changes anyway)... I'll keep playing with it and let you know how it turns out when I've flushed it out some more. On the side thought, I was just thinking how things would work if EF was based on persistent immutable collections but thats a whole new ball game that just won't work in EF core now or possibly ever.. Ignore.. |
That's true, but it's one of those places where a compromise for perf seems more than acceptable. A default behavior which performs structural comparisons on arbitrary-long data structures would produce potentially terrible perf...
I don't think that's necessarily true... There's potential for arbitrarily complex data structures there. |
Ah yeah sorry to clarify I meant in my scenario. I think implementing structural equality should be responsibility of the user where it makes sense, this is what I've done by override equals so it doesnt just fallback to referential equality, and it works for us such that setValues won't trigger entry state to change to modified where it is actually unchanged (in terms of the two dictionary being structurally the same). However we don't have a answer to first problem other than cloning on change (or manually setting entry state) because its the same instance of the property in CurrentValues and OriginalValues unless I can somehow change the behavior of the ChangeTracking so that the OriginalValues is cloned on population (and each time savechanges / reset is called) |
OK, I investigated a little bit. The System.Text.Json DOM types (JsonElement, JsonDocument) are immutable, and so there's no need to implement comparison or snapshot behavior. For POCOs the provider can't really do anything, as this could be any type at all. However, EF Core allows you to easily set up a ValueComparer yourself on a per-property basis: modelBuilder.Entity<Blog>()
.Property(b => b.SomeJsonProperty).Metadata.SetValueComparer(new ValueComparer<string>(.....)); The ValueComparer type provides comparison, hashing and snapshotting logic. I think this is the right thing to do for a JSON-mapped Dictionary as well. Am going to close this as I don't think anything should be done by default in the provider - it's better to let users decide on the behavior they want here. But am open to other ideas. |
Awesome. I will dig into the ValueComparer stuff. Thanks for the pointers. |
I just ran into this issue today as well. Previously, before moving everything to 3.0, I had been using this solution to store models into jsonb columns: https://stackoverflow.com/a/54687171 After migrating everything to 3.0, I removed all of these to use the native jsonb support (which has been working great without problems) and completely forgot about this equality issue. After realizing this, I added back the "SetValueComparer" code of the Stack Overflow answer and things went back to working as expected. I'm not sure that this solution is "efficient" as it serializes the objects and compares them as strings - however, a solution with an extension method like this can be applied only to properties that can encounter this issue. |
It works but could it could serialize up to 3 times. In event of hashcodes compute to the same (1st serialization), it will call equality method (2nd serialization) if is false then finally 3rd serialization to get the serialized representation of the object for persistence, there 3x the amount of serialization work required. By order of wins - implement hashcode expression then the equality expression, then something nicer than serialize/deserialize for the snapshotter. But if it's not causing a problem it may not be a priority... |
Agreed, but I'm not using it for anything that should be "large" so I'm not sure it will ever be a real issue. The reason I'm encountering this issue is because I'm using AutoMapper (and the Collection+EF packages) to map DTOs back to entities - which seems to also understand how to map/merge dictionaries into each other. I guess in my situation the "correct" solution is a string compare between the json passed back to the server and the json existing in the jsonb column, since I'm trying to check "if the client changed the data" and said data is simply stored as json. The only way I can really think of doing this is passing to/from the server with a JsonElement and working with that - but then I lose typing (in swagger as well as .NET), which I'd prefer not to do. |
Automatic change tracking in EF Core 3.0 doesn't quite work when using jsonb support (per https://www.npgsql.org/efcore/mapping/json.html?tabs=data-annotations%2Cpoco) to a POCO or Dictionary<string,string> and mutating the contaning entity since the jsonb item is the same by referential equality in both OriginalValues and NewValues. Containing entity remains in Unchanged state.
Current undesirable workaround, on the containing entity we create a clone of Dictionary, and set the additional key/value and set the new clone on the containing entity. Item changes to Modified in ChangeTracking as expected.
This may be a design limitation of EF Core 3.0 but worth mentioning in the docs for now at least. Thanks
The text was updated successfully, but these errors were encountered: