-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
Query: Support Include/ThenInclude for navigation on derived type #3910
Comments
@davidroth As far as I know EF Core currently doesn't support any pattern you could use to express that you want query results to include navigations that only exist in derived types. Cc @anpete in case I am wrong. As for how such pattern could look like, here is something we thought about in the past applied to the new ThenInclude method: context.Set<Order>()
.Include(x => x.Positions)
.ThenInclude(x => (x as SalesPosition).Group)
.Include(x => x.Positions)
.ThenInclude(x => (x as GroupPosition).GroupType) Of course, there are probably other things that could work and for instance the null-conditional operator would be better than this if it was supported in LINQ expressions. |
@divega @davidroth I think this same issue applies to Include as well as ThenInclude right? If the type you are querying is a base type then there is no way to Include navigations defined on derived types in the hierarchy. |
Yes, it would apply to both. Only this particular example had it in |
Does this also apply to base abstract classes? public abstract class Entity
{
public DateTime CreatedAt { get; set; }
public DateTime UpdatedAt { get; set; }
public Guid? CreatedById { get; set; }
public Guid? UpdatedById { get; set; }
public User CreatedBy { get; set; }
public User UpdatedBy { get; set; }
} public class Company : Entity
{
...
} context.Set<Company>().Include(t => t.CreatedBy); This results in: |
I'm writing an eager loading routine via graph building and expression trees and I'm running into this problem. Although the way I implemented it actually works as long as the actual database value is of the correct derived class
but it fails when the cast cannot be performed. Unfortunately, EF doesn't like Expression.TypeAs so I cannot combine it directly with Expression.Conditional to invoke a different path if the cast fails. So considering this if the explicit type is saved to DB ( which is anyways saved as a Discriminator field ) the initial check could be included in the Where clause? Does that make sense? You would still need to run multiple queries based on each separate derived class possible to eagerly load all possible combinations |
The
Currently, there's no Why do I want this? I already defined a I don't think the current workaround will work well for me since, after #1833 is implemented, I'll be filtering the upstream results before including the derived properties downstream. |
This is feature is very compelling to me, although it may also be achieved in a shorter way, something like: context.Set<Order>()
.Include(x => x.Positions)
.ThenInclude<SalesPosition>(sp => sp.Group)
.Include(x => x.Positions)
.ThenInclude<GroupPosition>(gp => gp.GroupType)
.ThenInclude(gt => gt.TypeName)
.Include<BusinessOrder>(bo => bo.Customer) Having the |
@divega @anpete @smitpatel this issue popped up in a conversation today |
And also @HappyNomad ditto. Constructing lambda expressions could be painful. There should be a reflection friendly version too, with and without a type filter (supporting inheritance). |
@maumar - What are we planning to do here for now? |
We are investigating supporting this via "as" and string based include for Preview2. |
We have discussed this in brief as a team. Though we haven't finalized exact sketch what it would be since we are bit short on time. Here are some of the thoughts/experiments I did around the topic. String based Include IQueryable<Base>().Include("NavigationOnBase") //We interpret as navigation defined on base
IQueryable<Base>().Include(typeof(Derived), "NavigationOnDerived") // We interpret as navigation on the type arg. This API looks clean. We have similar pattern for string based ModelBuilding support ( Expression based Include // Options 1
IQueryable<Base>().Include(e => e.NavigationOnBase)
IQueryable<Base>().Include(e => (e as Derived).NavigationOnDerived)
//Option 2
IQueryable<Base>().Include<Base, Base, ReferencedType>(e => e.NavigationOnBase)
IQueryable<Base>().Include<Base, Derived, DerivedReferencedType>(e => e.NavigationOnDerived) We are looking to support option 1 here (Explained option 2 below why its bit cumbersome for user experience) Option 2 I am still experimenting with different syntax for expression based include. Our goal is to provide good intellisense to user. We can always add extra overload if we find a good syntax and support the "as" syntax (which is somewhat similar to processing filtered include) Given the time constraint we have, we are doing the "as" syntax and string based as above.
In this syntax, all predicates will be applied to the first IQueryable only and not on the include. If you mean to apply the predicate on the included entity then its filtered include #1833 |
I believe he was referring to a convenient way of dynamically providing the includes, i.e. non LINQ-expressions. The one you mentioned earlier.
You're right.
Yep, that was it. Was just randomly pseudoing.
That's should be
Notice the Thanks! |
…erived type It was not possible to specify include on a derived type. Include was very strict when computing navigation paths from Include method and generated code was accounting for type discrepancies between caller and navigation. Fix is to allow specifying include on derived types using string overload: entities.Include("DerivedProperty") or using lambda with "as" operator: entities.Include(e => (e as Derived).DerivedProperty) Made tweaks to the logic that computes navigation paths - now it looks for navigation on derived types also. Added additional checks to the generated code (_Include method fixup) old: fixup: (QueryContext queryContext | SomeEntity entity | Object[] included) => { return !(bool ReferenceEquals(included[0], null)) { return entity.Navigation = (OtherEntity)included[0] } : default(OtherEntity) } new: fixup: (QueryContext queryContext | SomeEntity entity | Object[] included) => { return !(bool ReferenceEquals(included[0], null)) && (entity is DerivedEntity) ? { return ((DerivedEntity)entity).DerivedNavigation = (OtherEntity)included[0] } : default(OtherEntity) } This also allows to reuse include pipeline to project collection navigations on derived types (before it was producing N+1 queries)
…erived type It was not possible to specify include on a derived type. Include was very strict when computing navigation paths from Include method and generated code wasn't accounting for type discrepancies between caller and navigation. Fix is to allow specifying include on derived types using string overload: entities.Include("DerivedProperty") or using lambda with "as" operator: entities.Include(e => (e as Derived).DerivedProperty) Made tweaks to the logic that computes navigation paths - now it looks for navigation on derived types also. Added additional checks to the generated code (_Include method fixup) old: fixup: (QueryContext queryContext | SomeEntity entity | Object[] included) => { return !(bool ReferenceEquals(included[0], null)) { return entity.Navigation = (OtherEntity)included[0] } : default(OtherEntity) } new: fixup: (QueryContext queryContext | SomeEntity entity | Object[] included) => { return !(bool ReferenceEquals(included[0], null)) && (entity is DerivedEntity) ? { return ((DerivedEntity)entity).DerivedNavigation = (OtherEntity)included[0] } : default(OtherEntity) } This also allows to reuse include pipeline to project collection navigations on derived types (before it was producing N+1 queries)
@smitpatel @anpete for option 2, did you try writing the lambda like this? queryOfBase.Include((Derived e) => e.NavigationOnDerived); |
@divega - It indeed passes compilation and also provides intellisense. I am investigating into chaining (ThenInclude) in generic complex. 🤞 |
NVM I realize this is a 2.1 feature not 2.0 |
For the meanwhile is there any solution for an workaround? I'm also have this problem for an REST API Endpoint. I have currently the problem that in definiton of an ressouce only one API Endpoint shall be created, called URL:PORT/api/testmeans with optional type as query string
But how i can return a whole list of testmeans with specific characteristics? |
@subprime One workaround is to use multiple tracking queries with the same context instance and let the context fixup all the nav properties. |
@subprime : Dunno how your DbContext is set up, but when you add a property which only defines the base class, i.e.
If it's part of a other model, it works same. But if you only want a subset of it, you can only load the subset data first and then query your parent model. EF Core will connect them when loading
It's well covered by the Loading Related Data docs. And yes, this and the other listed workarounds in the docs means additional roundtrips to the database. |
@eL-Prova - I created a feature request for EF6 although I am not holding my breath :(. |
What I did was create a generic function and use the oftype to get the type and then discover the navigations of the type passed into my generic. You could use somethings like that and loop through all of your known types to get all of the correct properties public async Task<IEnumerable<T>> GetAsync<T>(string key)
where T:TestMeans
{
IEnumerable < T> result;
var query = _db.Set<TestMeans>().OfType<T>().Where(r => r.Key== key);
foreach (var nav in _db.Model.FindEntityType(typeof(T)).GetNavigations())
{
query = query.Include(nav.Name);
}
return await query.ToListAsync();
} |
Is there still any work to be done on this? |
ok, looks like because SupplierId is ForeignKey in one to one realtion, it generated the SQL with Inner Join, changing it to int? fixed the problem |
After this is released in 2.1, will I be able to order by derived property? Is there a way to do so now in 2.0? |
@maumar @anpete Can you answer the question above from @iudelsmann? |
@iudelsmann as what I've seen from Pull Requests, this issue/feature is related only to the Inlcude/ThenInclude. I'm pretty sure that OrderBy will require a different logic |
I did some testing and was able to OrderBy derived property by casting (in 2.0).
But when ordering by a derived joined property, it did not return the instances of the parent class (it performed inner join instead of left join)
I'm not sure if I'm doing things the right way or if this is still a limitation, I would only like to open an issue if it really is one. But you're right, this issue is only about Include/ThenInclude operations so I'll move this question to stack overflow soon. Thanks. |
@iudelsmann can you post the model you used to get the INNER JOIN query? (entities and contents of OnModelCreating method on DbContext). I tried similar query on current bits and got query with LEFT JOIN: SELECT [f].[Id], [f].[CapitalName], [f].[Discriminator], [f].[Name], [f].[CommanderName], [f].[Eradicated]
FROM [Factions] AS [f]
LEFT JOIN (
SELECT [f.Commander].*
FROM [LocustLeaders] AS [f.Commander]
WHERE [f.Commander].[Discriminator] = N'LocustCommander'
) AS [t] ON CASE
WHEN [f].[Discriminator] = N'LocustHorde'
THEN [f].[CommanderName] ELSE NULL
END = [t].[Name]
WHERE [f].[Discriminator] = N'LocustHorde'
ORDER BY [t].[ThreatLevel] |
@maumar Ok, I have omitted some stuff but this is basically it. One "detail" I forgot to mention, I'm using Postgresql and Npgsql (everything is 2.0, will update to 2.0.1 soon). Models (BaseModel and Model are abstract classes): public class Service : BaseModel
{
[Required]
public string Name { get; set; }
public ServiceCategory Category { get; set; }
}
public class Course : Service
{
[Required]
public CourseModality Modality { get; set; }
[Required]
public CourseType Type { get; set; }
}
public class CourseType : Model
{
[Required]
public string Name { get; set; }
} There is nothing related to this tables in When doing this query (paged search): _context.Services.Include().Include(service => service.Category).OrderBy(service => ((Course)service).Type.Name).Skip((pageNumber- 1) * pageSize).Take(pageSize).ToListAsync(); I got this query (for the first page) (omitted some stuff): SELECT [...]
FROM "Service" AS "service"
LEFT JOIN "ServiceCategory" AS "service.Category" ON "service"."CategoryId" = "service.Category"."Id"
INNER JOIN "CourseType" AS "service.Type" ON "service"."TypeId" = "service.Type"."Id"
WHERE "service"."Discriminator" IN ('Course', 'Service')
ORDER BY "service.Type"."Name"
LIMIT @__p_1 OFFSET @__p_0 |
After this is released in 2.1, will it be safe (i.e. no exceptions thrown) to call the string based include on a collection with items of mixed derived types, such some have the included property and others don't? To be useful, it seems the answer must be 'yes'. I'm just double checking. |
@HappyNomad Yes! |
@HappyNomad Feel free to create a separate issue for this, or send a PR. 😉 |
@anpete No, that's okay. I was just clarifying. In that same comment, I also suggested the alternative that you did end up implementing.
I assume that's how it works in 2.1. |
@iudelsmann looks like you are hitting #10101 - try performing Include as the last operation:
|
@maumar That did not work, it is still performing a inner join. Observe that I'm not |
@iudelsmann ahh, I understand now. In fact INNER JOIN for this scenario is the current behavior even in the current developer bits. Problem here is that navigation "Service.Type" is required and this is how we determine whether INNER JOIN or LEFT JOIN should be produced. (we don't take into account that the navigation is declared on derived). I have filed #10988 to track this issue. |
@anpete Should this be supported already in the current preview (2.1 preview 1)? |
Hello there folks, I have a simple Hierarchy that uses TPC public class Order : EntityBase
{
public long OrderId { get; set; }
public IList<OrderLineItem> OrderLineItems { get; set; }
}
public abstract class OrderLineItem : EntityBase
{
public long OrderLineItemId { get; set; }
public Order Order { get; set; }
public long OrderId { get; set; }
}
public class OrderLineItemDomesticFlight : OrderLineItem
{
public DateTimeOffset DepartureDateTime { get; set; }
public DateTimeOffset ArrivalDateTime { get; set; }
public string Origin { get; set; }
public string Destination { get; set; }
public string Airline { get; set; }
}
public class OrderLineItemTrain : OrderLineItem
{
public string Origin { get; set; }
public string Destination { get; set; }
} But I don't know how to get all types of OrderLineItems when fetching an Order from the database. I have read this doc https://learn.microsoft.com/en-us/ef/core/querying/related-data/eager#include-on-derived-types but I didn't figure it out. for example, this code does not work: var order = await _db.Orders
.Include(o => o.OrderLineItems as OrderLineItemDomesticFlight)
.Include(o => o.OrderLineItems as OrderLineItemTrain)
.FirstOrDefaultAsync(o => o.OrderId == 1); and this is the exception:
|
I found the answer. var order = await _db.Orders
.Include(o => o.OrderLineItems)
.FirstOrDefaultAsync(o => o.OrderId == orderId);
var train = order.OrderLineItems.OfType<OrderLineItemTrain>().ToList();
var domFlight = order.OrderLineItems.OfType<OrderLineItemDomesticFlight>().ToList(); |
Is there a solution for using
ThenInclude
with polymorphic types?Example model:
Example query:
AFAIK the only way this currently works is by manually querying the derived types after loading the base shape:
The text was updated successfully, but these errors were encountered: