More flexible Include API - allow walking back up include tree #4490
Comments
|
I don't see how can it be simplified. Mind writing a small example? |
|
Take a look at this example. Note that this example is fairly small to elucidate the need, but a real world example could be more complex depending on your needs. This example, the 4th level entity has 3 nav properties that we want included. With the current Api, you need to start all the way back at the top for each of these, correct? See how the AlsoInclude would reference the same parent that then ThenInclude before it referenced. |
|
One issue I see with this is that it only allows you to go back up one level, so it only helps with a certain subset of cases. I'm not saying we shouldn't add it, just that it doesn't prevent you having to go back to the root entity in all cases. There may be another solution where you do some kind of nesting inside the API call... rather than just chaining calls together at the root level (something like the nested closure pattern we use elsewhere). That may end up being super complex though and the code may be unreadable 😄. |
|
In order to make something like For completeness here is how one of the other variations previously requested would look like for this example: _dbContext.Things.Include(x => x.Level1Property)
.ThenInclude(y => y.Level2Property)
.ThenInclude(z => z.Level3Property)
.ThenInclude(a => new
{
a.Level4Property_A,
a.Level4Property_B,
a.Level4Property_C
});Notice that the last call to All these approaches have in common that they require any edge added to the graph of entities to be included to have its starting point on either the query's root type or on the landing type of a previous call to Besides specific limitations such as dotnet/roslyn#8237 this has the potential to provide a very nice intellisense experience for simple queries. But beyond certain threshold of complexity intellisense starts getting in the way instead of helping. Assuming we want to support simple patterns that allow expressing more complex queries in the future, I think we should look into approaches that remove the restriction that edges need to start on a previous type and allow developers to enumerate the edges that need to be traversed without following any particular order, e.g.: _dbContext.Things.Include(x => x.Level1Property)
.ThenInclude(y => y.Level2Property)
.ThenInclude(z => z.Level3Property) // Level3Property is of type Type3
.Include((Type3 t3) => t3.Level4Property_A)
.Include((Type3 t3) => t3.Level4Property_B)
.Include((Type3 t3) => t3.Level4Property_C);Like I remember two examples that followed this approach:
Both used an object that is separate from the query to describe the shape of the graph but I think just adding the edges inline in the query could also work. |
|
In fluid interfaces there is this concept of Perhaps with an expression? |
|
I like the Mark / Return syntax myself, and being consistent with other apis would be good. |
rowanmiller
changed the title
|
I wonder why should we have API that manually resolve include duplicates. |
|
We use a path definition API for includes / eager loading in llblgen pro: the main advantage is that you work with nodes to a graph to which the user can then add additional things, like filters / joins / excluding of fields / limits / ordering etc. for that node. We have a couple of variants on this api, here's one built with extension methods which under the hood translates to the more verbose API we have in the framework https://github.com/SolutionsDesign/LLBLGen.Linq.Prefetch. The tests give a good overview of what you can define at what level. This gives no limits on defining the graph and you can still do it from one method call. Additional documentation of our own API: http://www.llblgen.com/Documentation/5.0/LLBLGen%20Pro%20RTF/Using%20the%20generated%20code/Linq/gencode_linq_prefetchpaths.htm |
|
@FransBouma what stops you from Join / Order / Filter / Limit within the basic pattern of Include()/ThenInclude() + LINQ operations? When you write SQL you join table and can do everything withing your joined table. From my standpoint Include()/ThenInclude() is just smart joins that could attach data into your object, and when you use filtering or limiting it should perform this without any additional work and even more easy than write this directly in SQL. Isn't this why we all using ORMs? I worked with LLBLGen a wile ago and it was pretty painful comparing to Entity Framework. Entity Framework Core gives more abilities to generate queries on the fly with Expression Trees so you can feel free to create dynamic query caching, partial query caching and more flexible way to synchronize data over several applications with distributed cache. You actually can parse any of the EF Core targeted expressions manually and this isn't that painful and this is huge advantage when you can speak with ORM with the powerful Expression Trees instrument. |
|
Ok ok, I just didn't name it right.
Query matters, it depends on what extension methods you have, there the main difference between all LINQ based ORMs.
I would say it's fast enough for ORM and even for in memory cache (complex query).
I'm sorry maybe I posted too a lot critic on the LLBLGen side and mine personal feeling of it. |
Though the set of extension methods in that regard is small: most Linq query methods are the ones in the .NET framework. Eager loading directives are the ones setting things apart, besides smaller ones like excluding fields in a set fetch. Eager loading APIs are a pain to design, I've now done a couple and IMHO the biggest problem to solve is to guide the developer to specify a graph (with potentially multiple branches) using just 1 method call (which contains lambdas with multiple method calls etc.). This often feels unnatural to developers however the alternatives do too. The one I linked to is done by a community member of ours and is actually very elegant and easy to use. With 'easy to use' I mean: it feels natural to define the graph like that as the statement flow in the code look like the graph paths so you see what is fetched as related set where. The biggest hurdle to solve for people is this: how to specify the order details and employee split graph nodes relative to order so it makes sense in the query? This isn't that simple, especially in a sequence oriented DSL like linq.
That's OK, you likely worked with an older version which had our low-level non-linq API (e.g. 2.6 from 2008 ;)). If you look at the tests I linked to the queries look like EF queries as linq queries all look the same, except of course for the eager loading api which is the point of this thread! ;) |
|
@FransBouma I did a look at this The main difference with EF Core here from my understanding is recursive model and additionally you can add .With() to any node, instead of using Include()/ThenInclude() attached to the query itself. UPD But still it's a little complicated, technically this will result in several real queries (as well as every collection Include() in EF Core), but collection Include() in EF Core obvious, this thing isn't :) |
|
@multiarc any chance that could be made as a transformation to EF Core includes statements? I'm thinking about looking into that particular line of enquiry myself. |
|
I don't like What about |
|
Found this thread searching for "AlsoInclude" because I was hoping someone had the same idea as me-- sure enough they did. Ran into this today where I have Product (1) -> Variants (n) -> SubscriptionPlan (1) -> Features (n) & Licenses (n) As I was trying to figure out the chain (which I'm still not even sure how to do properly), it made sense to me that there should be an AlsoInclude, or just at least someway to traverse back up the chain ("also" made sense to me) So, to come all the way back up to Include variants seems counter intuitive. So personally, I liked AlsoInclude. Maybe it only solves one thing, yes, but it would solve that one thing very well :) For the record though, I liked this pattern suggested by divega |
|
One issue with the type-based approach is self-referential tables or tables with cycles. I still think _dbContext.Things.Include(x => x.Level1Property)
.ThenInclude(y => y.Level2Property)
.ThenInclude(z => z.Level3Property)
.ThenInclude(t3 => t3.Level4Property_A)
.Up().ThenInclude(t3 => t3.Level4Property_B)
.Up().ThenInclude(t3 => t3.Level4Property_C); |
|
@jnm2 that's just confusing. I have no idea to which element the Level4Property_B is included in. To define a proper graph on one line, you can simply use the scoping of the language. e.g.: _context.Products
.Where(p => p.Id == productId)
.Include(p => p.Variants)
.ThenInclude(pv => pv.SubscriptionPlan)
.ThenInclude(sp => sp.Licenses)
.AlsoInclude(sp => sp.Features)should become: _context.Products
.Where(p => p.Id == productId)
.Include(p => p.Variants
.Include(pv => pv.SubscriptionPlan
.Include(sp => sp.Licenses)
.Include(sp => sp.Features)
)
)It simply defines the scope the includes work on with the lambda. It's immediately clear into which elements the elements are merged with. |
|
@FransBouma Up() is at least no more confusing than AlsoInclude, and I think less confusing. But I have to agree that yours is the least confusing so far. |
|
Agree. That's probably the best of everything submitted so far, but would intellisense be able to parse it? |
|
The problem is that we'd have to have an Include extension method on every type to handle navigation properties, not just collections. We will have a perennial .Include suggested for every type everywhere in the code, along with Equals, GetHashCode, and ToString. |
|
Note for implementer: see note on #2953 since there is overlap between these two features. |
|
If the following
causes
then that could be fixed by throwing in a wrapper, and exposing the wrapper as a second argument to Include? _context.Products
.Where(p => p.Id == productId)
.Include(p => p.Variants, v=> v
.Include(pv => pv.SubscriptionPlan, s=>s
.Include(sp => sp.Licenses)
.Include(sp => sp.Features)
)
)One of the includes would look a like this: IQueryable<T> Include<T,TProperty>(this IQueryable<T> source, Expression<Func<T,TProperty>> selector, Action<IIncludeGraph<T,TProperty>> nested == null)And I'm guessing, that this could even be implemented on top of the existing system purely as extension methods (and support classes/interfaces), by recursively applying the Includes and ThenIncludes from IIncludeGraph directly on the queryable? and the same graph could be used for #2953 when mapped to the entity's model? |
|
I'm liking the semantics of |
|
Ideally I would love to see an option to configure sort of Database views is a great solution for more or less complex snapshots and the only way to get them more or less efficiently is to map raw SQL query or SP call to DbSet. Fluent LINQ query for that would be much better IMHO. And it sounds quite doable. We just need to add some syntax sugar based on additional Extension methods for The way I tried to explain (ping me if was not clear, my English muscles get weak on evenings) we could potentially reduce the data we grab from DB as we can get this way only those fields we really need. This is to cover the gap we have now - when query is not too complex to consider some DocumentDB sync and query from there, but already too complex to deal with it as we do with CRUD stuff over relational DB. I mean this is for searches with sorting, paging and filtering (for "reach" UI most of the clients love) rather than for reporting / exporting the huge datasets with 10K+ records with lots of nested details. |
|
Seems like this feature might eventually get me to what I need but maybe it doesn't so here is the use case I really need. We have lookup tables that reference other lookup tables and are heavily used in application tables. Example: public class School
{
public StateOrProvince StateOrProvince { get; set; }
}
public class StateOrProvince
{
public Country Country { get; set; }
}To include the children in a query we provide extension methods that pull in all the child entities for the queries that need it. This reduces all this duplicate code that we'd need in all the queries across our system. public static IQueryable<School> WithChildren ( this IQueryable<School> source )
=> source.IncludeStateOrProvince(x => x.StateOrProvince);
public static IQueryable<T, StateOrProvince> IncludeStateOrProvince<T> ( this IQueryable<T> source, Expression<Func<T, StateOProvince>> property )
=> source.Include(property).ThenInclude(x => x.Country);Where things fall apart is when this entity is a navigation property of a higher level object. The public class MedicalEducation
{
public School School { get; set; }
}
public static IQueryable<MedicalEducation> WithChildren ( this IQueryable<MedicalEducation> source )
=> source.IncludeSchool(x => x.School);
public static IQueryable<T, School> IncludeSchool<T> ( this IQueryable<T> source, Expression<Func<T, School>> property )
=> //Works: source.Include(property).ThenInclude(x => x.StateOrProvince).ThenInclude(x => x.Country);
//Doesn't: source.Include(property).ThenIncludeStateOrProvince(x => x.StateOrProvince);
public static IIncludeQueryable<T, StateOrProvince> ThenIncludeStateOrProvince<T> ( ... )The issue is that the return type of Being able to create extension methods that can include children for arbitrarily nested related objects is very useful to us and I believe a more flexible include API might give us this. Right now we are stuck replicating the include logic and it isn't fun. |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
I like the Include().ThenInclude() syntax of EF Core. It would also be great if there was an "AlsoInclude" so that when you want to include another peer several levels deep, you don't have to start at the top (with Include) again. You start getting these really long statements with many include, then include, then include (back to) include, then include, then include, then include, etc. Not a big deal or nothing. It works as is, but syntactically could certainly be more readable if we had a peer level include (ie, whatever entity was "above" the last ThenInclude). Does that make sense?
The text was updated successfully, but these errors were encountered: