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

IdentityResolution Configuration on AsNoTracking queries #19877

Closed
YZahringer opened this issue Feb 11, 2020 · 7 comments · Fixed by #20739
Closed

IdentityResolution Configuration on AsNoTracking queries #19877

YZahringer opened this issue Feb 11, 2020 · 7 comments · Fixed by #20739
Assignees
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported type-enhancement
Milestone

Comments

@YZahringer
Copy link

YZahringer commented Feb 11, 2020

Identity Resolution is no longer performed with no-tracking queries on EF Core 3: breaking change doc

In some cases, I think that creating several instances of the same object instead of reusing the instance is not optimal in terms of memory usage.

In other cases, such as in recursive objects, you want to be able to enable Identity Resolution to resolve all properties and be able to navigate recursively.

I created a Unit Test that reproduces the recursivity problem in EF Core 3 with NoTracking: https://github.com/MADSENSE/Madsense.EFCore.Tests/tree/no-tracking-recursive

Related issues: #19276, #18570

Proposal
Make Identity Resolution configurable when a request is executed with AsNoTracking.

@ajcvickers
Copy link
Contributor

Team notes: sometimes it is desirable to have full identity resolution in cases where it is not possible to use a tracking query. For example, when diffing the current and database graphs to resolve update resolutions. In these cases the only solution is to spin up a new context for the query. We could instead provide a query mode that automatically spins up a new state manager or similar to do identity resolution. This would leave the default no-tracking case usable as a streaming API without memory issues, while still allowing a different behavior where it is needed.

Related: #11564

@ole1986
Copy link

ole1986 commented Apr 1, 2020

This issue is absolutely underrated already!!! It is a "must-have" to properly resolve (repeating models) when using AsNoTracking()

@smitpatel smitpatel self-assigned this Apr 23, 2020
@smitpatel smitpatel modified the milestones: Backlog, 5.0.0 Apr 24, 2020
smitpatel added a commit that referenced this issue Apr 24, 2020
- Uses an adhoc statemanager in the background which is different from statemanager in the context

Resolves #19877
smitpatel added a commit that referenced this issue Apr 24, 2020
- Uses an adhoc statemanager in the background which is different from statemanager in the context

Resolves #19877
smitpatel added a commit that referenced this issue Apr 27, 2020
- Uses an adhoc statemanager in the background which is different from statemanager in the context

Resolves #19877
smitpatel added a commit that referenced this issue Apr 28, 2020
- Uses an adhoc statemanager in the background which is different from statemanager in the context

Resolves #19877
@smitpatel smitpatel added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Apr 28, 2020
@ajcvickers ajcvickers modified the milestones: 5.0.0, 5.0.0-preview5 May 11, 2020
@JonPSmith
Copy link

This comments might be too late, but here is my take on surfacing this feature. If its a done deal then ignore, but I only found out about this yesterday.

I'm not sure how you are going to control the identity resolution added to a AsNoTracking call, but personally I think something like ...AsNoTracking(true) for turning on identity resolution, and the original ...AsNoTracking() where identity resolution is turned off. I like that because most AsNoTracking queries are for the front-end, which doesn't care about identity resolution - I'm only interested in identity resolution in business logic that a) wants a read-only copy but b) needs the class instances map exactly to what the database says - which is a small set of AsNoTracking queries.

Also, you could provide a DbContext-wide setting to switch all AsNoTracking queries to include identity resolution. Then optional parameter to AsNoTracking allows you to turn off the identity resolution for an individual AsNoTracking query, e.g. ...AsNoTracking(false). Not sure that is worth the effort, but some people might find that useful (I wouldn't).

@smitpatel
Copy link
Contributor

@JonPSmith - We are going to update the exact API. We discussed about it during API review. Current plan is to add another enum value in QueryTrackingBehavior enum, which will enable

  • Making it separate from AsNoTracking to avoid confusion.
  • Ability to set at context level.
  • Possibly As* API to align with AsTracking/AsNoTracking which does not take any parameter.
  • Ability to use AsTracking with the enum value.

@smitpatel
Copy link
Contributor

New API - AsNoTrackingWithIdentityResolution
Added new enum value in QueryTrackingBehavior to control it at context level or use with AsTracking API which takes parameter.

smitpatel added a commit that referenced this issue Jun 17, 2020
…solution

- Add another enum value in QueryTrackingBehavior.
- Change query pipeline to work with enum rather than bool flag for IsTracking

Part of #20409
Part of #19877
smitpatel added a commit that referenced this issue Jun 17, 2020
…solution (#21303)

- Add another enum value in QueryTrackingBehavior.
- Change query pipeline to work with enum rather than bool flag for IsTracking

Part of #20409
Part of #19877
@ole1986
Copy link

ole1986 commented Jul 9, 2020

Öhm just asking a dumb question: will this be only available on EF Core 5.0 and higher?

@ajcvickers
Copy link
Contributor

@ole1986 Yes.

@ajcvickers ajcvickers removed this from the 5.0.0-preview5 milestone Nov 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported type-enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants