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

Discussion on lazy-loading of navigation properties #3797

Open
AndiRudi opened this Issue Nov 19, 2015 · 229 comments

Comments

Projects
None yet
@AndiRudi

AndiRudi commented Nov 19, 2015

Note:

  • Issue #10509 has been created to track actual work on lazy-loading. The new issue is locked so that it can be subscribed to for updates on the implementation without noise.
  • This issue has been re-purposed to be discussion about lazy-loading in EF Core. It will not (for the time being) be locked because it is important to us that we don’t shut down channels of communication with the community.

With regard to feedback, I think it is worth reiterating some comments made a few months ago. We read and consider all feedback (and try to make the right choices based on it) no matter how it is delivered; that is, whether it is provided politely and with respect or not. Yelling at us or others with different opinions at best makes you feel good and us/them feel bad. It doesn’t achieve anything concrete. We are doing our best to prioritize features and implement them in the best way we can. We really appreciate your feedback and it makes a big difference on shaping the product. We personally appreciate it more when feedback is delivered in a respectful way, but please don’t stop providing constructive feedback.


Original issue:

Hi,

i was wondering if I am the only one that thinks EF Core is useless without Lazy Loading? Or do I do something wrong? Lets just consider a simple scenario when a course provider cancels a course on a course booking plattform written in MVC.

The user calls courses/cancel/1. The action would get the course and call cancel method like here

Course course = context.Courses.SingleOrDefault(c => c.Id = 1);
course.Cancel();

The cancel method then needs to cancel each booking on the course so it would do something like this

foreach(Booking booking in this.Bookings)
{
    booking.Cancel();
}

The booking in turn would refund the transaction

foreach(Transaction transaction in this.Transactions)
{
    transaction.Refund();
}

So for this to work, lazy loading is needed?! (The solution would be to eager load all the data on the controllers action method. But I do not know what will be needed from the controller?)

I'd appreciate any information on that.

@jemiller0

This comment has been minimized.

Show comment
Hide comment
@jemiller0

jemiller0 Nov 19, 2015

I think you can do something like the following to eagerly load everything.

Course course = context.Courses.Include(c => c.Bookings).ThenInclude(b => b.Transactions).SingleOrDefault(c => c.Id = 1);

Otherwise, I'm assuming EF 7 has a way of explicitly loading properties like the following. However, I just upgraded to EF 7 today and haven't done that before. I think they changed the API around some. I'm assuming there has to be an equivalent though.

https://msdn.microsoft.com/en-us/data/jj574232.aspx#explicit

Also, as far as I know this site is for submitting bug reports, not questions.

jemiller0 commented Nov 19, 2015

I think you can do something like the following to eagerly load everything.

Course course = context.Courses.Include(c => c.Bookings).ThenInclude(b => b.Transactions).SingleOrDefault(c => c.Id = 1);

Otherwise, I'm assuming EF 7 has a way of explicitly loading properties like the following. However, I just upgraded to EF 7 today and haven't done that before. I think they changed the API around some. I'm assuming there has to be an equivalent though.

https://msdn.microsoft.com/en-us/data/jj574232.aspx#explicit

Also, as far as I know this site is for submitting bug reports, not questions.

@rowanmiller

This comment has been minimized.

Show comment
Hide comment
@rowanmiller

rowanmiller Nov 20, 2015

Member

We will be implementing explicit loading, but it's not there yet. Lazy loading is still undecided... so feedback is good - thank you.

Also, as far as I know this site is for submitting bug reports, not questions.

I think this can be considered a feature request. Lazy loading is still on the list of things we are considering... but it looks like we don't have a dedicated issue tracking it, so we'll use this one.

Member

rowanmiller commented Nov 20, 2015

We will be implementing explicit loading, but it's not there yet. Lazy loading is still undecided... so feedback is good - thank you.

Also, as far as I know this site is for submitting bug reports, not questions.

I think this can be considered a feature request. Lazy loading is still on the list of things we are considering... but it looks like we don't have a dedicated issue tracking it, so we'll use this one.

@rowanmiller rowanmiller added this to the Backlog milestone Nov 20, 2015

@yukozh

This comment has been minimized.

Show comment
Hide comment
@yukozh

yukozh Nov 22, 2015

Contributor

+1 for lazy load

Contributor

yukozh commented Nov 22, 2015

+1 for lazy load

@dneimke

This comment has been minimized.

Show comment
Hide comment
@dneimke

dneimke Nov 22, 2015

I'm not saying that Lazy Loading isn't important, but the example provided is not a good example of "why". Such logic should live in a business logic layer which would orchestrate all of the actions required to fulfil the transaction.

dneimke commented Nov 22, 2015

I'm not saying that Lazy Loading isn't important, but the example provided is not a good example of "why". Such logic should live in a business logic layer which would orchestrate all of the actions required to fulfil the transaction.

@jemiller0

This comment has been minimized.

Show comment
Hide comment
@jemiller0

jemiller0 Nov 22, 2015

It should at least have explicit loading. EF 7 doesn’t even have that at this point. In the example given, you are forced to load data that you don’t need since it was nested collections. I guess that’s no worse than it would be with lazy loading. I think you could be more selective with explicit. I guess the only way to load properties is using Include() ThenInclude() at this point. I find it surprising that the release won’t even have explicit loading. I guess it will still be workable. Lazy loading can get you into a lot of trouble though if you don’t pay attention to what’s happening. It can cause performance problems.

Jon

jemiller0 commented Nov 22, 2015

It should at least have explicit loading. EF 7 doesn’t even have that at this point. In the example given, you are forced to load data that you don’t need since it was nested collections. I guess that’s no worse than it would be with lazy loading. I think you could be more selective with explicit. I guess the only way to load properties is using Include() ThenInclude() at this point. I find it surprising that the release won’t even have explicit loading. I guess it will still be workable. Lazy loading can get you into a lot of trouble though if you don’t pay attention to what’s happening. It can cause performance problems.

Jon

@rowanmiller

This comment has been minimized.

Show comment
Hide comment
@rowanmiller

rowanmiller Nov 23, 2015

Member

It's not as tidy as proper explicit loading support, but you can run a second LINQ query to load related data (EF will fixup all the navigation properties for you).

Course course = context.Courses.SingleOrDefault(c => c.Id = 1);

// sometime later, load the related data
context.Bookings.Where(b => b.CourseId == course.CourseId).ToList();
Member

rowanmiller commented Nov 23, 2015

It's not as tidy as proper explicit loading support, but you can run a second LINQ query to load related data (EF will fixup all the navigation properties for you).

Course course = context.Courses.SingleOrDefault(c => c.Id = 1);

// sometime later, load the related data
context.Bookings.Where(b => b.CourseId == course.CourseId).ToList();
@AndiRudi

This comment has been minimized.

Show comment
Hide comment
@AndiRudi

AndiRudi Nov 23, 2015

I already thought about this approach. But the problem is, that I do not have access to the context inside a model. Or has this changed in EF Core? I would need a way to inject the context or some other scope into the model to have the same context.

AndiRudi commented Nov 23, 2015

I already thought about this approach. But the problem is, that I do not have access to the context inside a model. Or has this changed in EF Core? I would need a way to inject the context or some other scope into the model to have the same context.

@rowanmiller

This comment has been minimized.

Show comment
Hide comment
@rowanmiller

rowanmiller Nov 23, 2015

Member

@AndiRudi that is true... but that is also true of explicit loading since explicit loading is about calling an EF API to load data.

Member

rowanmiller commented Nov 23, 2015

@AndiRudi that is true... but that is also true of explicit loading since explicit loading is about calling an EF API to load data.

@dneimke

This comment has been minimized.

Show comment
Hide comment
@dneimke

dneimke Nov 23, 2015

@rowanmiller is there a way to prevent that behavior?

Course course = context.Courses.SingleOrDefault(c => c.Id = 1);

// sometime later, load the related data
context.Bookings.Where(b => b.CourseId == course.CourseId).ToList();

E.g. Ask EF not to 'fix-up' the navigation properties automatically?

dneimke commented Nov 23, 2015

@rowanmiller is there a way to prevent that behavior?

Course course = context.Courses.SingleOrDefault(c => c.Id = 1);

// sometime later, load the related data
context.Bookings.Where(b => b.CourseId == course.CourseId).ToList();

E.g. Ask EF not to 'fix-up' the navigation properties automatically?

@rowanmiller

This comment has been minimized.

Show comment
Hide comment
@rowanmiller

rowanmiller Nov 23, 2015

Member

@dneimke sure, if you want to execute a query without all the change tracking, fixup to existing entities that are already tracked, etc., then you can use the AsNoTracking() method on the query.

Out of interest, what is the scenario where you don't want that to happen?

Member

rowanmiller commented Nov 23, 2015

@dneimke sure, if you want to execute a query without all the change tracking, fixup to existing entities that are already tracked, etc., then you can use the AsNoTracking() method on the query.

Out of interest, what is the scenario where you don't want that to happen?

@dneimke

This comment has been minimized.

Show comment
Hide comment
@dneimke

dneimke Nov 23, 2015

Running tests against an InMemoryProvider. I want to ensure that I can execute multiple test cases from a Single serviceProvider instance without having the results dirtied by side effects

dneimke commented Nov 23, 2015

Running tests against an InMemoryProvider. I want to ensure that I can execute multiple test cases from a Single serviceProvider instance without having the results dirtied by side effects

@rowanmiller

This comment has been minimized.

Show comment
Hide comment
@rowanmiller

rowanmiller Nov 23, 2015

Member

@dneimke That would work. Possibly better to register the context as scoped and then have a scope per test? That way you get a clean context per test.

Member

rowanmiller commented Nov 23, 2015

@dneimke That would work. Possibly better to register the context as scoped and then have a scope per test? That way you get a clean context per test.

@dneimke

This comment has been minimized.

Show comment
Hide comment
@dneimke

dneimke Nov 23, 2015

True. Another example is that I want to write a test console app. To begin with I want to Ensure my Db exists and to seed it with data:

// Called from Main
db.Database.EnsureDeleted();
db.Database.EnsureCreated();

void SeedData()
{
    // Add some parent records using Add or AddRange

    // Add some child records using Add or AddRange
}

RunQuery();   // see below

Then, after adding that data, I want to query my DbSet and return only a Parent record - without Includ'ing Children.

var parent = db.Parent.SingleOrDefault(...);

How can I ensure that the Children won't be attached?

int? count = parent.Children?.Count;     // should be null

dneimke commented Nov 23, 2015

True. Another example is that I want to write a test console app. To begin with I want to Ensure my Db exists and to seed it with data:

// Called from Main
db.Database.EnsureDeleted();
db.Database.EnsureCreated();

void SeedData()
{
    // Add some parent records using Add or AddRange

    // Add some child records using Add or AddRange
}

RunQuery();   // see below

Then, after adding that data, I want to query my DbSet and return only a Parent record - without Includ'ing Children.

var parent = db.Parent.SingleOrDefault(...);

How can I ensure that the Children won't be attached?

int? count = parent.Children?.Count;     // should be null
@ilmax

This comment has been minimized.

Show comment
Hide comment
@ilmax

ilmax Nov 23, 2015

Contributor

I would downvote this if possible, lazy loading is a source of problem, and hides some db work. Explicit is better than implicit IMHO!
I prefer EF Core never implement this.

Contributor

ilmax commented Nov 23, 2015

I would downvote this if possible, lazy loading is a source of problem, and hides some db work. Explicit is better than implicit IMHO!
I prefer EF Core never implement this.

@rowanmiller

This comment has been minimized.

Show comment
Hide comment
@rowanmiller

rowanmiller Dec 1, 2015

Member

@dneimke you should use a separate instance of your context for the seeding.

Member

rowanmiller commented Dec 1, 2015

@dneimke you should use a separate instance of your context for the seeding.

@AndiRudi

This comment has been minimized.

Show comment
Hide comment
@AndiRudi

AndiRudi Dec 2, 2015

@ilmax I don't think that easying the writing of code has anything to do with hiding. If you understand lazy loading, you don't have issues. Let's just go into the example of @rowanmiller above.

Explicit:

Course course = context.Courses.SingleOrDefault(c => c.Id = 1);
//This translates more or less to SELECT * FROM COURSE WHERE Id = 1

// Sometime later, load the related data
context.Bookings.Where(b => b.CourseId == course.CourseId).ToList();
//This translates more or less to SELECT * FROM Bookings WHERE CourseId = ...

Lazy:

Course course = context.Courses.SingleOrDefault(c => c.Id = 1);
//This translates more or less to SELECT * FROM COURSE WHERE Id = 1

// Sometime later, load the related data
course.Bookings.ToList();
//This translates more or less to SELECT * FROM Bookings WHERE CourseId = ...

I can't see any difference here, but I see the following improvements

  1. The data context used by the second call automatically is the same than used by the first one. This is important for the Unit-Of-Work. In the explicit call a developer could even create another data context (ok, thats not directly true because correct injection would take care of this, but it feels better)
  2. I don't want to query for the context all the time. This is boilerplate code.
  3. It is easier to read and more ddd than.

AndiRudi commented Dec 2, 2015

@ilmax I don't think that easying the writing of code has anything to do with hiding. If you understand lazy loading, you don't have issues. Let's just go into the example of @rowanmiller above.

Explicit:

Course course = context.Courses.SingleOrDefault(c => c.Id = 1);
//This translates more or less to SELECT * FROM COURSE WHERE Id = 1

// Sometime later, load the related data
context.Bookings.Where(b => b.CourseId == course.CourseId).ToList();
//This translates more or less to SELECT * FROM Bookings WHERE CourseId = ...

Lazy:

Course course = context.Courses.SingleOrDefault(c => c.Id = 1);
//This translates more or less to SELECT * FROM COURSE WHERE Id = 1

// Sometime later, load the related data
course.Bookings.ToList();
//This translates more or less to SELECT * FROM Bookings WHERE CourseId = ...

I can't see any difference here, but I see the following improvements

  1. The data context used by the second call automatically is the same than used by the first one. This is important for the Unit-Of-Work. In the explicit call a developer could even create another data context (ok, thats not directly true because correct injection would take care of this, but it feels better)
  2. I don't want to query for the context all the time. This is boilerplate code.
  3. It is easier to read and more ddd than.
@jemiller0

This comment has been minimized.

Show comment
Hide comment
@jemiller0

jemiller0 Dec 2, 2015

I think a lot of people get burned by lazy loading. I can understand why some people don't like it. I think it's a nice feature to have, but, I can understand why people don't like it. Namely, because when you develop an app, you need to watch the SQL logs and make sure the ORM isn't kicking off a lot of N+1 queries and the like. I haven't seen much of that with EF, but, had issues with this with EclipseLink on another platform. As long as there is a way to get everything you need loaded in an efficient manner, then, I would probably be all right with not having lazy loading. The new way that EF loads collections looks cool to me. I'm hopeful that this is more efficient than N+1 queries.

jemiller0 commented Dec 2, 2015

I think a lot of people get burned by lazy loading. I can understand why some people don't like it. I think it's a nice feature to have, but, I can understand why people don't like it. Namely, because when you develop an app, you need to watch the SQL logs and make sure the ORM isn't kicking off a lot of N+1 queries and the like. I haven't seen much of that with EF, but, had issues with this with EclipseLink on another platform. As long as there is a way to get everything you need loaded in an efficient manner, then, I would probably be all right with not having lazy loading. The new way that EF loads collections looks cool to me. I'm hopeful that this is more efficient than N+1 queries.

@ilmax

This comment has been minimized.

Show comment
Hide comment
@ilmax

ilmax Dec 2, 2015

Contributor

@AndiRudi my opinion about lazy loading is different, here are my concerns

  1. it's not simplify but hiding (at least IMHO) because a non EF expert cannot see something EF related in course.Bookings.ToList() and explicit is better than implicit.
  2. Requires that your model matches some EF requirements (e.g. virtual navigation properties) and I prefer avoid.
  3. It's super easy to fall in the SELECT N+1 trap
Contributor

ilmax commented Dec 2, 2015

@AndiRudi my opinion about lazy loading is different, here are my concerns

  1. it's not simplify but hiding (at least IMHO) because a non EF expert cannot see something EF related in course.Bookings.ToList() and explicit is better than implicit.
  2. Requires that your model matches some EF requirements (e.g. virtual navigation properties) and I prefer avoid.
  3. It's super easy to fall in the SELECT N+1 trap
@AndiRudi

This comment has been minimized.

Show comment
Hide comment
@AndiRudi

AndiRudi Dec 2, 2015

@ilmax I see... but then we don't need the navigation properties (.Bookings) at all? Then I always have to query the database with context.Bookings to make sure I have the correct data and using .Bookings would always be kind of "I dont know whats in there"....

The only case it would make sense is when using .Include(). And .Include() only makes sense when you write everything in one method because you'll never know what data is needed later (exception you write everything in one method).

AndiRudi commented Dec 2, 2015

@ilmax I see... but then we don't need the navigation properties (.Bookings) at all? Then I always have to query the database with context.Bookings to make sure I have the correct data and using .Bookings would always be kind of "I dont know whats in there"....

The only case it would make sense is when using .Include(). And .Include() only makes sense when you write everything in one method because you'll never know what data is needed later (exception you write everything in one method).

@jemiller0

This comment has been minimized.

Show comment
Hide comment
@jemiller0

jemiller0 Dec 2, 2015

OK, you convinced me. I think lazy loading should be there as well. I'm surprised that even explicit loading isn't there. I think that tells you where EF 7 is. It is a rewrite and important features are still missing. EF is my preferred ORM for .NET, but, it has always lagged behind other ORMs such as NHibernate. Microsoft was late to the game to begin with. In some respects, they still aren't to the point where NHibernate was years ago (although in other respects they are ahead). Unfortunately, RedHat bought JBoss and they jettisoned NHibernate. Last time I checked, the new developers couldn't seem to even figure out how to put a link to the documentation on their website. So, I will stick with EF, even if it isn't perfect. I have a feeling we will be waiting for quite some time though before we see lazy loading.

jemiller0 commented Dec 2, 2015

OK, you convinced me. I think lazy loading should be there as well. I'm surprised that even explicit loading isn't there. I think that tells you where EF 7 is. It is a rewrite and important features are still missing. EF is my preferred ORM for .NET, but, it has always lagged behind other ORMs such as NHibernate. Microsoft was late to the game to begin with. In some respects, they still aren't to the point where NHibernate was years ago (although in other respects they are ahead). Unfortunately, RedHat bought JBoss and they jettisoned NHibernate. Last time I checked, the new developers couldn't seem to even figure out how to put a link to the documentation on their website. So, I will stick with EF, even if it isn't perfect. I have a feeling we will be waiting for quite some time though before we see lazy loading.

@AndiRudi

This comment has been minimized.

Show comment
Hide comment
@AndiRudi

AndiRudi Dec 4, 2015

Today i tried the new C# Interactive window and I think this is another +1 for Lazy loading. Maybe not as important, but quite cool.

Microsoft (R) Roslyn C# Compiler version 1.1.0.51109
Loading context from 'CSharpInteractive.rsp'.
Type "#help" for more information.
> #r "C:\coursika\Stories\2302-UpdatePurchaseOrder\Coursika.Core\bin\Debug\Coursika.Core.dll"
. using Coursika;
. var db = new CourseContext("Data Source=localhost;Initial Catalog=CoursikaProd;Integrated Security=SSPI;MultipleActiveResultSets=True");
. var course = db.Courses.FirstOrDefault();
. Console.WriteLine(course.Name.ToString());
. course.Bookings.Where(c => c.BookingStatus == Coursika.Models.BookingStatus.Booked).ToList().ForEach(c => Console.WriteLine(c.BookingStatus));
. 
Kurs 17 Pilates am Vormittag
Booked
Booked
Booked
Booked
Booked
Booked
> 

AndiRudi commented Dec 4, 2015

Today i tried the new C# Interactive window and I think this is another +1 for Lazy loading. Maybe not as important, but quite cool.

Microsoft (R) Roslyn C# Compiler version 1.1.0.51109
Loading context from 'CSharpInteractive.rsp'.
Type "#help" for more information.
> #r "C:\coursika\Stories\2302-UpdatePurchaseOrder\Coursika.Core\bin\Debug\Coursika.Core.dll"
. using Coursika;
. var db = new CourseContext("Data Source=localhost;Initial Catalog=CoursikaProd;Integrated Security=SSPI;MultipleActiveResultSets=True");
. var course = db.Courses.FirstOrDefault();
. Console.WriteLine(course.Name.ToString());
. course.Bookings.Where(c => c.BookingStatus == Coursika.Models.BookingStatus.Booked).ToList().ForEach(c => Console.WriteLine(c.BookingStatus));
. 
Kurs 17 Pilates am Vormittag
Booked
Booked
Booked
Booked
Booked
Booked
> 
@heraclesaraujo

This comment has been minimized.

Show comment
Hide comment
@heraclesaraujo

heraclesaraujo Dec 4, 2015

I'd vote for this to be something separated, like a plugin.

use dependency injection for creating a lazy loaded proxy

app.UseEntityFramework().UseSqlServer().UseLazyLoading()

or

some kind of explicit mapping for the entity
HasMany(model=>model.Collection).LazyLoad();

heraclesaraujo commented Dec 4, 2015

I'd vote for this to be something separated, like a plugin.

use dependency injection for creating a lazy loaded proxy

app.UseEntityFramework().UseSqlServer().UseLazyLoading()

or

some kind of explicit mapping for the entity
HasMany(model=>model.Collection).LazyLoad();

@yukozh

This comment has been minimized.

Show comment
Hide comment
@yukozh

yukozh Dec 4, 2015

Contributor

I'd vote for this to be something separated, like a plugin.

use dependency injection for creating a lazy loaded proxy

app.UseEntityFramework().UseSqlServer().UseLazyLoading()

or

some kind of explicit mapping for the entity
HasMany(model=>model.Collection).LazyLoad();

👍 +1

Or SomeDbContext.SomeDbSet.Where(xxx).LazyLoad();

Contributor

yukozh commented Dec 4, 2015

I'd vote for this to be something separated, like a plugin.

use dependency injection for creating a lazy loaded proxy

app.UseEntityFramework().UseSqlServer().UseLazyLoading()

or

some kind of explicit mapping for the entity
HasMany(model=>model.Collection).LazyLoad();

👍 +1

Or SomeDbContext.SomeDbSet.Where(xxx).LazyLoad();

@Spongman

This comment has been minimized.

Show comment
Hide comment
@Spongman

Spongman Dec 11, 2015

please consider un-removing lazy-loading. I don't have an issue with navigation properties 'hiding' database calls, but I do have an issue where the context itself is not necessarily available in scope. the problem with explicit loading is that all the code now needs to be context-aware which means you need to thread the context instance throughout your code. a significant part of our code navigates our entity graph via the navigation properties and entity extension methods, and the context is just not in scope. also we have found .Include() to be woefully inadequate due to the exponential size of the result set for non-trivial graph fetches.

Spongman commented Dec 11, 2015

please consider un-removing lazy-loading. I don't have an issue with navigation properties 'hiding' database calls, but I do have an issue where the context itself is not necessarily available in scope. the problem with explicit loading is that all the code now needs to be context-aware which means you need to thread the context instance throughout your code. a significant part of our code navigates our entity graph via the navigation properties and entity extension methods, and the context is just not in scope. also we have found .Include() to be woefully inadequate due to the exponential size of the result set for non-trivial graph fetches.

@alaatm

This comment has been minimized.

Show comment
Hide comment
@alaatm

alaatm Dec 17, 2015

I hope lazy-loading can make it to RTM. You can have it off by default and have API to explicitly enable it so that users are aware of the implications. At the end of day, lazy-loading has many advantages specially in the case where a DbContext instance shouldn't be exposed in certain places, e.g. model classes.

alaatm commented Dec 17, 2015

I hope lazy-loading can make it to RTM. You can have it off by default and have API to explicitly enable it so that users are aware of the implications. At the end of day, lazy-loading has many advantages specially in the case where a DbContext instance shouldn't be exposed in certain places, e.g. model classes.

@Boglen

This comment has been minimized.

Show comment
Hide comment
@Boglen

Boglen Jan 1, 2016

Or something like

Db.Parent.Single(p => p.Id == 1).WithLazy(p => p.ChildsThatIDontKnowUseOrNot);

Boglen commented Jan 1, 2016

Or something like

Db.Parent.Single(p => p.Id == 1).WithLazy(p => p.ChildsThatIDontKnowUseOrNot);

@Grinderofl

This comment has been minimized.

Show comment
Hide comment
@Grinderofl

Grinderofl Jan 9, 2016

Contributor

HasMany(model=>model.Collection).LazyLoad(); would work the best. This way, each individual property can be configured. There could also be a convention which enables Lazy Loading on properties by default if needed.

Contributor

Grinderofl commented Jan 9, 2016

HasMany(model=>model.Collection).LazyLoad(); would work the best. This way, each individual property can be configured. There could also be a convention which enables Lazy Loading on properties by default if needed.

@farlop

This comment has been minimized.

Show comment
Hide comment
@farlop

farlop Jan 12, 2016

+1 for Lazy Load. Maybe the best approach could be leaving it enabled by default (as current EF6) and add some mechanism to turn it off globally and use an extension method to enable it where the developer decides (on a single query or on a single relationship).

farlop commented Jan 12, 2016

+1 for Lazy Load. Maybe the best approach could be leaving it enabled by default (as current EF6) and add some mechanism to turn it off globally and use an extension method to enable it where the developer decides (on a single query or on a single relationship).

@TrabacchinLuigi

This comment has been minimized.

Show comment
Hide comment
@TrabacchinLuigi

TrabacchinLuigi Jan 14, 2016

I rarely use lazy loading, it only gave me headaches (how many of us end up hitting the whole database after a serialization?), i personally prefer eagerly loading, i won't change anything, i really like how good the new change tracker is, just use the same context if you want something related, or use a new one for unrelated stuff.
+1 for eagerly loading, makes everything easier to understand

TrabacchinLuigi commented Jan 14, 2016

I rarely use lazy loading, it only gave me headaches (how many of us end up hitting the whole database after a serialization?), i personally prefer eagerly loading, i won't change anything, i really like how good the new change tracker is, just use the same context if you want something related, or use a new one for unrelated stuff.
+1 for eagerly loading, makes everything easier to understand

@Garrus007

This comment has been minimized.

Show comment
Hide comment
@Garrus007

Garrus007 Jan 15, 2016

I facing same problem and tried to solve it using Include, but VS says, that this method is undefined.

Garrus007 commented Jan 15, 2016

I facing same problem and tried to solve it using Include, but VS says, that this method is undefined.

@divega

This comment has been minimized.

Show comment
Hide comment
@divega

divega Jan 15, 2016

Member

@Garrus007 Include() and ThenInclude() are extension methods for IQueryable<T> defined in the Microsoft.Data.Entity namespace. Are you missing a using statement with that namespace in your code?

Member

divega commented Jan 15, 2016

@Garrus007 Include() and ThenInclude() are extension methods for IQueryable<T> defined in the Microsoft.Data.Entity namespace. Are you missing a using statement with that namespace in your code?

@Garrus007

This comment has been minimized.

Show comment
Hide comment
@Garrus007

Garrus007 Jan 15, 2016

@divega Oh, greath thanks.

I tired solving progblem like this

Course course = context.Courses.SingleOrDefault(c => c.Id = 1);

// sometime later, load the related data
context.Bookings.Where(b => b.CourseId == course.CourseId).ToList();

and using Include() helps.

Garrus007 commented Jan 15, 2016

@divega Oh, greath thanks.

I tired solving progblem like this

Course course = context.Courses.SingleOrDefault(c => c.Id = 1);

// sometime later, load the related data
context.Bookings.Where(b => b.CourseId == course.CourseId).ToList();

and using Include() helps.

@Grinderofl

This comment has been minimized.

Show comment
Hide comment
@Grinderofl

Grinderofl Jan 15, 2016

Contributor

Lazy Loading is an integral pattern of enterprise application architecture. NHibernate + Fluent NHibernate is a good example of a well-structured and built lazy loading, where one specifies the behaviour from entity to entity. What is happening here is a requirement of heavy coupling of the ORM with business rules and optimization, and the necessity for business layer to know too much about the logic of the data source, with Include() statements. That does not domain driven design fit.

I'd be willing to give a go with implementing it, if there is a proposed mechanism for that as well as the best way to do that? I would imagine adding the feature to Relational and derivatives with perhaps extension methods akin to ReferenceCollectionBuilder.LazyLoad(LazyLoadingOptions) => LazyLoadingOptions = enum{ None, Select, Join } ?

@rowanmiller Thoughts?

Contributor

Grinderofl commented Jan 15, 2016

Lazy Loading is an integral pattern of enterprise application architecture. NHibernate + Fluent NHibernate is a good example of a well-structured and built lazy loading, where one specifies the behaviour from entity to entity. What is happening here is a requirement of heavy coupling of the ORM with business rules and optimization, and the necessity for business layer to know too much about the logic of the data source, with Include() statements. That does not domain driven design fit.

I'd be willing to give a go with implementing it, if there is a proposed mechanism for that as well as the best way to do that? I would imagine adding the feature to Relational and derivatives with perhaps extension methods akin to ReferenceCollectionBuilder.LazyLoad(LazyLoadingOptions) => LazyLoadingOptions = enum{ None, Select, Join } ?

@rowanmiller Thoughts?

@strich

This comment has been minimized.

Show comment
Hide comment
@strich

strich Jan 20, 2016

Seems like this feature barely needs another +1, however whilst this is being sorted out can we please put specific mention of the lack of this feature and some example Include() code in the EF Core and ASP.NET Core docs? I just spent the last 2 days trying to figure out why my project wasn't working with one-to-many relationship and almost gave up on EF entirely.

The Blog example here specifically could benefit from an additional step showing the preferred manual method of using the one-to-many relationship between the Blog and its Posts.

strich commented Jan 20, 2016

Seems like this feature barely needs another +1, however whilst this is being sorted out can we please put specific mention of the lack of this feature and some example Include() code in the EF Core and ASP.NET Core docs? I just spent the last 2 days trying to figure out why my project wasn't working with one-to-many relationship and almost gave up on EF entirely.

The Blog example here specifically could benefit from an additional step showing the preferred manual method of using the one-to-many relationship between the Blog and its Posts.

@hikalkan

This comment has been minimized.

Show comment
Hide comment
@hikalkan

hikalkan Jan 11, 2018

Thanks @ajcvickers for your time to answer.

hikalkan commented Jan 11, 2018

Thanks @ajcvickers for your time to answer.

ajcvickers added a commit that referenced this issue Jan 18, 2018

Lazy-loading proxies package
Part of #10509, #3797.

This change adds a new package--Microsoft.EntityFrameworkCore.Proxies--that contains a lazy-loading proxy implementation making use of the EF lazy-loading infrastructure and the Castle.Core proxies package. Current plan is for this to ship on NuGet as an optional package for use with EF.

To use in a normal application, just add a call to `UseLazyLoadingProxies()` on the DbContext options builder.

There are also `context.CreateProxy()` methods for creating stand-alone proxy instances if needed.

Entity types must be public and navigation properties must be virtual. Also, the entity type and constructor must be "visible" to the castle proxies assembly, which usually means public, but could mean internal if "InternalsVisibleTo" is used. Exceptions are thrown if these requirements are not met--you don't just not get a proxy like in EF6.

Note that this is an optional package for EF that we chose to create because the infrastructure in place made it easy to do so. It does not preclude a Roslyn-based rewriting solution in the future.

ajcvickers added a commit that referenced this issue Jan 19, 2018

Lazy-loading proxies package
Part of #10509, #3797.

This change adds a new package--Microsoft.EntityFrameworkCore.Proxies--that contains a lazy-loading proxy implementation making use of the EF lazy-loading infrastructure and the Castle.Core proxies package. Current plan is for this to ship on NuGet as an optional package for use with EF.

To use in a normal application, just add a call to `UseLazyLoadingProxies()` on the DbContext options builder.

There are also `context.CreateProxy()` methods for creating stand-alone proxy instances if needed.

Entity types must be public and navigation properties must be virtual. Also, the entity type and constructor must be "visible" to the castle proxies assembly, which usually means public, but could mean internal if "InternalsVisibleTo" is used. Exceptions are thrown if these requirements are not met--you don't just not get a proxy like in EF6.

Note that this is an optional package for EF that we chose to create because the infrastructure in place made it easy to do so. It does not preclude a Roslyn-based rewriting solution in the future.
@ajcvickers

This comment has been minimized.

Show comment
Hide comment
@ajcvickers

ajcvickers Jan 19, 2018

Member

PR #10720 is an initial implementation of a lazy-loading proxies package.

This change adds a new package--Microsoft.EntityFrameworkCore.Proxies--that contains a lazy-loading proxy implementation making use of the EF lazy-loading infrastructure and the Castle.Core proxies package. Current plan is for this to ship on NuGet as an optional package for use with EF Core.

To use in a normal application, just add a call to UseLazyLoadingProxies() on the DbContext options builder.

There are also context.CreateProxy() methods for creating stand-alone proxy instances if needed.

Entity types must be public and navigation properties must be virtual. Also, the entity type and constructor must be "visible" to the castle proxies assembly, which usually means public, but could mean internal if "InternalsVisibleTo" is used. Exceptions are thrown if these requirements are not met--you don't just get a non-proxy like in EF6.

Note that this is an optional package for EF Core that we chose to create because the infrastructure in place made it easy to do so. It does not preclude a Roslyn-based rewriting solution in the future.

Member

ajcvickers commented Jan 19, 2018

PR #10720 is an initial implementation of a lazy-loading proxies package.

This change adds a new package--Microsoft.EntityFrameworkCore.Proxies--that contains a lazy-loading proxy implementation making use of the EF lazy-loading infrastructure and the Castle.Core proxies package. Current plan is for this to ship on NuGet as an optional package for use with EF Core.

To use in a normal application, just add a call to UseLazyLoadingProxies() on the DbContext options builder.

There are also context.CreateProxy() methods for creating stand-alone proxy instances if needed.

Entity types must be public and navigation properties must be virtual. Also, the entity type and constructor must be "visible" to the castle proxies assembly, which usually means public, but could mean internal if "InternalsVisibleTo" is used. Exceptions are thrown if these requirements are not met--you don't just get a non-proxy like in EF6.

Note that this is an optional package for EF Core that we chose to create because the infrastructure in place made it easy to do so. It does not preclude a Roslyn-based rewriting solution in the future.

@FSou1

This comment has been minimized.

Show comment
Hide comment
@FSou1

FSou1 Jan 28, 2018

I'm sorry for probably a dummy question, but I'm a little bit confused why my User property always loaded by itself :)

I even would like to Ignore it, because otherwise I have to set ReferenceLoopHandling = ReferenceLoopHandling.Ignore.

I'll be really happy to have an opportunity to prevent loading every of my properties and use .Include only when I need it.

Here is an example of my model:

    public class ApplicationUser : IdentityUser
    {
        public virtual List<Alert> Alerts { get; set; }
    }

    public class Alert
    {
        [Required]
        public virtual ApplicationUser User { get; set; }
    }

The result of this method will be a collection of alerts and every entity will contain a user, and the user will contain all related alerts e.t.c.:

public async Task<List<Alert>> GetUserAlerts(ApplicationUser user)
{
    return await _context.Alerts.ToListAsync();
}

Is it a proper behavior?

FSou1 commented Jan 28, 2018

I'm sorry for probably a dummy question, but I'm a little bit confused why my User property always loaded by itself :)

I even would like to Ignore it, because otherwise I have to set ReferenceLoopHandling = ReferenceLoopHandling.Ignore.

I'll be really happy to have an opportunity to prevent loading every of my properties and use .Include only when I need it.

Here is an example of my model:

    public class ApplicationUser : IdentityUser
    {
        public virtual List<Alert> Alerts { get; set; }
    }

    public class Alert
    {
        [Required]
        public virtual ApplicationUser User { get; set; }
    }

The result of this method will be a collection of alerts and every entity will contain a user, and the user will contain all related alerts e.t.c.:

public async Task<List<Alert>> GetUserAlerts(ApplicationUser user)
{
    return await _context.Alerts.ToListAsync();
}

Is it a proper behavior?

@ajcvickers

This comment has been minimized.

Show comment
Hide comment
@ajcvickers

ajcvickers Jan 29, 2018

Member

@FSou1 EF always keeps pairs of navigation properties in sync so that the graph remains in a consistent state with respect to its relationships. Include tells EF to load a given relationship, which may have one or two navigation properties associated with it--if there are two, EF keeps them in sync.

Member

ajcvickers commented Jan 29, 2018

@FSou1 EF always keeps pairs of navigation properties in sync so that the graph remains in a consistent state with respect to its relationships. Include tells EF to load a given relationship, which may have one or two navigation properties associated with it--if there are two, EF keeps them in sync.

@FSou1

This comment has been minimized.

Show comment
Hide comment
@FSou1

FSou1 Jan 29, 2018

@ajcvickers Ok, thank you. And is there a workaround to make loading of these pairs of navigation properties optional?

FSou1 commented Jan 29, 2018

@ajcvickers Ok, thank you. And is there a workaround to make loading of these pairs of navigation properties optional?

@ajcvickers

This comment has been minimized.

Show comment
Hide comment
@ajcvickers

ajcvickers Jan 29, 2018

Member

@FSou1 No, other than if you only use the navigation in one direction, then don't include the inverse in the model at all.

Member

ajcvickers commented Jan 29, 2018

@FSou1 No, other than if you only use the navigation in one direction, then don't include the inverse in the model at all.

ajcvickers added a commit that referenced this issue Jan 31, 2018

Enable property injection of services, mostly for lazy-loading
Part of issues #10509 #3797

This change introduces a new kind of property: IServiceProperty. These properties are bound to some service of the context, such as ILazyLoader. They can be constructor injected as before, but can also be set as a property both in materialization and on Attach/Detach. When attaching, the service property is set to the service from the current context. When detaching, the service is set to null.

Lazy-loading proxies add an ILazyLoader property to the proxied entity. These means that lazy-loading is disabled when the entity is detached and can then be safely used after the context has been disposed. (It is also possible to disable the context disposed warning for this.) Also, if a proxy is attached to a new context, then it will start doing lazy-loading using that new context.

The service property can also be used when doing lazy-loading without proxies to get the same behavior on detach/attach. In this case it needs to be added to the entity explicitly, but then can be discovered by convention.
@bcemmett

This comment has been minimized.

Show comment
Hide comment
@bcemmett

bcemmett Jan 31, 2018

@ajcvickers really cool to see this happening. Since it's a separate to the main EFCore library, do you plan to ship the proxies package soonish, or does it need to wait to go out at the same time as .NET Core 2.1? It'd be really helpful for something I need to do in the next couple of weeks, so any massive gotchas if I built that package myself and used it alongside EFCore 2.0?

bcemmett commented Jan 31, 2018

@ajcvickers really cool to see this happening. Since it's a separate to the main EFCore library, do you plan to ship the proxies package soonish, or does it need to wait to go out at the same time as .NET Core 2.1? It'd be really helpful for something I need to do in the next couple of weeks, so any massive gotchas if I built that package myself and used it alongside EFCore 2.0?

@ajcvickers

This comment has been minimized.

Show comment
Hide comment
@ajcvickers

ajcvickers Jan 31, 2018

Member

@bcemmett The proxies package depends on the lazy-loading infrastructure added to 2.1. Unfortunately it would not be feasible to built it against 2.0.

Member

ajcvickers commented Jan 31, 2018

@bcemmett The proxies package depends on the lazy-loading infrastructure added to 2.1. Unfortunately it would not be feasible to built it against 2.0.

ajcvickers added a commit that referenced this issue Feb 1, 2018

Enable property injection of services, mostly for lazy-loading
Part of issues #10509 #3797

This change introduces a new kind of property: IServiceProperty. These properties are bound to some service of the context, such as ILazyLoader. They can be constructor injected as before, but can also be set as a property both in materialization and on Attach/Detach. When attaching, the service property is set to the service from the current context. When detaching, the service is set to null.

Lazy-loading proxies add an ILazyLoader property to the proxied entity. These means that lazy-loading is disabled when the entity is detached and can then be safely used after the context has been disposed. (It is also possible to disable the context disposed warning for this.) Also, if a proxy is attached to a new context, then it will start doing lazy-loading using that new context.

The service property can also be used when doing lazy-loading without proxies to get the same behavior on detach/attach. In this case it needs to be added to the entity explicitly, but then can be discovered by convention.

ajcvickers added a commit that referenced this issue Feb 1, 2018

Throw when attempting to lazy-load after no-tracking query
Part of #10042, #10509, #3797

It would be good to make this work in a future release, but this involves running a no-tracking query with fixup where the root entity is already materialized. For now, this will throw so we can make it work later without it being a breaking change.

Lazy-loading behaviors for non-tracked entities:
* Proxies:
  * No-op if entity was explicitly detached
  * Throw if it was queried as no-tracking
* Lazy-loading entities with loader service property:
  * No-op if entity was explicitly detached
  * Throw if it was queried as no-tracking
* Lazy-loading entities without service property:
  * Throw even if entity was explicitly detached, but entity can set loader to null, or a service property can be defined
  * Throw if it was queried as no-tracking

ajcvickers added a commit that referenced this issue Feb 5, 2018

Throw when attempting to lazy-load after no-tracking query
Part of #10042, #10509, #3797

It would be good to make this work in a future release, but this involves running a no-tracking query with fixup where the root entity is already materialized. For now, this will throw so we can make it work later without it being a breaking change.

Lazy-loading behaviors for non-tracked entities:
* Proxies:
  * No-op if entity was explicitly detached
  * Throw if it was queried as no-tracking
* Lazy-loading entities with loader service property:
  * No-op if entity was explicitly detached
  * Throw if it was queried as no-tracking
* Lazy-loading entities without service property:
  * Throw even if entity was explicitly detached, but entity can set loader to null, or a service property can be defined
  * Throw if it was queried as no-tracking
@dasMulli

This comment has been minimized.

Show comment
Hide comment
@dasMulli

dasMulli Feb 5, 2018

Glad to see basic support for lazy loading coming in 2.1.

However, is there going to be a story for doing the lazy loading asynchronously?

The API may not be as sleek as blog.Posts… but having the option to go async would help preventing thread pool exhaustion issues due to blocking calls in high load sceanrios.

Are there any other options that could help here? I could also think of making the navigation property accessible as IQueryable<T> to call .ToListAsync() on..

dasMulli commented Feb 5, 2018

Glad to see basic support for lazy loading coming in 2.1.

However, is there going to be a story for doing the lazy loading asynchronously?

The API may not be as sleek as blog.Posts… but having the option to go async would help preventing thread pool exhaustion issues due to blocking calls in high load sceanrios.

Are there any other options that could help here? I could also think of making the navigation property accessible as IQueryable<T> to call .ToListAsync() on..

@ajcvickers

This comment has been minimized.

Show comment
Hide comment
@ajcvickers

ajcvickers Feb 5, 2018

Member

@dasMulli We don't intend to add support for async navigation properties at this time, although this is something we have discussed and may revisit in the future. That being said, I will investigate adding async support to the infrastructure such that async loading could be triggered from your entity.

Member

ajcvickers commented Feb 5, 2018

@dasMulli We don't intend to add support for async navigation properties at this time, although this is something we have discussed and may revisit in the future. That being said, I will investigate adding async support to the infrastructure such that async loading could be triggered from your entity.

ajcvickers added a commit that referenced this issue Feb 7, 2018

Add async infrastructure for lazy-loading
Part of #10509, #3797

Adds ILazyLoader.LoadAsync and allows binding to delegate for it.
@joehoeller

This comment has been minimized.

Show comment
Hide comment
@joehoeller

joehoeller Feb 7, 2018

Add in lazy loading for many to *, and have a good way to handle many to many in controller, not just w linq query in dbcontext file.

joehoeller commented Feb 7, 2018

Add in lazy loading for many to *, and have a good way to handle many to many in controller, not just w linq query in dbcontext file.

ajcvickers added a commit that referenced this issue Feb 7, 2018

Add async infrastructure for lazy-loading
Part of #10509, #3797

Adds ILazyLoader.LoadAsync and allows binding to delegate for it.

ajcvickers added a commit that referenced this issue Feb 7, 2018

Add async infrastructure for lazy-loading
Part of #10509, #3797

Adds ILazyLoader.LoadAsync and allows binding to delegate for it.
@Magic73

This comment has been minimized.

Show comment
Hide comment
@Magic73

Magic73 Feb 10, 2018

Microsoft.EntityFrameworkCore.Proxies Is already available as nuget or do I have to download it manually from github?

Magic73 commented Feb 10, 2018

Microsoft.EntityFrameworkCore.Proxies Is already available as nuget or do I have to download it manually from github?

@firelizzard18

This comment has been minimized.

Show comment
Hide comment
@firelizzard18

firelizzard18 Feb 11, 2018

@Magic73 Based on the fact that a nuget.org search returns no results, I'm going to say you can't get it from NuGet. Additionally, to use EFCore proxies, you need EFCore 2.1, which is not out on nuget.

However, EFCore 2.1 and Proxies (2.1) are available on dotnet.myget.org/feed/aspnetcore-dev (proxies) if you want prerelease versions.

firelizzard18 commented Feb 11, 2018

@Magic73 Based on the fact that a nuget.org search returns no results, I'm going to say you can't get it from NuGet. Additionally, to use EFCore proxies, you need EFCore 2.1, which is not out on nuget.

However, EFCore 2.1 and Proxies (2.1) are available on dotnet.myget.org/feed/aspnetcore-dev (proxies) if you want prerelease versions.

@alexk8

This comment has been minimized.

Show comment
Hide comment
@alexk8

alexk8 Mar 11, 2018

@ajcvickers,
I came here quite late, but...
IMHO, lazy-loading is an "antipattern" and should not have been implemented.
Instead, developers should encourage users to use best-practices (by good documentation), such as context-aware-unit-of-work or something. Example:

//Blog blog  - came from somewhere
var posts = service.EnsureLoadedMany(blog, x=>x.Posts);

plus

//service is context-aware, like this 
class BlogsService{
  //inject DbContext
        public TProp EnsureLoadedOne<T, TProp>(T obj, Expression<Func<T, TProp>> prop) where T : class where TProp : class
        {
            var Ref = ctx.Entry<T>(obj).Reference<TProp>(prop);
            if(!Ref.IsLoaded) Ref.Load();
            return Ref.CurrentValue;
        }
        public IEnumerable<TProp> EnsureLoadedMany<T, TProp>(T obj, Expression<Func<T, IEnumerable<TProp>>> prop) where T : class where TProp : class
        {
            var Ref = ctx.Entry<T>(obj).Collection<TProp>(prop);
            if (!Ref.IsLoaded) Ref.Load();
            return Ref.CurrentValue;
        }

}

that's how I usually do it, and I think that's a good approach. IMHO.
Lazy-loading causes ploblems, like circular-references (entitty->proxy->context->entitystate->entity), preventing garbage collection, unwanted unexpected db-trips, and others

alexk8 commented Mar 11, 2018

@ajcvickers,
I came here quite late, but...
IMHO, lazy-loading is an "antipattern" and should not have been implemented.
Instead, developers should encourage users to use best-practices (by good documentation), such as context-aware-unit-of-work or something. Example:

//Blog blog  - came from somewhere
var posts = service.EnsureLoadedMany(blog, x=>x.Posts);

plus

//service is context-aware, like this 
class BlogsService{
  //inject DbContext
        public TProp EnsureLoadedOne<T, TProp>(T obj, Expression<Func<T, TProp>> prop) where T : class where TProp : class
        {
            var Ref = ctx.Entry<T>(obj).Reference<TProp>(prop);
            if(!Ref.IsLoaded) Ref.Load();
            return Ref.CurrentValue;
        }
        public IEnumerable<TProp> EnsureLoadedMany<T, TProp>(T obj, Expression<Func<T, IEnumerable<TProp>>> prop) where T : class where TProp : class
        {
            var Ref = ctx.Entry<T>(obj).Collection<TProp>(prop);
            if (!Ref.IsLoaded) Ref.Load();
            return Ref.CurrentValue;
        }

}

that's how I usually do it, and I think that's a good approach. IMHO.
Lazy-loading causes ploblems, like circular-references (entitty->proxy->context->entitystate->entity), preventing garbage collection, unwanted unexpected db-trips, and others

@hikalkan

This comment has been minimized.

Show comment
Hide comment
@hikalkan

hikalkan Mar 11, 2018

IMHO, lazy-loading is an "antipattern" and should not have been implemented

As opposite, I think lazy-loading is a good (maybe best) practice for ORMs. Let me explain why;

In DDD, Aggregate roots are considered as transaction units. If we want to make a change on an aggregate root, we normally get it as a whole (including child entities) from data source, make the change and save it as a whole (even if the change is made on a child entity). We are sure that all child collections are available.

NoSQL databases, like MongoDb perfectly works like that by default. However, for relational databases, we need to make joins to get an entity as a whole. If most of the cases we will not work with the child entities, making joins (to get the whole entity with childs pre-loaded) will be unnecessary and not so natural for relational databases.

Lazy loading is the best mimic for truely implementing aggregate roots in relational databases. In this way, when a code works on an aggregate root it does not care if child collections/properties are loaded. They are just loaded on demand by the infrastructure (ORM).

I don't want to call EnsureLoaded method every time when I access to a child collection. There are 3 problems with that approach:

  1. Obviously it's boring.
  2. It makes my code make assumption on ORM/data source and this is an implicit dependency from domain layer to infrastructure layer.
  3. And the most important: the code I write may not have access to the dbcontext (or another abstraction like UOW). Think that the code inside an entity or inside a static context where I can't inject dbcontext or UOW). I definitely don't like to pass dbcontext/uow everywhere in my domain layer.

The only problem with lazy loading is when you query a list of entities from database, work on child properties but forgot to include child properties on the query (this leads to 1+N querying problem). This is obviously a programmer mistake. DDD does not care about such querying. When you query a list from database, you should specifically care about performance, create necessary indexes and carefully write your query.

(BTW, I adready implemented EnsureCollectionLoadedAsync/EnsurePropertyLoadedAsync with repository/uow pattern for ABP framework. But I don't like it and waiting for lazy loading).

hikalkan commented Mar 11, 2018

IMHO, lazy-loading is an "antipattern" and should not have been implemented

As opposite, I think lazy-loading is a good (maybe best) practice for ORMs. Let me explain why;

In DDD, Aggregate roots are considered as transaction units. If we want to make a change on an aggregate root, we normally get it as a whole (including child entities) from data source, make the change and save it as a whole (even if the change is made on a child entity). We are sure that all child collections are available.

NoSQL databases, like MongoDb perfectly works like that by default. However, for relational databases, we need to make joins to get an entity as a whole. If most of the cases we will not work with the child entities, making joins (to get the whole entity with childs pre-loaded) will be unnecessary and not so natural for relational databases.

Lazy loading is the best mimic for truely implementing aggregate roots in relational databases. In this way, when a code works on an aggregate root it does not care if child collections/properties are loaded. They are just loaded on demand by the infrastructure (ORM).

I don't want to call EnsureLoaded method every time when I access to a child collection. There are 3 problems with that approach:

  1. Obviously it's boring.
  2. It makes my code make assumption on ORM/data source and this is an implicit dependency from domain layer to infrastructure layer.
  3. And the most important: the code I write may not have access to the dbcontext (or another abstraction like UOW). Think that the code inside an entity or inside a static context where I can't inject dbcontext or UOW). I definitely don't like to pass dbcontext/uow everywhere in my domain layer.

The only problem with lazy loading is when you query a list of entities from database, work on child properties but forgot to include child properties on the query (this leads to 1+N querying problem). This is obviously a programmer mistake. DDD does not care about such querying. When you query a list from database, you should specifically care about performance, create necessary indexes and carefully write your query.

(BTW, I adready implemented EnsureCollectionLoadedAsync/EnsurePropertyLoadedAsync with repository/uow pattern for ABP framework. But I don't like it and waiting for lazy loading).

@alexk8

This comment has been minimized.

Show comment
Hide comment
@alexk8

alexk8 Mar 11, 2018

I agree that there is only problem, but this problem is Huge problem. Programmers do make mistakes.
By using extra-verb, you explicitly allow platform to go to database. The platform will not go there without that permission. Nobody wants implicit db-trips, especially in loops... especially in nested loops.
So, if lazy-loading exists, it should be clearly visible in every statement where it can happen. And injecting services everywhere is not that bad, when it's "scoped"

alexk8 commented Mar 11, 2018

I agree that there is only problem, but this problem is Huge problem. Programmers do make mistakes.
By using extra-verb, you explicitly allow platform to go to database. The platform will not go there without that permission. Nobody wants implicit db-trips, especially in loops... especially in nested loops.
So, if lazy-loading exists, it should be clearly visible in every statement where it can happen. And injecting services everywhere is not that bad, when it's "scoped"

@ErikEJ

This comment has been minimized.

Show comment
Hide comment
@ErikEJ
Contributor

ErikEJ commented Mar 11, 2018

@dasMulli

This comment has been minimized.

Show comment
Hide comment
@dasMulli

dasMulli Mar 11, 2018

I'm glad there is an option to manually wire it up using async calls, though I'd appreciate if there was some "more natural" async way (ideally utilizing ValueTask<T>).
Since some users already got bitten by sync-over-async processing due to ASP.NET Core not being limited to classic ASP.NET's parallel request handling limit, having synchronous lazy-loading navigation properties may be a bit dangerous for perf-sensitive workloads.
On the other hand, having this opt-in option for simple "I don't need to worry about perf and want to write super simple code" scenarios is very good to have.

dasMulli commented Mar 11, 2018

I'm glad there is an option to manually wire it up using async calls, though I'd appreciate if there was some "more natural" async way (ideally utilizing ValueTask<T>).
Since some users already got bitten by sync-over-async processing due to ASP.NET Core not being limited to classic ASP.NET's parallel request handling limit, having synchronous lazy-loading navigation properties may be a bit dangerous for perf-sensitive workloads.
On the other hand, having this opt-in option for simple "I don't need to worry about perf and want to write super simple code" scenarios is very good to have.

@firelizzard18

This comment has been minimized.

Show comment
Hide comment
@firelizzard18

firelizzard18 Mar 11, 2018

@alexk8 If you don't like it, then don't use it

firelizzard18 commented Mar 11, 2018

@alexk8 If you don't like it, then don't use it

@nicbavetta

This comment has been minimized.

Show comment
Hide comment
@nicbavetta

nicbavetta Mar 13, 2018

@ajcvickers,

Does this new implementation of Lazy Loading support the retrieval of a children of children?
I have the following object structure:

  • Parent1
    • Child1 [public virtual]
      • Child1A (reference back to Parent1) [public virtual]
      • Child1B [public virtual]
    • Child2 [public virtual]

I am using EF Core 2.1 Lazy Loading. When I retrieve Parent1, Child1 and Child2 are loaded, however, Child1A and Child1B are null and remain null even when directly accessed via Parent1.Child1.Child1A and Parent1.Child1.Child1B. I find this odd, especially since Child1A should just be a reference back to Parent1 (which has already been loaded).

nicbavetta commented Mar 13, 2018

@ajcvickers,

Does this new implementation of Lazy Loading support the retrieval of a children of children?
I have the following object structure:

  • Parent1
    • Child1 [public virtual]
      • Child1A (reference back to Parent1) [public virtual]
      • Child1B [public virtual]
    • Child2 [public virtual]

I am using EF Core 2.1 Lazy Loading. When I retrieve Parent1, Child1 and Child2 are loaded, however, Child1A and Child1B are null and remain null even when directly accessed via Parent1.Child1.Child1A and Parent1.Child1.Child1B. I find this odd, especially since Child1A should just be a reference back to Parent1 (which has already been loaded).

@ajcvickers

This comment has been minimized.

Show comment
Hide comment
@ajcvickers

ajcvickers Mar 13, 2018

Member

@nicbavetta Based on the info given it seems this should work. Please file a new issue with a runnable project/solution or complete code listing that demonstrates the issue.

Member

ajcvickers commented Mar 13, 2018

@nicbavetta Based on the info given it seems this should work. Please file a new issue with a runnable project/solution or complete code listing that demonstrates the issue.

@faibistes

This comment has been minimized.

Show comment
Hide comment
@faibistes

faibistes Mar 27, 2018

Does the current (2.1) approach work with non-navigational properties? I have a field in my model which is a large text, and I would like it to be loaded only if I reference it. Can it be done just making the property virtual?

What if I called Include().ToList() to avoid n+1 queries? Would that force the loading of the property? Would it even make sense to use Include() with that property?

faibistes commented Mar 27, 2018

Does the current (2.1) approach work with non-navigational properties? I have a field in my model which is a large text, and I would like it to be loaded only if I reference it. Can it be done just making the property virtual?

What if I called Include().ToList() to avoid n+1 queries? Would that force the loading of the property? Would it even make sense to use Include() with that property?

@optiks

This comment has been minimized.

Show comment
Hide comment
@optiks

optiks Jul 11, 2018

Hi, we're injecting LazyLoader into our entity constructors and then calling LazyLoader.Load() within our navigation properties. (Pssst: I really like this design choice; well done team.)

Anyway, I'm currently dealing with the following scenario, and I'd love to hear your thoughts on the best solution.

  1. Eager-load an entity with a collection navigation property (e.g. var someEntity = ctx.Set<SomeEntity>().Include(x => x.SomeNavigationProperty).FirstOrDefault();).
  2. Dispose the context (e.g. ctx.Dispose();).
  3. Access the collection navigation property, which internally calls LazyLoader.Load (e.g. someEntity.SomeNavigationProperty).
  4. The following exception is thrown:
    System.InvalidOperationException: Error generated for warning 'Microsoft.EntityFrameworkCore.Infrastructure.LazyLoadOnDisposedContextWarning: An attempt was made to lazy-load navigation property 'SomeNavigationProperty' on entity type 'SomeEntity' after the associated DbContext was disposed.'. This exception can be suppressed or logged by passing event ID 'CoreEventId.LazyLoadOnDisposedContextWarning' to the 'ConfigureWarnings' method in 'DbContext.OnConfiguring' or 'AddDbContext'.

The issue with ignoring CoreEventId.LazyLoadOnDisposedContextWarning is that it'll mask the following scenario:

  1. Do NOT eager-load an entity with a collection navigation property (e.g. var someEntity = ctx.Set<SomeEntity>().FirstOrDefault();).
  2. Dispose the context (e.g. ctx.Dispose();).
  3. Access the collection navigation property, which internally calls LazyLoader.Load (e.g. someEntity.SomeNavigationProperty).
  4. In this case, someEntity.SomeNavigationProperty will be an empty collection, instead of throwing an exception.

Any thoughts on what the best way to solve this is? I understand the EF Core predicament: I'm assuming that after the context has been disposed, any knowledge of which navigation properties have been loaded is lost.

Any assistance/guidance would be appreciated.

optiks commented Jul 11, 2018

Hi, we're injecting LazyLoader into our entity constructors and then calling LazyLoader.Load() within our navigation properties. (Pssst: I really like this design choice; well done team.)

Anyway, I'm currently dealing with the following scenario, and I'd love to hear your thoughts on the best solution.

  1. Eager-load an entity with a collection navigation property (e.g. var someEntity = ctx.Set<SomeEntity>().Include(x => x.SomeNavigationProperty).FirstOrDefault();).
  2. Dispose the context (e.g. ctx.Dispose();).
  3. Access the collection navigation property, which internally calls LazyLoader.Load (e.g. someEntity.SomeNavigationProperty).
  4. The following exception is thrown:
    System.InvalidOperationException: Error generated for warning 'Microsoft.EntityFrameworkCore.Infrastructure.LazyLoadOnDisposedContextWarning: An attempt was made to lazy-load navigation property 'SomeNavigationProperty' on entity type 'SomeEntity' after the associated DbContext was disposed.'. This exception can be suppressed or logged by passing event ID 'CoreEventId.LazyLoadOnDisposedContextWarning' to the 'ConfigureWarnings' method in 'DbContext.OnConfiguring' or 'AddDbContext'.

The issue with ignoring CoreEventId.LazyLoadOnDisposedContextWarning is that it'll mask the following scenario:

  1. Do NOT eager-load an entity with a collection navigation property (e.g. var someEntity = ctx.Set<SomeEntity>().FirstOrDefault();).
  2. Dispose the context (e.g. ctx.Dispose();).
  3. Access the collection navigation property, which internally calls LazyLoader.Load (e.g. someEntity.SomeNavigationProperty).
  4. In this case, someEntity.SomeNavigationProperty will be an empty collection, instead of throwing an exception.

Any thoughts on what the best way to solve this is? I understand the EF Core predicament: I'm assuming that after the context has been disposed, any knowledge of which navigation properties have been loaded is lost.

Any assistance/guidance would be appreciated.

@ajcvickers

This comment has been minimized.

Show comment
Hide comment
@ajcvickers

ajcvickers Jul 24, 2018

Member

@optiks I have been thinking about something similar to this to make the experience better when using proxies--see #12780. The idea there is to store the IsLoaded state for each navigation as part of the proxy, so that it travels with the entity instance. You could probably do something similar here--for example:

public ICollection<Post> Posts
{
    get
    {
        if (!_postsIsLoaded)
        {
            LazyLoader.Load(this, ref _posts);
            _postsIsLoaded = true;
        }

        return _posts;
    }
    set => _posts = value;
}

The problem is getting the flag set when EF eagerly loads the collection. When I implement #12780 I will try to do so in a way that there is a hook that can be used both by proxies and non-proxies to make this easier. Until then, I can't think of a way to get EF to set the flag for you on an eager load, but maybe it could be done manually in your code after executing the query.

Member

ajcvickers commented Jul 24, 2018

@optiks I have been thinking about something similar to this to make the experience better when using proxies--see #12780. The idea there is to store the IsLoaded state for each navigation as part of the proxy, so that it travels with the entity instance. You could probably do something similar here--for example:

public ICollection<Post> Posts
{
    get
    {
        if (!_postsIsLoaded)
        {
            LazyLoader.Load(this, ref _posts);
            _postsIsLoaded = true;
        }

        return _posts;
    }
    set => _posts = value;
}

The problem is getting the flag set when EF eagerly loads the collection. When I implement #12780 I will try to do so in a way that there is a hook that can be used both by proxies and non-proxies to make this easier. Until then, I can't think of a way to get EF to set the flag for you on an eager load, but maybe it could be done manually in your code after executing the query.

@optiks

This comment has been minimized.

Show comment
Hide comment
@optiks

optiks Jul 24, 2018

@ajvickers Thanks. We ended up suppressing the warning as the shortest path forward, but we’ll definitely get back to addressing this soon.

I’d imagine, if the loaded state needs to be stored somewhere, on the entity itself is probably the best place. So, if the entity implemented an interface such as INavigationPropertyLoadStateKnowledge, then the LazyLoader.Load(this, ...) call could check, and ignore if it’s already been loaded. Just a thought.

From my experience it’s pretty common for entities to inherit from a common base class, so it’d only need to be done once. It’s definitely some extra overhead, but if developers are already dealing with injecting the lazy loader and invoking, it doesn’t seem unreasonable.

Just setting the state flag in the property getter won’t be sufficient for the case where EF Core is working directly with fields and eager loading.

optiks commented Jul 24, 2018

@ajvickers Thanks. We ended up suppressing the warning as the shortest path forward, but we’ll definitely get back to addressing this soon.

I’d imagine, if the loaded state needs to be stored somewhere, on the entity itself is probably the best place. So, if the entity implemented an interface such as INavigationPropertyLoadStateKnowledge, then the LazyLoader.Load(this, ...) call could check, and ignore if it’s already been loaded. Just a thought.

From my experience it’s pretty common for entities to inherit from a common base class, so it’d only need to be done once. It’s definitely some extra overhead, but if developers are already dealing with injecting the lazy loader and invoking, it doesn’t seem unreasonable.

Just setting the state flag in the property getter won’t be sufficient for the case where EF Core is working directly with fields and eager loading.

@optiks

This comment has been minimized.

Show comment
Hide comment
@optiks

optiks Jul 25, 2018

@ajcvickers If we wanted to handle this on our side, what would be the best way of identifying which references/collections were eager loaded? i.e. if we wanted to set a flag representing that a reference/collection was loaded. Using the ChangeTracker.Tracked event, it looks like it's too early; the IsLoaded flag is still false. i.e.

context.ChangeTracker.Tracked += (s, e) =>
{
   var isLoaded = e.Entry.Collections.Single(x => x.Metadata.Name == nameof(Employee.Devices)).IsLoaded; // this is false
};
var q = context.Set<Employee>().Include(a => a.Devices).ToList();

We can intercept the IQueryProvider.Execute and IEnumerable<T>.GetEnumerator() methods, and then check the ChangeTracker.Entrys, but I'd be interested in hearing if you've got a cleaner solution.

e: After playing around a bit, there's extra complexity to deal with regarding creating new entities/updating collections. I think I'll leave this one for the EF Core team to solve ;).

optiks commented Jul 25, 2018

@ajcvickers If we wanted to handle this on our side, what would be the best way of identifying which references/collections were eager loaded? i.e. if we wanted to set a flag representing that a reference/collection was loaded. Using the ChangeTracker.Tracked event, it looks like it's too early; the IsLoaded flag is still false. i.e.

context.ChangeTracker.Tracked += (s, e) =>
{
   var isLoaded = e.Entry.Collections.Single(x => x.Metadata.Name == nameof(Employee.Devices)).IsLoaded; // this is false
};
var q = context.Set<Employee>().Include(a => a.Devices).ToList();

We can intercept the IQueryProvider.Execute and IEnumerable<T>.GetEnumerator() methods, and then check the ChangeTracker.Entrys, but I'd be interested in hearing if you've got a cleaner solution.

e: After playing around a bit, there's extra complexity to deal with regarding creating new entities/updating collections. I think I'll leave this one for the EF Core team to solve ;).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment