dotnet / EntityFramework.Docs

Documentation for Entity Framework Core and Entity Framework 6
https://docs.microsoft.com/ef/
Creative Commons Attribution 4.0 International
1.57k stars 1.94k forks source link

Document: OwnedEntities requires AsNoTracking when queried separately #2205

Open slipdef opened 4 years ago

slipdef commented 4 years ago

The code below just crashes with "A tracking query projects owned entity without corresponding owner in result. Owned entities cannot be tracked without their owner":

.Select(x => new A
{
     OwnedEntity = x.OwnedEntity
});

It worked fine on EF Core 2. No single word about that in the breaking changes doc!

I have had a hard time finding this comment: https://github.com/aspnet/EntityFrameworkCore/issues/17617#issuecomment-529162589

I believe this pattern of querying data is quite often. At least I used to it since EF 6. Adding AsNoTracking() in these cases doesn't make sense for me considering the pattern. I understand it's too late to re-consider but please describe that behavior in the breaking changes at least.

ajcvickers commented 4 years ago

@slipdef What is it that you're trying to do that needs an owned entity tracked without it's owner? In other words, why is it an owned entity if it can exist without its owner?

slipdef commented 4 years ago

I indeed don't need an owned entity being tracked without owner. So I would be totally fine if AsNoTracking() is added by default in such cases.

ajcvickers commented 4 years ago

@slipdef In that case can you elaborate on, "adding AsNoTracking() in these cases doesn't make sense for me."

slipdef commented 4 years ago

I didn't need to write that code before EF Core 3:

DbSet.AsNoTracking().Select(x => new A
{
     OwnedEntity = x.OwnedEntity
});

I just did

DbSet.Select(x => new A
{
     OwnedEntity = x.OwnedEntity
});

and all worked fine.

Now I need because AsNoTracking() is required when querying owned entity without principal. As you pointed out: why would someone need to query owned entity without owner and expect it to be tracked by EF. It doesn't make sense for me too.

ajcvickers commented 4 years ago

@slipdef Thanks; we will discuss.

ajcvickers commented 4 years ago

We discussed in triage and decided to keep this on the backlog to consider not requiring AsNoTracking in this case.

@smitpatel Can you write down something that explains the main reason owned types can be more confusing than other non-tracked objects.

AndriySvyryd commented 4 years ago

Adding AsNoTracking implicitly wouldn't prevent the breaking change if the code relied on them being tracked.

joelmdev commented 4 years ago

Might be worth noting that mapping properties of an Owned Entity Type in a projection will not cause this issue but mapping the Owned Entity Type itself in a projection will, even if properties of the owner are mapped. Examples:

_context.DomainUser.Select(du => new { id = du.Id, Address = du.Address.Line1 }).ToList(); //no exception

_context.DomainUser.Select(du => new { id = du.Id, Address = du.Address }).ToList(); //throws exception

_context.DomainUser.Select(du => new { Address = du.Address.Line1 }).ToList(); //no exception

This really makes Owned Entity Types painful to use when mapping to view models or DTOs.

smitpatel commented 4 years ago

With model

public class Customer
{
    public int Id { get; set; }
    public Address Address { get; set; }
}

[Owned]
public class Address
{
    public int CityId { get; set; }
    public City City { get; set; }
}

public class City
{
    public int Id { get; set; }
}

Query

var query = db.Customers.Select(c => new
{
    One = c,
    Two = c.Address,
    Three = c.Address.City
});

Above query is marked as tracking by default. If you project Customer, we have enough data to update Address also.

Overall, making implicit non-tracking forces users to understand what case it would be tracked vs non-tracked with above set of complex rules, alongwith potential of data loss if something is not saved correctly.

By making AsNoTracking required explicitly, users would know when they are querying owned entity which cannot be updated. So they can take corrective action based on what usage they want. Further, by putting explicit AsNoTracking it becomes clearer that what happens to unrelated entities like City in such queries.

marcwittke commented 1 year ago

I have a domain layer that is provided with an IQueryable<TAggregate> that in reality is a DbSet<TAggregate> created by the persistence layer. The domain layer now wants to do such a projection but out of a sudden gets an Exception, that yesterday wasn't there.

Plus: The domain layer has no idea what Entity Framework is. It is persistence agnostic, so there is no .AsNoTracking(). I am basically forced to do a load of the whole table into memory, just to prevent someone who copies code from the documentation without reading the documentation to shoot themseves into the foot.

Sadly, this is a recurrent pattern with Entity Framework Core: you spent too much energy on the "Demoware" use cases, but leave behind the serious architectual concepts like domain driven design. Don't get me wrong, I really love EF Core, but the time I spent to make thing such as an aggregate working is really high and I start thinking that an implementation at lower level like Dapper would be easier in the end, and using EF Core only as query implementation as the Q in CQRS

bkoelman commented 1 year ago

One of our customers reported this error, though the scenario is slightly different. JsonApiDotNetCore dynamically produces LINQ queries of the following form when the API request contains sparse fieldsets (basically selecting a subset of the entity properties and navigations).

var customers = db.Customers.Select(c => new Customer()
{
    Id = c.Id,
    Address = c.Address
});

When no owned entities are involved, this works without explicitly setting a tracking behavior. The entities won't get tracked (obviously), which is fine because we only do this on read-only requests. But when owned entities are used, we now need to explicitly mark the query as non-tracking.

I understand that the rules when tracking occurs are complex, but the current experience is pretty inconsistent: We've all been building queries for years that weren't trackable, and we never cared. Now we suddenly need to rationalize and specify when we want to track or not, which is a pain because it may heavily affect performance. It would make more sense to me to downgrade this exception to a warning, which we can suppress in configuration. As a workaround, we're now setting the tracking behavior to NoTrackingWithIdentityResolution for all read-only requests.

Aside from that, in our example query there is actually an owner, so the error message does not seem to apply. Is this a bug?

ajcvickers commented 1 year ago

@bkoelman We will discuss.

ajcvickers commented 10 months ago

Note from triage: we discussed this again, but we still believe that the error is useful for preventing people falling into a pit of failure with tracking queries. We are not going to make changes here.