This also happens with me: #18017

It seems it's happening to quite a few people. I know my query sucks, as do you in OP, but it would be a shame to have to revert an upgrade due this.

KevinMallinson on 24 Sep 2019

👍17

@KevinMallinson I'm not surprised, it's a pretty significant change. I was hoping when I first read about it, that there would be some way to opt-out of this new behaviour, but unfortunately, that doesn't seem to be the case.

dragnilar on 24 Sep 2019

👍8

Some observations:

The main source of redundancy in a query like (and the higher potential for improvement if you decide to refactor the queries in your application) this comes from the traversal of collection navigation properties with high cardinality. So we are not talking about having to split a query into ~57 separate queries, but about judiciously partition it where it matters.
As @smitpatel implied on https://github.com/aspnet/EntityFrameworkCore/issues/17455#issuecomment-534629896, the best case scenario for the multiple query approach taken in 2.2 is that some of the intermediary results will come out empty, in which case not all paths need to be traversed and hence we end up not executing all queries.
However, in the worst case scenario for the multiple query approach, there is data on all paths and several of the queries generated will have similar complexity to the one query we generate in 3.0. There is no data redundancy in the results, but every table on each path needs to be joined and predicates applied several times.
Although the approach we take in 3.0 is similar to EF6 and is relatively well understood, there might be still things we can do to improve the performance that we haven't investigated. I think there is a small chance that some profiling of the queries and trying different combinations may help get better results.
I expect that we will keep our new design to generate a single SQL query from each LINQ query because of all the benefits, but to mitigate cases like this we can consider providing API to explicitly generate and execute all the queries. I can see a spectrum of solutions, ranging from a utility method that takes the original query and does this automatically, to some extension method that simplifies creating a query for a related data based on the original query.

divega on 24 Sep 2019

👍5 😕3

@divega Having an API or extension to use the previous behavior would be good for those who don't have the bandwidth to rewrite large queries like this one. I personally would like to rewrite mine since it's inefficient and cumbersome to read, but as it stands I'll have to remain on 2.2.6 for a while longer. I'm sure others are in the same boat.

As far as refactoring it goes, I seldom use explicit loading, so excuse my unfamiliarity with it, but would I be able to use that in tandem with No Tracking to be able to obtain a copy of the entity that can have its keys changed before saving it again?

dragnilar on 24 Sep 2019

👍6

@dragnilar you just made me realize there are two interesting points I didn't think of:

Any workaround that relies on explicit loading or executing multiple queries would require fixup behavior that only happens for tracked queries.
It is possible that some of the negative performance impact of 3.0 comes from the fact that we no longer perform identity resolution in non-tracked queries, which should result in many more objects being created. There are two things you could measure that may give us a hint here:
1. Execute the original LINQ query with tracking enabled
2. Execute the generated SQL directly in SQL Management Studio

divega on 24 Sep 2019

👍4

@divega

Now I remember now that I needed the fixup behaviour, which was why I couldn't use explicit loading in the first place for this particular query.

I tried out what you suggested:

Running the query through EF with tracking enabled has it finish successfully. It takes about ~25 seconds on average.

Running the generated SQL (I obtained it via the profiler) in SSMS takes almost 2 minutes to finish.

The generated SQL is pretty large in this case, but it's not as large as when I have tracking turned off.

dragnilar on 24 Sep 2019

I am very glad of the new behavior. Many times I had written a query expecting it to run in a single call, when it makes multiple roundtrips to the database to retrieve what I know to be very small amounts of data. Now it is in my hands as a developer whether to make one call to SQL for all the joins, or make separate requests to the database to retrieve all the pertinent information. Rewriting a few complex queries seems a small price to pay for the benefits gained in the majority of queries, at least certainly going forwards.

Ideally, we could provide SQL-compilation hints to the query, where it would run those queries separately. Or maybe opt-in behavior for the query as a whole. However, I never found EF 2.2 to produce very efficient queries in this scenario, so I would hope for more optimized behavior.

Shane32 on 25 Sep 2019

❤5 👎4 👍3

@shane32 I agree it's better that it adhire to what is expected but at the time it was the seemingly recommended approach based on the documentation provided and what other resources recommended for the cloning scenario. Im fine with rewriting it, but as I said in the OP, I'd rather explore all options first before committing to a rewrite. I'm not sure what others will say, for me it's a pain, but not a super huge impact on my team since my code base uses pretty simple queries for all other operations.

dragnilar on 25 Sep 2019

👍4

we can consider providing API to explicitly generate and execute all the queries.

Please, provide an API because this behavior change is very problematic. We need to rewrite a lot of our queries in EF Core 3.
I can understand this change but the problem is that I don't see how I can easily go back to the previous behavior. I can enable tracking but I must write a lot of queries to do a ThenInclude(...).ThenInclude(...).ThenInclude(...).ThenInclude(...).

kakone on 26 Sep 2019

👍12

I would like to chime in and say that we are also having this issue, and is very problematic for us. An .AsMultipleSqlStatements() extension method would suffice for those of us who need faster execution.

erikmf12 on 26 Sep 2019

👍22 ❤1

Same here, seeing how the multiple queries returned their own subset of results then were consolidated in the framework that resulted in 100s of results. My query is fairly basic, it's really just: Get me products by brand, their images, their options and those option's values.

Example:
await _context.Products.Include("Images").Include("Options.Values").ToListAsync();

To my surprise, when I opened SQL Inspector when I started getting timeouts, I found out it was returning 150,000+ results when running the command manually.

DoughtCom on 27 Sep 2019

👍8

For anyone wondering how to 'hack' your way around this to get 2.2 speed on 3.0, follow the idea on this comment. Basically, you define an initial query to filter your results to the entity you want, and then create dummy variables with .includes to include the related entities you need.

Important: I have only tested if this returns the correct data, nothing else. If you need to do something more than reading/displaying, make sure you test first.

For example, I went from this:

var equipment = await _context.Equipment.Include(x => x.Group)
                .Include(x => x.SystemInfo).ThenInclude(x => x.SystemUsers)
                .Include(x => x.SystemInfo).ThenInclude(x => x.Frameworks)
                .Include(x => x.SystemInfo).ThenInclude(x => x.VideoCards)
                .Include(x => x.SystemInfo).ThenInclude(x => x.StorageDrives)
                .Include(x => x.SystemInfo).ThenInclude(x => x.Software)
                .Include(x => x.SystemInfo).ThenInclude(x => x.NetworkAdapters)
                .Include(x => x.SystemInfo).ThenInclude(x => x.Printers)
                .Include(x => x.Parts).ThenInclude(x => x.ChildrenParts)
                .Include(x => x.Parts).ThenInclude(x => x.ParentParts)
                .Include(x => x.Parts).ThenInclude(x => x.Vendor)
                .Include(x => x.MaintenanceHours)
                .Include(x => x.Attachments)
                .Include(x => x.Notes)
                .Include(x => x.Status)
                .Include(x => x.Area)
                .Include(x => x.EquipmentType)
                .Include(x => x.Department)
                .Include(x => x.PMaintenance)
                .Include(x => x.Request)
                .FirstOrDefaultAsync(x => x.EquipmentId == id);

to this:

var equip = _context.Equipment.Include(x => x.Group).Where(x => x.EquipmentId == id);
var equipment = await equip.Include(x => x.MaintenanceHours)
                .Include(x => x.Attachments)
                .Include(x => x.Notes)
                .Include(x => x.Status)
                .Include(x => x.Area)
                .Include(x => x.EquipmentType)
                .Include(x => x.Department)
                .Include(x => x.PMaintenance)
                .Include(x => x.Request).FirstOrDefaultAsync();

            var d2 = await equip.Include(x => x.SystemInfo).ThenInclude(x => x.SystemUsers).FirstOrDefaultAsync();
            var d3 = await equip.Include(x => x.SystemInfo).ThenInclude(x => x.Frameworks).FirstOrDefaultAsync();
            var d4 = await equip.Include(x => x.SystemInfo).ThenInclude(x => x.VideoCards).FirstOrDefaultAsync();
            var d5 = await equip.Include(x => x.SystemInfo).ThenInclude(x => x.StorageDrives).FirstOrDefaultAsync();
            var d6 = await equip.Include(x => x.SystemInfo).ThenInclude(x => x.Software).FirstOrDefaultAsync();
            var d7 = await equip.Include(x => x.SystemInfo).ThenInclude(x => x.NetworkAdapters).FirstOrDefaultAsync();
            var d8 = await equip.Include(x => x.SystemInfo).ThenInclude(x => x.Printers).FirstOrDefaultAsync();
            var d9 = await equip.Include(x => x.Parts).ThenInclude(x => x.ChildrenParts).FirstOrDefaultAsync();
            var d10 = await equip.Include(x => x.Parts).ThenInclude(x => x.ParentParts).FirstOrDefaultAsync();
            var d11 = await equip.Include(x => x.Parts).ThenInclude(x => x.Vendor).FirstOrDefaultAsync();

You should only need to do this for the entities that need a .ThenInclude(). It's not the best at all, but it allows me to still use .NET Core 3.0 without having to sacrifice load times.

erikmf12 on 27 Sep 2019

👍1

@erikmf12 not sure if this is a full work around. I think we tried something like that and ran into a problem with the entity not having been fixed up since we need it to be non tracked so we can reset the primary keys before recomitting the entity back to the database as a copy of the original. Currently we're considering just writing a manual clone.

dragnilar on 27 Sep 2019

@dragnilar True, I have not tested anything besides making sure all the data is included. In my case, I just need to display the data on a detail page.

erikmf12 on 27 Sep 2019

Figured I would reference the original change request for this feature on here so it would be tracked back on that request.

https://github.com/aspnet/EntityFrameworkCore/issues/12098

DoughtCom on 27 Sep 2019

@dragnilar Correct, my workaround only works with tracked entities. You should be able to write a snippet to untrack ALL entities from a dbcontext, if that works for you. Something like this: (not sure if this is a EF6 or EF Core snippet)

``` C#
public void DetachAllEntities()
{
var changedEntriesCopy = this.ChangeTracker.Entries()
.Where(e => e.State == EntityState.Added ||
e.State == EntityState.Modified ||
e.State == EntityState.Deleted)
.ToList();

foreach (var entry in changedEntriesCopy)
    entry.State = EntityState.Detached;

}
```
If that works for you, you can retrieve the data with tracking, then detach all tracking, then use your existing code to clone the data.

You could probably also write code to inspect any entity object, and utilize the IModel to recursively iterate through all of its navigation members, creating a list of all related entities, then detach them all. Should be relatively easy to do. With a little extra work, it could reset primary keys and foreign keys, which may be necessary as well.

Shane32 on 27 Sep 2019

😕2

@dragnilar Correct, my workaround only works with tracked entities. You should be able to write a snippet to untrack ALL entities from a dbcontext, if that works for you. Something like this: (not sure if this is a EF6 or EF Core snippet)
public void DetachAllEntities()
{
    var changedEntriesCopy = this.ChangeTracker.Entries()
        .Where(e => e.State == EntityState.Added ||
                    e.State == EntityState.Modified ||
                    e.State == EntityState.Deleted)
        .ToList();

    foreach (var entry in changedEntriesCopy)
        entry.State = EntityState.Detached;
}
If that works for you, you can retrieve the data with tracking, then detach all tracking, then use your existing code to clone the data.

You could probably also write code to inspect any entity object, and utilize the IModel to recursively iterate through all of its navigation members, creating a list of all related entities, then detach them all. Should be relatively easy to do. With a little extra work, it could reset primary keys and foreign keys, which may be necessary as well.

Edit: I was probably being too polite. In actuality, we did try this first when we saw what the problem was, since it was an old trick we all had learned back in our EF6 days. Unfortunately it did not work due to a bug (see other comments I made). When we finally found out about the bug and the recommended work around though, the whole thing worked. Hence, the advice was appreciated though. However your remark about IModel did not mean anything to me. Also, I could not find any significant documentation of it besides some vague statements over at MSDN. 😑 It sounds cool but I didn't want to get wrapped up in chasing something blindly; alas I left it alone.

dragnilar on 27 Sep 2019

Triage decision:

We decided that single query mode is right decision we have made and we will keep the behavior
We will look into ways if we can improve perf of single SQL scenario.
Adding a hook which allows EF to generate split queries is infra costly and it has same bugs which made us change the behavior in first place. So we are not looking to add such a hook.
Due to being expression tree, and extension method which does processing would be much more expensive (perf & implementation cost both) than user rewriting the same query in split themselves. Linq is easy to write in code but hard to generate dynamically.
We will document how to rewrite the query to do split queries similar to how previous version did including stitching in tracking/non-tracking both scenarios.
Depending on complexity of above, we may look into provide some API to user where they can pass multiple queries and we would stitch the result. (Perhaps by new-ing up a context behind the hood to run multi-queries.

smitpatel on 28 Sep 2019

👎13 😕12 👍10

@smitpatel I think the last point would probably provide the best flexibility, but I'll understand if your team decides against it. In the least, the documentation definitely needs to be updated so others can avoid running into this situation.

dragnilar on 28 Sep 2019

👍4

I think this is quickly going to become the most frequent issue/question with EF Core 3.x once people to start to transition and have the WTF moment, and indeed whilst my EAV model is mandated by a legacy system and is a questionable design choice, this new restriction affects even the most simple scenarios - the canonical Blog/Tags/Comments/Upvotes. If we are really restricted to realistically 2 or 3 to .Include 1-to-many relations and/or carry the cognitive burden of computing the avg number of related rows to power of number of .Include so we don't stream too much redundant information then I can't see how EF Core is any better than the micro ORMs for the query side.

I understand the argument that splitting the queries can potentially carry consistency risks that aren't clear when not used with the right transaction modes and so wasn't perfect fit, however it was pragmatic and this strive for the right choice seems like a strive for purity rather than what the consumers of EF Core 3.0 desired.

sitepodmatt on 1 Oct 2019

👍21

@sitepodmatt Indeed, it was a pretty nasty surprise to see things like this with EF Core 3.0. As I implied, I am in a position where I can go either way and see both sides. And also for what it is worth, me and my team did notice certain areas perform better with 3.0 versus 2.6. Unfortunately, with the number of breaking changes (such as this) and the other issues that have cropped up on here over the past couple of days, we have decided to table upgrading to EF 3.0 for the time being.

Also, I think for the sake of backwards compatibility, they should have left in a hook or some type of flag on the context that you could toggle to use multi-query mode vs single query mode. I believe that would have been more pragmatic.

I almost wonder if @roji was being semi-prophetic when he said this on #12098:

Agreed, but we're not talking about a bad query, but rather about what happens with the default, standard EF Core join construct. At the end of the day, if the proposal is that EF Core translate the standard join syntax (Include()) via an SQL construct that results in extremely bad performance for the common scenario, then I think everyone will end up losing.

I doubt "everyone" lost with this design change. But at this stage I think it's too early to say definitively what was the net gain/loss. I am under the impression that most devs are not rushing to upgrade to Core 3.0, so it will take some time.

dragnilar on 1 Oct 2019

👍9

Just realize that there are a lot of devs, like me, that felt that EF Core 1.x / 2.x was a big step backwards from EF6 where in EF Core 2.x there is no way to issue certain queries without them being broken up into multiple queries. I can provide samples of seemingly simple queries on EF Core 2.x that would run a subquery individually for EVERY result in the primary table, totally breaking performance! I've had to write and rewrite queries many times to get EF 2.x to produce the desired SQL. For us, EF 3.0 is a pleasant relief, and while it would be nice if there was a 'multi-query mode' for EF 3.0, I'd rather take the 3.0 behavior over the 2.x behavior any day.

Ideally, I'd like to see a parameter added to Include to indicate multi-query behavior (e.g. .Include(x => x.Images, separateQuery: true)), plus an .AsSeparateQuery() method for splitting custom joins, but I understand that this might be a great deal of work. Hopefully an enhancement will make it into EF Core 5 to help with these scenarios.

Shane32 on 1 Oct 2019

👍9 👎2

@Shane32 I'm in agreement with you there, A LOT of the changes in EF 1.x/2.x were weird at first but me and the rest of the guys I work with got used to it. And again, we definitely saw improvements in a lot of the much simpler queries in the rest of our code base after the testing with EF 3, so I can see where the change back to "all at once" has a lot of benefit.

That being said, it definitely seems like there are others who are impacted by this change and they can't take advantage of the benefits of upgrading. And I imagine that this issue isn't the only factor, since there are a number of other breaking changes in EF 3.0.

Edit: Also, that would be more than ideal if we had API functionality like that added to EF (being able to adjust bits and pieces of the Linq chain as separate or whole query). But yes, as you said, I can also imagine that would be a lot of work due to the complexity of handling prioritization of queries parts, etc.

dragnilar on 1 Oct 2019

If we're talking a lot of joins or anything complex I feel like EF shouldn't be used with LINQ but with something like a SQL View or Stored Procedure, especially when performance is of concern.

With that said, imagine my scenario:

I'm wanting to get back 20 products
Each with 5 Lifestyle Images
Each with 5 options
Each with 20 Option Values

Nothing crazy, nothing complicated, just asking for 3 includes. In EF 2.2 that would return 50 accumulative results. In EF 3.0 that would return 10,000 results. In a real world example using actually data where not every product has 5 images, 5 options and 20 values. It went from approximately 1000 results (the EF 2.2 way) to approximately 3500 results for a random brand I just tested.

Now keep in mind, that's someone using a statement like I posted above.
await _context.Products.Include(x => x.Images).Include("Options.Values").Where(x => x.BrandId = 'xxx').ToListAsync();

I assume most people will be fine using this? And perhaps my use-case is more of an edge-case, however it seems like anything with an exponential join can be bad for performance. I came from using EF 6.0 and never ran into what I'm running into now, so maybe it _is_ an edge-case.

In my situation I'm going to have to rewrite my entire Service/Repository layer. Because we pass in the includes to the repository so that each service request is only getting information relevant to what it needs, rather than wasteful includes. This will all have to be rewritten before going to EF 3.0, unless there's a way to change how it creates the statements.

With all of that said, I'm sure you all had a reason to do this, especially if it's proven to work well in EF 6.0 and if the frameworks are going to merge in the future that's just another reason. I also realize that doing a single async call for the new Async Streams that's new in core 3.0 is probably another good reason to go to a single query.

DoughtCom on 1 Oct 2019

👍2

@doughtcom For simple queries like those, there's a simple workaround shown here https://github.com/aspnet/EntityFrameworkCore/issues/18017#issuecomment-535763068 that should work well for you without rewriting much code. Just write the code like this:
C# var products = _context.Products.Where(x => x.BrandId = brandId); var ret = await products.Include(x => x.Images).ToListAsync(); var dummy1 = await products.Include("Options.Values").ToListAsync(); return ret;
This will split the SQL access into two queries, most importantly without returning duplicates of the images. No other code needs to change in your service layer (for this database call), as EF will stitch the models back together, and ret will have all the results (including both images and options/values).

Shane32 on 1 Oct 2019

👍1

I will be posting a detailed work-around later. For easier work-around in tracking queries, you need to rewrite in following way to get exactly same queries as before. Tracking is necessary part to do fix-up. Doing fix-up manually is not impossible but slightly more involved.

Support we have following entities & navigations in the model

Customer
Customer.Address (reference nav)
Customer.Orders (collection nav)
Order.OrderDetails (collection nav)
Order.OrderDiscount (reference nav)

You would need to rewrite only for collection navigations, reference navigations can be part of same query.

```C#
var baseQuery = db.Customers.Include(c => c.Address).Where(c => c.CustomerName == "John");
var result = baseQuery.ToList(); // Or async method, If doing FirstOrDefault, add Take(1) to base query
baseQuery.Include(c => c.Orders).ThenInclude(o => o.OrderDiscount).SelectMany(c => c.Orders).Load();
baseQuery.SelectMany(c => c.Orders).SelectMany(o => o.OrderDetails).Load();

This will generate 3 queries to server. It would be slightly more optimized SQL then what EF Core 2.2 generated. And StateManager will fix up navigations. It also avoids duplicating any records coming from server to client.

Generated SQL:
```SQL
// Customer Include Address
SELECT [c].[Id], [c].[CustomerName], [a].[Id], [a].[City], [a].[CustomerId]
FROM [Customers] AS [c]
LEFT JOIN [Address] AS [a] ON [c].[Id] = [a].[CustomerId]
WHERE ([c].[CustomerName] = N'John') AND [c].[CustomerName] IS NOT NULL

// Order Include Order discount
SELECT [o].[Id], [o].[CustomerId], [o].[OrderDate], [o0].[Id], [o0].[Discount], [o0].[OrderId]
FROM [Customers] AS [c]
INNER JOIN [Order] AS [o] ON [c].[Id] = [o].[CustomerId]
LEFT JOIN [OrderDiscount] AS [o0] ON [o].[Id] = [o0].[OrderId]
WHERE ([c].[CustomerName] = N'John') AND [c].[CustomerName] IS NOT NULL

// OrderDetails
SELECT [o0].[Id], [o0].[OrderId], [o0].[ProductName]
FROM [Customers] AS [c]
INNER JOIN [Order] AS [o] ON [c].[Id] = [o].[CustomerId]
INNER JOIN [OrderDetail] AS [o0] ON [o].[Id] = [o0].[OrderId]
WHERE ([c].[CustomerName] = N'John') AND [c].[CustomerName] IS NOT NULL

Edit: If you do not have CLR navigation to use in SelectMany then you can use EF.Property to reference the collection navigation.

@Shane32 - Your work-around avoids large result set upto an extent but still doing Customer include with Orders will repeat customers in the records. By using SelectMany & selecting Customer separately you remove all duplication.

I have also linked this post from top.

smitpatel on 1 Oct 2019

👍8

@smitpatel ~~With many-to-many relationships, where EF is creating a hidden mapping table, will the suggested method of using SelectMany still work?~~ _Not supported by EF Core, so doesn't matter_
Also, if using FirstOrDefault / Skip / Take, would not the baseQuery be required to be selected, or else only the first ## records of the SelectMany be returned?

Shane32 on 1 Oct 2019

@Shane32 - Slightly updated my code. The thing is, you would apply Skip/Take to first query and do SelectMany afterwards, so you are not selecting first ## of Orders but rather orders for first ## customers.

smitpatel on 1 Oct 2019

@Shane32 - M2M is not supported but the navigation may not always be public to use in SelectMany. Using EF.Property in select many also works. Edited the post to add that too.

smitpatel on 1 Oct 2019

Hey folks. I must confess I am having a moment of cognitive dissonance with regards to the new Single Query behavior. I understand why we want low roundtrips and how we could argue it improves performance... but the attached zip has a pretty stark example of why EF 3.0 isn't going to be particularly useful for dealing with object graphs.

No Code Changes
Via Nuget, upgraded from EF 2.2 to 3.0
Ran the scenario both ways with SQL Profiler, copy pasted results into SSMS and saved the result sets to compare payload size.

Salient Query
C# var result = db.FanProducts.Where(x => x.FanProductId == key) .Include(x => x.FanModel) .ThenInclude(x => x.Amca208fanType) .Include(x => x.FanSize) .Include(x => x.FanClass) .Include(x => x.FanArrangement).ThenInclude(x => x.FanDriveMethod) .Include(x => x.FanDischarge) .ThenInclude(x => x.FanDischargeDirection) .Include(x => x.FanProductAirPerformances) .Include(x => x.InducedFlowFanProperty) .Include(x => x.FanProductPerformanceCorrections) .ThenInclude(y => y.FanPerformanceCorrection) .ThenInclude(x => x.FanPerformanceCorrectionConstants) .Include(x => x.FanProductPerformanceCorrections) .ThenInclude(x => x.FanProductPerformanceCorrectionVariables) .Include(x => x.FanProductImpellers) .ThenInclude(x => x.Impeller) .ThenInclude(x => x.ImpellerType) .Include(x => x.FanProductImpellers) .ThenInclude(x => x.Impeller) .ThenInclude(x => x.Material) .Include(x => x.FanProductImpellers) .ThenInclude(x => x.Impeller) .ThenInclude(x => x.ImpellerRpmderateSet) .ThenInclude(x => x.ImpellerRpmderateFactors) .Include(x => x.FanProductCertifications) .ThenInclude(x => x.Certification) .ThenInclude(x => x.CertifyingBody) .Include(x => x.FanProductSpeedControlRpms) .Include(x => x.FanProductMotorNameplates) .FirstOrDefault();
EF v | Rountrips | Total Payload Size
---- | ----- | ------
2.2 | 10 | ~11.0KB |
3.0 | 1 | ~1.22GB |

Mileage is going to vary incredibly and exponentially based upon the number of Includes. But look at that payload difference... holy null values resulting in a hot mess batman. (the nulls causing a giant amount of not null data tagging along just so we can say 'yup... that recordset is empty')

I get it. Some people are OCD about roundtrips, and for small apps, it's probably fine... possibly better.

Certainly for our use case, which I would characterize as somewhat unique (read-only, highly mathematical with thousands of discrete and variable graphs), this recent change is a deal breaker. Even with clever stitching. In fact, if we have to tap 'off the beaten path' EF/Linq functionality to defeat the "Single Query" feature, are we truly better off? I'd have to say at that point, the likes of Dapper and hand-rolled serialization and fixup would the more responsible and efficient course of action.

Our API on EF2.2, from a total cold start and unprimed caches can turn the request out in ~2.5 seconds for 38 seed graphs. With EF 3.0, we can't complete _1_ before the SqlConnection timeout kicks in and bombs the lot of it (what's that? 30s? 60s?)

Please give serious consideration to at least adding hooks to disable "Sinqle Query" dispatch and using the old approach. (or something that you can inject between Includes to allow manual tuning of the roundtrips. Think .Include().Go().Include().ThenInclude().Go().Include().ToList() )

-joel

PS> I'm happy to give anyone a personal demo if the zip is in doubt.

2.2v3.0_ScriptsAndResults.zip

jostapotcf on 5 Oct 2019

👍22 😄2 😕1

@smitpatel I am concerned by this suggested workaround in the scenario of cloning a large entity. Can you elaborate on what you mean by performing the fix up manually? I presume you mean taking the entities retrieved from the query and manually setting the relationships via the navigation properties?

Also, for what it is worth: One of us had some free time this week and did try testing a multi-part tracking query and then detaching the entities by manipulating the change tracker, but the one who tested it told me it was giving them strange exceptions on save.

I'd have to get more details from the one who tested it, but they think they were running into the problem on https://github.com/aspnet/EntityFrameworkCore/issues/17997 (which btw, as I noted in that issue, affected some other spot in our codebase. Unfortunately, running into that problem made us decide that we cannot upgrade to any 3.X versions until it is verified as fixed).

(Edit: I spoke to them briefly after posting this, they said they were getting the exceptions saying the key already existed even though the ids had been changed before running SaveChanges).

dragnilar on 6 Oct 2019

I presume you mean taking the entities retrieved from the query and manually setting the relationships via the navigation properties?

The manual stitching is not about letting ChangeTracker do the fixup and then detach the entities. It is about doing that task in the code without involving EF or ChangeTracker.

smitpatel on 7 Oct 2019

I presume you mean taking the entities retrieved from the query and manually setting the relationships via the navigation properties?

The manual stitching is not about letting ChangeTracker do the fixup and then detach the entities. It is about doing that task in the code without involving EF or ChangeTracker.

@smitpatel Yes, that is what I meant. Setting the relationships for the objects manually in code, doing what the ChangeTracker would have done for us. Thanks for clarifying that.

At this point I think we will probably end up doing what we were trying to avoid doing in the first place - writing our own Clone() methods. The more I think about this, I am not sure if this (out of the box support for cloning huge entities) was really something your team intends to support right now, unless I am mistaken?

dragnilar on 7 Oct 2019

Problem is orderby on every field instead like in 2.2.6 by id only changing orderby in ganareted sql fixes issue, but still i have felling that queries are slower in 3.0 than in 2.2.6 even when orederby changed

BartoszWiszniewski on 8 Oct 2019

@BartoszWiszniewski You bring up an interesting point. DevExpress recently did a benchmark that shows that EF Core 3.0 is marginally slower in a lot of cases than 2.2.x:

https://github.com/DevExpress/XPO/tree/master/Benchmarks

It looks like in cases of doing deletes and inserts, the speed improved.

I'm hoping some other benchmarks become available soon to provide comparison/contrast.

Also... I did notice performance improvements when we switched to Net Core 3, which I first thought was due to EF, but even with EF 2.6 in use with the rest of the software using Core 3, the gains appear to have remained.

dragnilar on 8 Oct 2019

This is actually an important point. Data set size aside (1.22gb is what - 0.15 seconds on a modern backend network), the order by set is ridiculous and seems to speak from bad optimization. And that really puts all that data down into tempdb for a very heavy order by. THAT definitely must be fixed. It is broken SQL being WAY over protective on the required sorts, it seems.

NetTecture on 9 Oct 2019

👍1

@NetTecture If it were in a format that was ready to be spooled over the network, sure it could be streamed that fast. But the SQL Server's got to do a lot of work serializing that, and then the client has to de-serialize it, all before EF ever even sees it.

The SQLTimeout would indicate that the TDS stream was simply taking too long (ie, EF never even saw the results)

(This was all running on my local machine with ample specs. the situation would be much worse hosting up on azure when DTU throttles come in to play)

jostapotcf on 9 Oct 2019

👍1

Nope, not the way I read it ;)

I think the real culprit - assuming a modern high end network, not the 1gb end user stuff many companies use - is NOT the size but....

there is mention of a LOT of orderby hat is not really needed. OrderBy means that:

streaming can only start once order has been established and
1.22gb sorting in tempdb is something MOST servers will struggle with.

It would require some investigating (not that hard, hook into sql server and debug the sql statement) but I would bet we see some serious deloays before straming starts including overloading, possibly, the tempdb space.

NetTecture on 9 Oct 2019

I'll grant ORDER BY is likely the biggest culprit in the SQLConnection timeout. Let's say the ORDER BY _is_ instantaneous.

That's has to then get serialized over the wire to the client into memory, unpacked, POCO'ified, and finally fixed up.

I should have recorded the row count difference. I'll wager it is in the order of 10000:1 of useless data to useful.

Even if the SQLCmd and TDS Streaming happened instantly, we'd still end up wrecked by EF's time spend wrapping the results in objects (by virtue of having to contemplate a _lot_ of useless rows)

jostapotcf on 9 Oct 2019

👍1

@jostapotcf When we ran into this problem in our environment, we were using a test database running on a separate server that was not on the same machine. As you probably saw in my reply up above, it took too long for it to run in even SSMS (I think it takes like 2-3 minutes for the whole result to finish). The situation was pretty similar to yours, judging from the sample query you provided.

And like you said, I am concerned about the entirety of this change when it comes to dealing with large object graphs. I was under the impression that switching from single query to multi-query with EF Core initially was to get away from the Cartesian explosion problem... and now we're back to where we were before with classic EF. I find this odd.

dragnilar on 9 Oct 2019

👍6

I have create a sample app which shows a way to rewrite a multiple level collection include in LINQ to do split SQL queries and stitch that up.
You can access the app as gist here https://gist.github.com/smitpatel/d4cb3619e5b33e8d9ea24d3f2a88333a

I have added code comments for better understanding. Feel free to ask if any questions.
The sample covers tracking/non-tracking queries with buffering/streaming both cases.
The sample is written for sync query but same can be done with async queries easily. You can also compose the base query to apply filters/orderings/paging.

smitpatel on 15 Oct 2019

👍12

@smitpatel Thanks for posting that gist, we may try something out like that once we feel like we are in a good spot to try upgrading to 3.x (probably 3.1 at this point).

Edit:
@smitpatel One question that came to my mind (it's kind of outside the scope of this, sorry): back in my EF6 days, I remember another strategy that I saw being used for cloning large object graphs was using lazy loading and writing manual cloning procedures to allow one to maintain control over what parts of the graph that are cloned and which parts do not. Do you foresee any risks with doing that in EF Core 3.x?

dragnilar on 15 Oct 2019

@dragnilar - I don't know answer to that. Can you file a new issue for visibility to others?

smitpatel on 16 Oct 2019

Closing this issue as a simple way to rewrite the query has been posted.
To document it in our docs is being tracked at https://github.com/aspnet/EntityFramework.Docs/issues/1854
To improve overall perf of query is being tracked at https://github.com/aspnet/EntityFrameworkCore/issues/15892
Providing a method which would do fix-up needs to create new context behind the scene anyway as the issue with shadow FK to connect objects. Which users can do themselves right now too.

smitpatel on 16 Oct 2019

👎1 👍1

@smitpatel Sure, I can do that. I appreciate the honesty.

dragnilar on 16 Oct 2019

Providing a method which would do fix-up needs to create new context behind the scene anyway as the issue with shadow FK to connect objects. Which users can do themselves right now too.

Thanks for the example @smipatel. Is there any intention to provide a general method to do fix-ups for AsNoTracking queries? Maybe something along the lines suggested by @jostapotcf above?

franzo on 16 Oct 2019

@franzo - No there is no intention as it defeats the purpose of the design. EF Core doing split queries had bugs and incorrect results in some cases. The sample app I have linked has a way to do reconnection for no-tracking queries too. If you want to have a hook to do split queries then just initialize new DbContext and do tracking query inside of it and throw away the context afterwards.

smitpatel on 16 Oct 2019

EF Core doing split queries had bugs and incorrect results in some cases.

Can you outline some examples out of interest? I ponder this briefly and I think most of them can be mitigated with the correct transaction modes.

sitepodmatt on 17 Oct 2019

👍1

Can you outline some examples out of interest? I ponder this briefly and I think most of them can be mitigated with the correct transaction modes.

12098 captures the discussion over the design which should satisfy anyone's interest.

smitpatel on 17 Oct 2019

👍1

Specifically for the transactionality, see https://github.com/aspnet/EntityFrameworkCore/issues/9014 as well.

roji on 17 Oct 2019

👍1

I'm really disappointed by this very significant change as well. Something that was easy to use and worked really well is now broken and requires weird workarounds.

Suggesting people should use manual enumerators with GetEnumerator() and MoveNext() is very error-prone, not intuitive and really shouldn't be the "recommended" solution.

This issue adds a significant amount of time to our migration to .NET Core 3, as we now have to closely look at every usage of Include(), rewrite many queries and do a lot more testing than we anticipated.

cwe1ss on 22 Nov 2019

👍2

I've been trying to refactor one of our cases and it turned out to be even more frustrating than I'd expected it too be. I understand that there's many scenarios where executing one query makes sense, but there should really be a way to keep the old behavior. Even opt-in would be totally fine IMO. This is a huge step back otherwise in my opinion!

I'm therefore asking to reopen this issue.

Here's the issues I ran into so far:

Loading data for a single entity requires storing the entity and the query

If I want to load just a single object, I can no longer use .FirstOrDefaultAsync() directly as I (AFAIK) don't have a way to load the related data on the resulting entity. I therefore have to keep the query in a separate variable.

// EF Core 2.2 (nice and clean)
var user = await _dbContext.Users
    .Include(x => x.Roles)
    .Include(x => x.Permissions)
    .FirstOrDefaultAsync(x => x.UserId == userId);

// EF Core 3 (way too verbose)
var query = _dbContext.Users.Where(x => x.UserId == userId); 
var user = await query.FirstOrDefaultAsync();
await query.Include(x => x.Roles).SelectMany(x => x.Roles).LoadAsync();
await query.Include(x => x.Permissions).SelectMany(x => x.Permissions).LoadAsync();

A manual check before calling LoadAsync is necessary to prevent empty queries

If I'd use the previous code, any calls to non-existing userIds would result in 2 useless SQL queries for the LoadAsync calls, so to do this properly I have to add a check and end up with the following code:

var query = _dbContext.Users.Where(x => x.UserId == userId); 
var user = await query.FirstOrDefaultAsync();
if (user != null)
{
    await query.Include(x => x.Roles).SelectMany(x => x.Roles).LoadAsync();
    await query.Include(x => x.Permissions).SelectMany(x => x.Permissions).LoadAsync();
}

AsNoTracking is not usable anymore

I can no longer use AsNoTracking() as this would require me to use manual enumerators and populate the navigation properties myself in the loading code. If I had to load 7 child objects I'd have to write about 10 lines of code per relation instead of 7 in total just to get my object hierarchy back and I'm not willing to do that.

Relying on `null` for non-loaded collection properties is no longer possible.

We have never initialized our one-to-many navigation properties in our entities because this way, we knew that the relation had not been loaded if the property was null. This was great to differentiate between "relation has not been loaded" and "there's no related objects" and was a great safeguard against forgetting to call Include(). With the new LoadAsync() the navigation property stays null if there are no related objects.

Generic paging / sorting methods are more complex

We have a generic ToPagedListAsync(...) methd that takes a IQueryable variable, runs it once with Count() to get the total item count, applies paging & sorting, executes the query again and returns the paged data. Since the query that goes in doesn't have the paging/sorting filters, I can't call LoadAsync() on it - this would load too much data. I therefore have to return the final IQueryable as well so that I can apply LoadAsync() on it.

Of course, I also have to check if there even were any results before I do my calls to LoadAsync() (see above).

// EF Core 2

Page<User> page = await _dbContext.Users
    .Include(x => x.Roles)
    .Include(x => x.Permissions)
    .Where(x => x.FirstName == request.FirstName)
    .ToPagedListAsync(request.Paging);

// EF Core 3

Page<User> page = await _dbContext.Users
    .Where(x => x.FirstName == request.FirstName)
    .ToPagedListAsync(request.Paging);

if (page.Items.Any())
{
    await page.FinalQuery.Include(x => x.Roles).SelectMany(x => x.Roles).LoadAsync();
    await page.FinalQuery.Include(x => x.Permissions).SelectMany(x => x.Permissions).LoadAsync();
}

This rewrite might also be necessary later when performance problems arrise

Since the simple .Include() statements "just work" and since development databases often are small with little data, people will most likely use the old code and not realize their mistake until performance problems occur in the production database. Switching to the manual process can easily lead to other bugs/issues if one of the above things is forgotten / done wrong. If there were a simple .RunInSeparateQueries() switch, this would be an easy & safe change.

??? Switching to a document DB like Cosmos requires reverting to the old code

Don't know about this one but I'm just wondering if using Cosmos would require me to switch back to the "old" code again if the child objects are part of the parent document?

cwe1ss on 22 Nov 2019

👍18

I'm with @cwe1ss on this one.

EFc 3.0 has already started the timer for us to switch away from it's use all together. Our concern being, someone is going to see 'oh, this is an old package, i'll just update it' and boom... tanked performance.

It wouldn't be a big deal if it was a minor code change to 'stay within the fold which is ef goodness', but it's not... it's mid-level surgery to stay on the right side of EFc 3.0. As I stated before, at THAT point, the friction that EF was removing for us (moving relational data into a cache), has in fact increased the friction and we might as well fall back on Dapper or ADO.Net (arguably the most responsible choice to begin with... but we're lazy).

@smitpatel included a reference to #12098. I've read the thread twice now, and I'm still not seeing a black and white argument in favor of single query dispatch. I must be missing something because I really don't get why this change is considered an obvious choice.

The one qualifier being, I always assume the ORM is lying to me, so I double check I haven't hit any of the edge cases where it might have messed up on the query generation. Is this why the choice was made to change behaviors?

If that's the case, why not have single-query-dispatch be the default "will never lie" mode, with an option a "i think all ORM's are liars mode" for those of us that are ok with that.

To be clear, I love what you guys are doing, and want to stay within a sweet spot that means I get to use your thoughtfully crafted code and STILL be lazy... but your making the sweet spot awfully small (imo).

jostapotcf on 22 Nov 2019

👍12

I felt weird this one got closed with its resolution, but I also did not feel sure about what to say since my circumstances felt weird (trying to clone an object graph because we didn't want to write manual clone code). I still feel that this issue is a big one since it can cause a significant amount of time having to rewrite queries and test app performance. It isn't as painless as what @smitpatel was saying, if you ask me.

Do not take this the wrong way @smitpatel , I respect you and the rest of the EF team immensely. But at the end of the day, the "work around" suggestion, IMO, doesn't feel like a satisfactory solution to the problem. I understand you guys are short staffed, but this isn't something that I think can be easily dealt with in all cases with the suggested work around.

Thus I agree that this issue be reopened and there needs to be considered some type of API or other way to toggle the default behavior instead of having to be forced to rewrite code.

dragnilar on 22 Nov 2019

❤6

We'll investigate a way to execute a query in split query mode for common scenarios.

AndriySvyryd on 23 Nov 2019

👍21 ❤6

Spent a fair few hours going through all the samples here to improve on a much simpler include series with just 4 tables involves. Not getting much luck!
Is there an easy way to just remove the Order By which is added at the end? That in itself seems to be causing the majority of the pain when run in SSMS

Edit: Managed to get it working - i hadn't split the queries _enough_
Still - not a huge fan of what's required to get round these problems. I was fortunate enough that google led me to this issue. Still several hours dropped trying to resolve. Not great.

RDalziel on 25 Nov 2019

We'll investigate a way to execute a query in split query mode for common scenarios.

Assuming this won't make the 3.1 release since that's now locked down, will this be included in the 5.0 release for November 2020?

mguinness on 25 Nov 2019

@mguinness That's the plan as long as this issue is in the 5.0.0 milestone

AndriySvyryd on 25 Nov 2019

👍3 ❤1

I'd just like to say, while an opt-in approach for single-query mode would be great, please don't change the default behavior. Split queries are immensely useful at times, but having a single LINQ statement run a single SQL statement is not only intuitive, but _required_ for writing performance oriented queries. EF 3 is so much better at translating complex queries, it would be a shame to go backwards.

Shane32 on 25 Nov 2019

👍2

@Shane32 I think the team agrees with that, and we don't currently have any plans to switch the default back to split queries.

roji on 26 Nov 2019

@Shane32 I don't think anyone wants to go "backwards". I believe the problems that myself, @jostapotcf , @cwe1ss and others have run into are cases where we need "backwards compatibility" /more ability to control what EF does, because we wound up with edge cases where we still need the split query functionality and/or the flexibility it allotted.

dragnilar on 26 Nov 2019

👍2

Yes, there's no need to switch back the default. There just should be an easy, integrated way to do split queries because it was an extremely powerful and easy-to-use feature. And in my opinion, this is not an edge case but a very common one, especially with big databases.

I think my favorite way to use this would be an additional parameter on an individual .Include() method - something like .Include(x => x.PreviousLogins, useSeparateQuery: true). This way, one could combine directly joined relations with separately loaded relations.

dbContext.Users
  .Include(x => x.Department) // This should probably not even offer the additional parameter because it's not an ICollection
  .Include(x => x.Roles) // This does a direct join
  .Include(x => x.PreviousLogins, useSeparateQuery: true) // This does a separate SELECT
  .ToList();

This way, switching from one way to the other would be very easy!

PS: I understand that building a LINQ provider is extremely complicated so any work you do here is very much appreciated!

cwe1ss on 26 Nov 2019

👍7

I'd preferred some QueryContext

var users = await dbContext.Users
  .Include(x => x.Department) // This should probably not even offer the additional parameter because it's not an ICollection
  .Include(x => x.Roles) // This does a direct join
  .Include(x => x.PreviousLogin)
  .QueryStrategy(queryContext => {
    queryContext.Collection(x => x.PreviousLogin).UseSeperateQuery(true);
// (below) let's get Filter done for 5.0 too instead of messing about with custom join logic, some hack on alternate keys, or some third party closed source paid extensions for EF
    queryContext.Collection(x => x.Roles).UseFilter(r => !r.IsDisabled); 
    queryContext.Collection(x => x.Roles).Collecton(r=> r.AnotherCollectionOffRoles).UseSeperateQuery(true);
}).ToListAsync();

Could handle @cwe1ss 's proposal of useSeperateQuery as an extra argument on Include as an extension method that sets the querycontext, a bit of sugar per se.

sitepodmatt on 26 Nov 2019

Whatever you do, allow the ability to post split the query.

I.e. i define the query with .Include, .Include, .Include

THEn call. .Subquery (path) to split off a path ino a separat query.

I can see a subsystem coming for my odata API that takes the generated queries and then applies a attribute or file based cost estimator (using the cardinality estimates on navigation properties) to decide to (dynamically) split of parts of the query into subqueries.

NetTecture on 26 Nov 2019

Yes, there's no need to switch back the default. There just should be an easy, integrated way to do split queries because it was an extremely powerful and easy-to-use feature. And in my opinion, this is not an edge case but a very common one, especially with big databases.

Right, "edge case" probably wasn't the best phrase, looking back at what I posted.

Personally I would like to see the ability to "control" when the query is split at the actual query level like in what you specified (it looks the least verbose and most intuitive IMO) and possibly also a flag (I.E. DbContext.Configuration.UseSplitQueries) when you want to do it on every step of the way for certain reasons.

dragnilar on 29 Nov 2019

i think it should be 1 query, not even sure what multiple query's means. does it apply the where multiple times.. anyway, if you are concerned about performance, then i suggest that you measure and if you find inadequate sql is being generated, i suggest that the convert to Linq (from a in cx.Accounts), you can use Linq Pad to ensure the sql is being generate like you would like. I have been using EF6 and Core for years, and if 3 is the same as EF6 then i think this is the way someone would expect it to work. If it now starts making CTE tables or multiple query's internally, then you should be explicit on that. havnt done much comparing generated SQL here with regards to 2 vs 3, but that's why i write the Linq instead of relying on includes to do the joins efficiently.

Seabizkit on 4 Dec 2019

👍1

i think it should be 1 query, not even sure what multiple query's means

The team has stated they won't revert back to split queries so single query mode is staying.

but that's why i write the Linq instead of relying on includes to do the joins efficiently

Then this discussion is not relevant to you, this is about navigation properties.

I think what people are looking for is a way to optionally split queries into more manageable/efficient pieces and then stitch the results together. Easier said then done I'm sure, so I welcome any effort to get this done for .NET 5.0 before 2.1 LTS ends in 2021.

mguinness on 4 Dec 2019

👍3

Actually, it does not matter if Includes is used; EF Core 2.x still will run in "multi query mode" in certain instances. Today I ran into a simple dbContext.Set<>().Select().GroupBy().ToDictionary() where, with no apparent reason, EF 2.x would not execute it server-side in a single query. It's reasons like these that I'm thankful for the hard work that was put into EF 3 to make a wider variety of queries execute exactly as written. That being said, if opt-in "multi-query" support is added, I will certainly take advantage of it on occasion as well.

Shane32 on 4 Dec 2019

mguinness - unsure why you are quoting me.

I was reinforcing the decision that was made.
The decision/decisions are relevant to any one who is interested in EF,
I use both, even linq rely on navigation properties..., use linq when "include" is going to suffer too much.

...navigation properties reinforces relationships, which allows you to create the typed versions which can use translation to sql. I think its extremely difficult for the EF team to be able to create everything considering that each usage/consumption has its quirks. Hence why I'm suggesting, that when your query goes beyond what built-in helpers can do, in terms of getting the most optimal sql. Its often much easier to write the sql, and then write the linq which generates that sql (also reinforces what is possible in terms of the sql that can be written). As any short hand will probability generate sub optimal version. This is a discussion at hand.., I'm suggesting that perhaps the answer is to use linq rather than creating behavior that no one expects, which is the route they have gone. Open to the idea that it could be toggled for those who choose not to write linq version of their query's.

Seabizkit on 10 Dec 2019

So, I've had this discussion with a developer friend of mine who a few weeks ago "tried to migrate" to .Net Core 3 and EF Core 3 who said: "We stopped the migration because of major performance issues, pages taking even 20 seconds to load, while before they were sub 0.5 sec ". Well, apparently the cause is the new single query model. Even funnier, is that one year ago he was heavily arguing against me saying EF core 2.0 splitting of queries is bad for performance and a single query should be generated, while I was arguing the opposite.

Leaving anecdotal evidence aside. The single query will always perform worse once the join level reaches a certain threshold. That's why ever since EF 1.0 thee fix for this performance issue was to split the queries with the most common optimal pattern being 1 query per collection with related entities included.

I wish multiple queries would become an option again as it's better for cases where large graphs with many includes are loaded. So I hope EF team really (re)considers this.

popcatalin81 on 10 Dec 2019

👍9

"We stopped the migration because of major performance issues, pages taking even 20 seconds to load, while before they were sub 0.5 sec ".

Same thing here. Today we started the migration to EF Core 3.1. 6 hours later we are running screaming from it. Some queries that ran instantaneously now take almost a minute to run.

valuator18 on 11 Dec 2019

👍3 👀1

I have rather complex query which now after upgrade spits out invalid SQL because JOIN for GROUP BY is also including IDs which are not part of the GROUP BY.

Honestly I don't think it's even possible to do the full request using just one query. Desperately need a way to selectively enable old behavior.

MaikuMori on 12 Dec 2019

I'm a little concerned since it looks like a lot of us are still on 2.2.x and I noticed today in the VS installer while running an update that Core 2.2 is going to be end of life at the end of the month. 😢

Also, for what it is worth, ZZ Projects has update their library EF Plus today (at lest on NuGet) and reintroduced IncludeOptimized. From testing it with the query that I have in the OP, it actually performs faster with that in 3.1 than with the Include/ThenInclude chains in 2.6.

The only drawback is that extension does not support NoTracking() queries, so it may not be an immediate work around for anyone who is trying to find a way to find a quick solution.

dragnilar on 12 Dec 2019

Split-Query behavior was a big improvement over EF6, at least in our scenarios. The new single-join behavior is a step backwards and in my experience leads to unnecessary slow and complex cartesian result queries. (Especially when the number of includes increases it gets really slow).

As already commented here, the recommended workaround of manually fetching related collections via Set<>().Load() is cumbersome and increases code-size + complexity.

So a big +1 from my side, for getting the old behavior back.
I dont believe this should be enabled by adding additional IncludeSplit (or whatever its name would be) overloads, as this is too much provider specific detail on this level (E.x. when executing in-memory, this does not make sense IMO). Furthermore, this does not work when using implicitly included collections (owned type mapping).

Instead, I think this needs to be enabled globally, or at least at the Entity-level. This is especially important when using Owned-Collection mapping to model aggregate roots (canonical example: Order->OrderPositions), as related collection gets included automatically when mapping collections as owned.

Maybe a relational overload in OnModelCreating could be the solution:

modelBuilder.Entity<Order>().OwnsMany(p => p.Positions)
modelBuilder.Entity<Order>(configure =>
{
       // DependentIncludeBehavior+RelationalIncludeBehavior types could be declared in the relational package
       // This would enable split-query behavior, when calling include on the "Order" entity, or if child-entities get automatically included (e.x. in the owned collection mapping scenario)
      configure.DependentIncludeBehavior(RelationalIncludeBehavior.MultipleQueries);
});

// Loads order + order positions in 2 separate queries because of the configuration above:
Set<Order>().FindAsync(1);

Another option could be to enable it globally (e.x. if this per-entity level configuration is too complex or with too little value).

davidroth on 12 Dec 2019

👍5

I am sorry, you have not thought this through. At all.

Splitting a query is not a global or entity specific set - it is a query specific set and expected cardinality may really play a big role here.

Imagine a query A that has B and C items. I know that B is small (2-3 items) and I do not mind in this case to combine A and B into one query - but C are hundred items and no, I do want them split.

I can i.e. imagine a cardinality file for one of my odata extensions that is filled with expected cardinality and then decindes ti split queries based on expected cardinality, not static. And not all of them.

Order -> OrderItem isa classical "must be split" scenarios. Order -> Deliveries not so much as in many cases an order is handled is 0 (not yet delivered) to 1 deliveries and possibly most orders are only having 1 delivery, with a few having more. No need to split this.

Yes, in memory that makes no sense. So - provider specific - the in memory provider will ignore the split instructions.

And no, you can not always predetermine this - unless you work in anemic edge cases. We expose query functionality via Odata to the client side, and this means we really would prefer to be able to determine this dynamically.

Heck, the NEXT step would be to mark specific extensions to be NOT loaded via SQL but by running a global callback. We often do extend into static navigation properties that we could run into a central cache and preload at application start (currency list, country list). This could be stripped from the dynamic part toally if I would inject entities based on primary key (and my cache would be cross transaction - those do NOT change).

NetTecture on 12 Dec 2019

I am sorry, you have not thought this through. At all.

@NetTecture can you please make an effort to be more respectful with your comments? Regardless of the content of you comment, it's possible to express it politely and without implying that the other party hasn't thought things through. Just trying to maintain a good and open environment for everyone to express themselves.

roji on 12 Dec 2019

❤10

@Shane32 Thanks again for that suggestion about detatching after doing a tracking query. We finally had some time due to things slowing down in the office to revisit the linq query. Myself and another guy thought we were going nuts because what you recommended (using the change tracker to detach) worked fine in EF 6 and Core 2.2.6 but we kept seeing collections getting dropped from the context. After doing some more research, it turned out it was a by-product of https://github.com/aspnet/EntityFrameworkCore/issues/18982 . Using the work around that @ajcvickers recommended was the final piece of the puzzle, so it looks like we're officially now on EF Core 3.1.

All that being said, I still hope to see split query as an option in some shape or form in Dot Net 5.

dragnilar on 16 Dec 2019

Took me 3 days to upgrade from 2.2 to 3.0 (or 3.1). I can see that in 3.0 the execution time takes longer than in 2.2. For example, my query (without any join statement) takes only around 700ms in 2.2 but in 3.0 it takes around 1700ms. Hope it can be improved in next version. Cheers.

tuanbs on 30 Dec 2019

Took me 3 days to upgrade from 2.2 to 3.0 (or 3.1). I can see that in 3.0 the execution time takes longer than in 2.2. For example, my query (without any join statement) takes only around 700ms in 2.2 but in 3.0 it takes around 1700ms. Hope it can be improved in next version. Cheers.

@tuanbs Agreed. As you can see from some benchmarks, EF 3.x is slower in several scenarios:

Benchmarks Showing 3.X
Benchmarks Showing 2.X

There are some cases where it does perform better though.

These were done by DevExpress so keep in mind these benchmarks may be more or less biased towards their own ORM. I haven't been able to find any other benchmarks though for 2 vs 3.

dragnilar on 30 Dec 2019

You can still get the same behavior in EF 3 that you got in EF 2 with a little refactoring.

// Changing this
var blogs = context.Blogs
    .Include(blog => blog.Posts)
    .Include(blog => blog.Owner)
    .ToList();

// to this seems to give the same split query that the above code gave in EF 2
var query = context.Blogs;
query.Include(blog => blog.Posts).Load();
query.Include(blog => blog.Owner).Load();    
var blogs = query.ToList();

This is based off @smitpatel comment (https://github.com/aspnet/EntityFrameworkCore/issues/18022#issuecomment-542397085).

I would like to see some way of changing the default behavior but this has been an acceptable workaround for me.

DrewBrasher on 30 Dec 2019

👍1

@DrewBrasher, this gets me part way there in that it produces the same number of queries, but 3.1 still runs 2x slower than 2.2 for my query, which uses Include().ThenInclude().Include().ThenInclude() to go two levels deep on two of the navigation properties. Any advice on what to do for ThenInclude()?

Also, since we are enumerating the entire result rather than deferring with IQueryable, I experimented with switching to Dapper and found I still could not map the Includes with client-side LINQ anywhere near as fast as 2.2 brought back results. On that front, 2.2 deserves some kudos.

flipdoubt on 1 Jan 2020

You certainly could simply execute a query.Include(Posts).Include(LikedBy).Load and then run query.Include(Posts).Include(DislikedBy).Load - which will prevent a cross join from the LikedBy and DislikedBy tables.

But this type of coding will continually return the query and Post rows back from SQL. For optimal execution, you should/could use SelectMany to only load the related records required, which will execute more similarly to EF 2. Search this thread for SelectMany to find smitpatel’s post which describes the correct approach.

Shane32 on 1 Jan 2020

👍1

>

For optimal execution, you should/could use SelectMany to only load the related records required, which will execute more similarly to EF 2. Search this thread for SelectMany to find smitpatel’s post which describes the correct approach.

Can someone explain what SelectMany does to aid performance? When I suggest this during code review, someone is bound to ask "why".

flipdoubt on 1 Jan 2020

blogsQuery.Include(x => x.Posts).Load will return all blogs inner joined with posts. This means that if there are 5 columns in the blogs table, and 5 in the posts table, it will pull 10 columns back from SQL. If there was 10 blogs and each had 20 posts (200 records), that is 2000 columns of data.

blogsQuery.SelectMany(x => x.Posts).Load will perform the inner join, but will only return the posts. So for the previous example, it's only pulling 1000 columns of data back (5 columns * 10 blogs * 20 posts).

Depending on your dataset, this could be significant -- for example, let's say there was a binary image column on the blogs table containing 20KB images. This might not normally appear to be an issue when you only have 10 blogs. But the original query already returned these images, and using SelectMany for this one query alone would reduce the data returned from SQL by at least 4MB.

SQL can of course execute the queries faster if it is returning less data, and make better use of its indexes and available I/O resources.

Shane32 on 1 Jan 2020

👍6

@Shane32 , thanks.

@smitpatel , when I use the GetEnumerator strategy in your gist, the first call to IEnumerator.MoveNext() yields InvalidOperationException: There is already an open DataReader associated with this Command which must be closed first. Does the enumerator strategy not work when using with an actual database?

Stacktrace below:
Microsoft.Data.SqlClient.SqlInternalConnectionTds.ValidateConnectionForExecute(SqlCommand command) in SqlInternalConnectionTds.cs Microsoft.Data.SqlClient.SqlConnection.ValidateConnectionForExecute(string method, SqlCommand command) in SqlConnection.cs Microsoft.Data.SqlClient.SqlCommand.ValidateCommand(bool isAsync, string method) in SqlCommand.cs Microsoft.Data.SqlClient.SqlCommand.RunExecuteReader(CommandBehavior cmdBehavior, RunBehavior runBehavior, bool returnStream, TaskCompletionSource<object> completion, int timeout, out Task task, out bool usedCache, bool asyncWrite, bool inRetry, string method) in SqlCommand.cs Microsoft.Data.SqlClient.SqlCommand.RunExecuteReader(CommandBehavior cmdBehavior, RunBehavior runBehavior, bool returnStream, string method) in SqlCommand.cs Microsoft.Data.SqlClient.SqlCommand.ExecuteReader(CommandBehavior behavior) in SqlCommand.cs Microsoft.Data.SqlClient.SqlCommand.ExecuteDbDataReader(CommandBehavior behavior) in SqlCommand.cs System.Data.Common.DbCommand.ExecuteReader() Microsoft.EntityFrameworkCore.Storage.RelationalCommand.ExecuteReader(RelationalCommandParameterObject parameterObject) Microsoft.EntityFrameworkCore.Query.Internal.QueryingEnumerable<T>+Enumerator.InitializeReader(DbContext _, bool result) Microsoft.EntityFrameworkCore.SqlServer.Storage.Internal.SqlServerExecutionStrategy.Execute<TState, TResult>(TState state, Func<DbContext, TState, TResult> operation, Func<DbContext, TState, ExecutionResult<TResult>> verifySucceeded) Microsoft.EntityFrameworkCore.Query.Internal.QueryingEnumerable<T>+Enumerator.MoveNext() BillingPackage.Data.EF.EfExtensions._GetIncludes(IBillingDbContext db, int[] classId) in EfExtensions.cs

flipdoubt on 2 Jan 2020

@Shane32 - Nice explanation.

@flipdoubt - If you want to split into multiple queries then you are going to be having each SQL query have an associated data reader open. When you use buffering, the data reader is fully consumed and closed. When you use enumerating, you are keeping data readers open and only reading what you need. You need to enable MARS in SqlServer to have multiple data readers open at same time else you will hit that exception. On side note, previous versions of EF Core did buffering behind the scene when running multiple queries and not using MARS, so if you cannot have MARS enabled then buffering is nearly equivalent of what EF Core 2.x did.

smitpatel on 2 Jan 2020

👍4

@smitpatel, thanks for the tip to use the buffer strategy. Enumerating the result before loading the child properties (as you did on line 64) pulled the performance back to 2.2 speeds for me. That was the subtle detail I overlooked.

flipdoubt on 2 Jan 2020

👍2

I've been using EF since 1.0. EF6 was good and 2.x felt like a step forward. This honestly feels like a step backwards, especially for a new engineer (not most of us in this thread). I like the idea of adding an extension method to split the queries. Unfortunately it looks like 3.1 isn't the version for our needs, we will be 'downgrading' back to 2.1.

stevestokes on 12 Jan 2020

👍11

I would like to second @stevestokes sentiment. I have been an ardent supporter of EF code, and was very excited to port our product from dotnetcore 2.2 to 3.1. Man was I surprised to see EF slow down so much the whole product became complete unusable! I was assuming I would get a perf boost...but now I have to roll back and can't go to 3.1. Those cartesian product joins of entities with multiple child collections are a complete killer - all queries timing out. Many of the objects my business logic fetches include several collections of several nested children, each with nested collections - your typical object graph. There needs to be a way to opt in to the old behavior, maybe even a global default to opt in to it. I am not convinced it's in the general case to do one query - data access patterns vary so much. We need choice.

johnkwaters on 15 Jan 2020

👍8 ❤1

don't blame the tools

there are different grads of craftsmanship, noob, junior, junior-fuzy, all the way to expert**

What i think ppl forget is that EF isn't just magic, which can take also sorts of thinking out of the equation and magically make things work without side effects or meaning.

On the one hand you have lazy dev and on the other you have critical dev, sure its about finding a balance... but if you don't want _EF to write SQL for you_, then structure LINQ which generate the SQL you want.

with great power come great responsibility.

Even with the older implementation (altho may be "faster" in alot of situations) it masks what is being done. This help you identity where query are really poor to begin with, when if the older was "faster"*
its probably not optimal (lazy dev)

It allow you the opportunity to rethink how you are calling your data.

altho would like an exploration of what it was(older) doing like was it using CTE/Temp tables in the DB or multiple db calls and loading into memory.

Seabizkit on 15 Jan 2020

👎13

I AM blaming the tools. I have been using dotnet, EF, dotnetcore and EF core since they each respectively were available, quite a while now, I know the ins and outs of them. I love the frameworks for the most part, and was very excited to see this iteration. I have always been an early adopter and as such willing to update my code to take advantage of new features or follow better patterns. I understand what is happening here and what the workarounds are. The problem is that the tools have changed drastically with no opt out, and the amount of work to rewrite the product it too high of a bar to justify it.

johnkwaters on 15 Jan 2020

👍4

@AndriySvyryd You mentioned last November that the team would investigate a way to execute a query in split query mode, do you have any further updates? Since there won't be a EF Core 3.2 release, would this possible solution have to wait until this November for the 5.0 release? Some clarity from the team would be most welcome as if it doesn't make that release we'd have to wait until 6.0 release in November 2021?

mguinness on 15 Jan 2020

don't blame the tools

there are different grads of craftsmanship, noob, junior, junior-fuzy, all the way to expert**

What i think ppl forget is that EF isn't just magic, which can take also sorts of thinking out of the equation and magically make things work without side effects or meaning.

On the one hand you have lazy dev and on the other you have critical dev, sure its about finding a balance... but if you don't want _EF to write SQL for you_, then structure LINQ which generate the SQL you want.

with great power come great responsibility.

Even with the older implementation (altho may be "faster" in alot of situations) it masks what is being done. This help you identity where query are really poor to begin with, when if the older was "faster"*
its probably not optimal (lazy dev)

It allow you the opportunity to rethink how you are calling your data.

altho would like an exploration of what it was(older) doing like was it using CTE/Temp tables in the DB or multiple db calls and loading into memory.

While I somewhat agree with your point, particularly around "EF isn't Magic", at the same time I have to disagree with most of it. The greatest part about being a developer is creative freedom. Constricting that is never healthy for any engineer or even the framework creators. Becoming a great engineer comes from deep learning. If there are framework options that enable more learning and mistakes I welcome that vs boxing people in. A framework shouldn't restrict engineers from doing what they need to do, it should enable them. So yes it does create the opportunity to have adverse affects, performance issues, ect but without making those mistakes, you do not learn. So with this, your Yoda quote applies as well.

stevestokes on 15 Jan 2020

👍1

A framework shouldn't restrict engineers from doing what they need to do, it should enable them.

Agreed. The issue is that EF 2.x could not perform certain basic queries -- like a left join with another table without using navigation fields -- without significant performance drawbacks, often resulting in client side processing. In this example, sometimes the framework would issue an individual SQL query for each record in the primary table, and sometimes it will retrieve all records in the secondary table, depending on the exact LINQ query. It's split-query mode is incapable of handling this request, and nor will it execute with a single query.

I've spent many days trying to get my EF 2.x queries to result in SQL that could be optimized with indexes. Among other issues, one of the problems is that EF 2.x forces certain queries into the multi-query mode, which eliminates the ability to optimize these queries fully.

These reasons are among those that, I believe, cause some people to steer clear of EF altogether, using linq2db or other frameworks which allow for a direct and optimized translation of LINQ to SQL. This is what the EF 3 team was striving for, and I believe they have met with success, even if it is not as easy to use as its predecessor.

So personally, given the choice, I'd much rather have to write a few extra lines of EF 3 code than to be restricted by the EF 2.x framework.

All that said, I'll be excited to see the parsing engine of EF 3 combined with an opt-in multi query mode. Hopefully this feature makes it into .Net 5.

Shane32 on 15 Jan 2020

The issue is actually deeper. I would say soemthing is seriously wrong with Ef Core in general.
Up to 3.1 release (and even now) EfCore is struggling to generate sql for anything not totally basic queries. Up before 3.0 you had features that where incompatible - global filters work, but do not expect to use global filters in all cases because it generates internal errors.

I have been repeatedly berated in errors (when saying I would move back or stay with Ef non core) how EF Core is better and the future and has more features (by people associated with the team). This is all nice and dandy, but if I get a sql error generated because Ef Core decides to create crap SQL or if I get internal errors because it has problems handling it's own functions, I simply do not care that it is "better" and "the future".

Even today (not tries it out NOW - preparing to make a 3.1 upgrade run next week) the toolset is off - there is no way to wokrk database first and generate proper stored procedure bindings, I have been told - which is fine with me, I use a third party tool.

But something is seriously broken if the "new and better tool" is not suitable for usage in the 4th generation or so and still sold as better than an older tool that acutally works.

If I run into ANY issue with EfCore, I will use dotnetcore 3.1 to jump to a third party implementation of Ef (non core) that supports global query filters in Entity Framework. Performance is secondary, imho - not saying it is not important, but if things like simple group by generates internal errors or bad sql, then I do not care about performance.

And do not get me talk about the bug fixing non-cascade. Right now it seems any "bad sql" issue again has to wait for - not sure. 5.0? 6.0? 10.0?

I would love a dynamic efficient query mechanism - but right now I am happy with "generate proper sql for all basic scenarios that I can throw at it via odata" - WITHOUT pulling all data into memory and THEN filtering, because this is my current workaround in dotnetcore 2.2. Next weke I try 3.1 - and as I said, if i run into any issues, it is classical EF again. I have to ship, not to wait another year for another major version.

NetTecture on 15 Jan 2020

@mguinness We will provide something to help with this issue for 5.0. If it's deemed useful and low risk we'll try to port it back to 3.1.x, but the bar for adding new API in patch releases is very high. Currently we are focusing on new features so we can ship something meaningful in 5.0 preview 1 so no work has been done for this issue yet, but it is high on our list.

AndriySvyryd on 16 Jan 2020

❤4

All this ideological talk is just noise.

The thing is the EF Core 2.X Load model is significantly better than EF Core 3.X for certain scenarios, and we would like it back. That's the core of the issue.

popcatalin81 on 16 Jan 2020

👍6 👎1

Note to anyone who is curious - I edited the OP with an example of a work around we used to get around this issue. I should note that we ended up using EF Plus for it due to the fact that it was the most pragmatic choice. What EF Plus is doing is IMO what EF Core should offer (a method that lets you fire off a split query within a big includes chain). Unfortunately it still requires you to use a tracking query. I imagine this may not still be the best for people who have queries like this that are more pervasive throughout their code base, but in our case this is the only query we have that is this huge. All of our other ones are very simple.

I hope it helps anyone else, although it does require using a third party library. 😕

dragnilar on 19 Jan 2020

👎1 👍1

For some queries it make more sense to repeat the parent query rather than perform multiple round trips to the database. You could query for parent, then query for parent-join-child returning only the children and then combine the two. We have some queries where we do this manually as the parent query is very fast, allowing us to pull back multiple results without the combinatorial explosion of a single query with joins.

Is there any chance of having this sort of thing optional? Picking one behaviour is always going to annoy somebody.

iainb123 on 19 Jan 2020

👍1

For some queries it make more sense to repeat the parent query rather than perform multiple round trips to the database. You could query for parent, then query for parent-join-child returning only the children and then combine the two. We have some queries where we do this manually as the parent query is very fast, allowing us to pull back multiple results without the combinatorial explosion of a single query with joins.

Is there any chance of having this sort of thing optional? Picking one behaviour is always going to annoy somebody.

I cannot speak for MS about what the plan is, hopefully @AndriySvyryd can answer that one, but I think most of the feedback here desires it to be a method of some sort that lets you pick to use split/no split or a context flag that lets you turn the feature on and off. I am going to presume that the 🦄 team will probably have more details to share when they get around to tackling this one.

This is mostly conjecture on my part, but if EF were smart enough to determine where to split and where not to split, then I suppose we probably wouldn't be having this discussion. Alas, I tend to doubt we will ever get to the point where the ORM can generate the most optimal query strategies 100% of the time... at least not in the near future.

dragnilar on 19 Jan 2020

I cannot speak for MS about what the plan is, hopefully @AndriySvyryd can answer that one, but I think most of the feedback here desires it to be a method of some sort that lets you pick to use split/no split or a context flag that lets you turn the feature on and off. I am going to presume that the 🦄 team will probably have more details to share when they get around to tackling this one.

Whether to split or not to split will be a decision that the users will have to make. As we've learned that this and evaluating parts of the query on the client automatically is something not maintainable in the long term. When bugs are fixed or new features are added to query translation these behaviors could flip resulting in unintentional breaking changes, so we've front-loaded all the breaking changes in 3.0 by making all the queries behave in the same way by default.

As a reminder @smitpatel has posted workarounds: https://github.com/dotnet/efcore/issues/18022#issuecomment-537219137
https://github.com/dotnet/efcore/issues/18022#issuecomment-542397085

The additional API that we would add will likely be just some sugar to get the same behavior with less code.

AndriySvyryd on 20 Jan 2020

❤1 👍1

I cannot speak for MS about what the plan is, hopefully @AndriySvyryd can answer that one, but I think most of the feedback here desires it to be a method of some sort that lets you pick to use split/no split or a context flag that lets you turn the feature on and off. I am going to presume that the 🦄 team will probably have more details to share when they get around to tackling this one.

Whether to split or not to split will be a decision that the users will have to make. As we've learned that this and evaluating parts of the query on the client automatically is something not maintainable in the long term. When bugs are fixed or new features are added to query translation these behaviors could flip resulting in unintentional breaking changes, so we've front-loaded all the breaking changes in 3.0 by making all the queries behave in the same way by default.

As a reminder @smitpatel has posted workarounds: #18022 (comment)
#18022 (comment)

The additional API that we would add will likely be just some sugar to get the same behavior with less code.

Interesting; I'll be looking forward to seeing what it looks like in the end. And it's understandable you had to make the choice you made. And like you said, I think it comes down to the user's judgement/situation whether or not to split it up. Glad you guys are looking into it!

dragnilar on 20 Jan 2020

I cannot speak for MS about what the plan is, hopefully @AndriySvyryd can answer that one, but I think most of the feedback here desires it to be a method of some sort that lets you pick to use split/no split or a context flag that lets you turn the feature on and off. I am going to presume that the 🦄 team will probably have more details to share when they get around to tackling this one.

Whether to split or not to split will be a decision that the users will have to make. As we've learned that this and evaluating parts of the query on the client automatically is something not maintainable in the long term. When bugs are fixed or new features are added to query translation these behaviors could flip resulting in unintentional breaking changes, so we've front-loaded all the breaking changes in 3.0 by making all the queries behave in the same way by default.
As a reminder @smitpatel has posted workarounds: #18022 (comment)
#18022 (comment)
The additional API that we would add will likely be just some sugar to get the same behavior with less code.

Interesting; I'll be looking forward to seeing what it looks like in the end. And it's understandable you had to make the choice you made. And like you said, I think it comes down to the user's judgement/situation whether or not to split it up. Glad you guys are looking into it!

It sounds like there are three behaviours:
(a) inner join: simpler query, potentially lots of data coming back
(b) batch of multiple queries with repeated criteria query: single round trip, greater server load, reduced data coming back
(c) multiple queries with multiple round trips: multiple round trips, possibly reduced server load, reduced data coming back

While (c) is relatively easy, I'm not sure you can get the (b) behaviour with the existing model and helper code?

iainb123 on 20 Jan 2020

For batching, see also #10879 which is about an API to batch together multiple LINQ queries.

However, it's true that if we provide some sugar API for executing a single LINQ query via multiple SQL queries, it would be great for that to use batching as well.

roji on 20 Jan 2020

You can not. A proper implementation of c may i.e. utilize the use of session level sql table variables to store primary keys from the first query and then reference this in additional followupqueries. It may also utilize using multiple parallel SQL statements (MARS - multiple active result sets) to get data out in parallel over one connection. There is no way to do both without significant changes to the pipeline - they can IIRC not be expressed by LINQ.

This would actually be a good area for an ORM to shine and get away from the simple and "stupid" (as in: mechanical expression.

The decisions of which subqueries to isolate, though, can not be effectively done by EF - it may require additional data and thus be an application elvel decision. I would love to be able to split a query with methods, though, and then use metadata on the objects to decide when an object hierarcy gets goo expensive ;)

NetTecture on 20 Jan 2020

👍1

You can not. A proper implementation of c may i.e. utilize the use of session level sql table variables to store primary keys from the first query and then reference this in additional followupqueries. It may also utilize using multiple parallel SQL statements (MARS - multiple active result sets) to get data out in parallel over one connection. There is no way to do both without significant changes to the pipeline - they can IIRC not be expressed by LINQ.

@NetTecture we're not currently doing any of that as far as I'm aware, and have no plans to do so. If anything, these would likely be quite SQL Server-specific and wouldn't work on other databases. I'm not an expert in related entity loading, but I don't see a reason why batching wouldn't be possible.

roji on 20 Jan 2020

👍3

Same here, seeing how the multiple queries returned their own subset of results then were consolidated in the framework that resulted in 100s of results. My query is fairly basic, it's really just: Get me products by brand, their images, their options and those option's values.

Example:
await _context.Products.Include("Images").Include("Options.Values").ToListAsync();

To my surprise, when I opened SQL Inspector when I started getting timeouts, I found out it was returning 150,000+ results when running the command manually.

Me too 127,000+ rows for a query with 4 Include and 3 ThenInclude statements.
and 182 rows when i did split the query for 3 pieces.

omer-hatip on 21 Jan 2020

👍5

The approach mentioned by @smitpatel using SelectMany and a Load() call worked nicely for me. However, a significant downside with that seems to be: For any related objects where the list of results is empty, the _navigational collection property will be null_ and uninitialized instead of an empty list/collection. This is also mentioned in the snippet at https://gist.github.com/smitpatel/d4cb3619e5b33e8d9ea24d3f2a88333a .

This null-value potentially breaks all other code that expects a properly initialized collection.

What is the best way to solve this then? Using the mentioned approach without SelectMany will slow down performance again, because the initial set of rows (from the original query) would be returned redundantly then.

martinstein on 4 Feb 2020

@martinstein If you aren't using lazy-loading you can initialize the collections in the entity constructor.

AndriySvyryd on 4 Feb 2020

Good point @AndriySvyryd , I hadn't considered that. However, the nice thing about keeping the collection properties uninitialized is that the code will break in case they are null. So, if you accidentally forget to eager-load a collection then at least there is an error when trying to access that property (instead of silently wrong behavior because the collection looks empty).

martinstein on 4 Feb 2020

@martinstein Yes, but you don't get that for reference navigations, so you still need to test that everything is loaded correctly.

AndriySvyryd on 4 Feb 2020

You're right, we don't get that for reference navigations. And it's not possible there, since a reference property can either hold an instance or null. So we cannot distinguish uninitialized from empty there anyway.

But just because we cannot distinguish those cases for reference properties doesn't mean that we should throw out that nice behavior for collection properties.

For collection navigations, EF Core currently has this behavior:

Value when not eager-loaded: null
Value when eager-loaded with .Include(..) but nothing was found: empty collection
Value when eager-loaded with .Include(..) but something was found: non-empty collection

To me that feels very sensible, intuitive and it makes the usage of collection properties robust. (Forgot to eager-load it? You will know because you'll get an exception because the collection you tried to access is null.)

The technique with SelectMany(...) that is suggested as a workaround (and don't get me wrong, I like the general idea of being able to split the query manually) leads to the collection-property being null in the case where nothing was found. That would break the nice distinction of the three cases above.

You suggested as a workaround to initialize the collection navigation with an empty list in the constructor. That would still break the nice distinction of the three cases above, just in a different way.

Feel free to correct me if any of my assumptions are wrong... I'm not an expert at EF Core. Anyway, I feel it would be great if there was an elegant, performant way to eager-load collection properties without losing the sensible null-behavior that we currently have in EF Core. In my opinion it doesn't have to be an extension method that magically transforms one big query into multiple small ones. Telling the user to split the query manually is okay. But the "officially sanctioned" workaround for splitting the query ideally shouldn't remove the nice unitialized -> null; initialized -> collection behavior.

martinstein on 5 Feb 2020

@smitpatel

Per your workaround in 18022 (comment):

I'm getting A command is already in progress: ... exceptions when running your 'non-tracking' code. Honestly, I expected this based on the code in the example. It appears that you're holding two enumerators (and thus two commands) open on a single DbContext. I expected that EF will throw to avoid concurrent commands.

Would you mind elaborating on why this shouldn't throw an exception?

Cooksauce on 7 Feb 2020

@Cooksauce - I just ran that code example too. SQL Server supports ;MultipleActiveResultSets=true on the connection string, which that Gist had. Therefore, multiple DataReaders works on one connection without throwing an exception. I don't know if SQL Server is the only provider that supports MARS, but I do know MySql does not and that's my DB.

AdamDotNet on 7 Feb 2020

@AdamDotNet - Ahhh, I'm using PostgreSQL. Npgsql doesn't appear to support MARS.

@roji Is this correct that Npgsql does not support MARS? The issue for it is still open (Npgsql #462)

EF Team:
If so, does this mean there is not really a workaround for a non-buffered scenario?
(_IMO, an real workaround would work for more than just one provider_)

Cooksauce on 7 Feb 2020

@Cooksauce correct, Npgsql doesn't support MARS. However, although I haven't looked in detail I don't think there's anything that fundamentally requires MARS here - just make sure to do ToList each time rather than enumerating the query directly in foreach, to avoid having multiple queries actually running at the same time.

roji on 7 Feb 2020

@roji Unfortunately calling ToList isn't the most reasonable solution if you have a large table/navigation collection

Cooksauce on 10 Feb 2020

@Cooksauce another possibility is to work with two database connections via two DbContext instances, this way you stream results on each connection separately. That doesn't work well if you need to track entities coming back in a single context, or if you need transactionality.

At the end of the day MARS is a SQL Server feature that may also cause certain important perf degradation (as multiple roundtrips to the database are done to fetch result, instead of one). At the SQL level you can do something like MARS with cursors; you can have multiple cursors open at the same time for multiple queries, and draw X rows at a time from each cursors. That again is bad for perf because of the additional roundtrips, and also won't be generated by EF Core.

roji on 10 Feb 2020

👍1

@roji That is our current workaround (if you can call it that). Since we don't need transaction honoring, it works fine for us.

So at this point, is it probably safe to assume MARS will not become an Npgsql feature unless Postgres explicitly adds it?

Cooksauce on 10 Feb 2020

Show stopper for us upgrading. looks like 2.2 for a while longer, very unfortunate.

jwisener on 10 Feb 2020

👍4

@Cooksauce the only way Npgsql could "implement" MARS would be:

Use cursors under the hood, adding a lot of roundtrips and hurting perf (users can already do that via SQL if they want, but admittedly not with EF).
Simulate MARS by buffering the entire resultset in memory (something like this may actually get implemented), but that's very similar to you calling ToList.

So at least in the foreseeable future I can't see anything like this added to Npgsql...

roji on 10 Feb 2020

👍1

Show stopper for us upgrading. looks like 2.2 for a while longer, very unfortunate.

2.2 is end of life so you need to drop back to 2.1 to get security updates, see .NET Core release lifecycles.

mguinness on 11 Feb 2020

👍1

The differences in my project are extreme. I ran the 2.2 sql query and it ran in <1 second. I ran the 3.1 sql query and it ran in about an hour and a half. The amount of rows being returned was astronomical as well. 19 compared to 12 million. This is really screwing over the transition to 3.1

Mitch-Healy on 12 Feb 2020

😄5 👎1 👍1

@Mitch-Healy and others, we're aware that queries with many includes will be much slower when executed as-is on 3.1 compared to 2.2. You can achieve the 2.2 performance again by rewriting your queries to split as detailed above, although we understand the frustration and extra work this entails. Although this was an intentional change in 3.0, we definitely don't take it lightly but do believe we did the right thing for the product and many users; there's a long discussion on the reasoning above (though this issue is definitely getting quite long...).

roji on 12 Feb 2020

👍2

but do believe we did the right thing for the product and many users

I'd like to to respectfully disagree, with your assessment if you don't mind. The dreaded Cartesian join was the No. 1 problem why EF 6 was perceived as slow for over a decade. And this behavior is brought back to EF Core.

I think there a relatively small numbers of users who see current behavior as a benefit when doing only small queries where milliseconds are gained, but the vast number of users working on medium to complex applications as a design and performance regression for complex graph loads going from 1 to 2 or more orders of magnitude.

What I'm basing the above statement on? On working for over a decade fixing hundreds and hundreds of EF6 performance issues, while some of which were overly complex Linq queries, most of the issues were Loads with too many included collections (not including DB specific performance issues here)

popcatalin81 on 12 Feb 2020

👍6

@popcatalin81 the main point for me is that although we've changed the default behavior, you can still produce split queries by writing your queries in another way. The opposite was not true - you could not produce a single query with JOINs previously. At the end of the day we did make this as a reaction to issues and bug reports from actual users.

I totally get and understand what you're expressing. I do believe that a lot of the frustration comes from the fact that the behavior changed here from 2.x to 3.x - and that's completely legitimate and understandable. As written above we also do recognize the difficulty here and are looking into sugar for expressing query splitting and making that easier.

roji on 12 Feb 2020

👍8 👎1

Although this was an intentional change in 3.0, we definitely don't take it lightly but do believe we did the right thing for the product and many users

@roji ,

I've been monitoring this situation closely since 3.0 came out and our application's performance fell apart after upgrading.

I want to point out that, at the time of my comment, this is the 5th most commented on open Issue in the EF Core repository. Positions 1-4 are occupied by Issues which were opened in 2014 and 2015. This one was only opened in late 2019. In just a few months this Issue has managed to garner more attention than many complex Issues as old as EF Core itself. Most Commented Open Issues The reason I want to bring this up is that I want to establish that, for many users, this may well be the most pressing issue EF Core faces today.

I do believe that a lot of the frustration comes from the fact that the behavior changed here from 2.x to 3.x - and that's completely legitimate and understandable.

I do acknowledge that you likely wouldn't have backlash if EF Core was like this from the start. However, I am concerned that you may have used that axiom as a pretext for devaluing the feedback you're receiving.

As written above we also do recognize the difficulty here and are looking into sugar for expressing query splitting and making that easier.

Seriously, this is fantastic news. And I do think this is a sufficient to make everyone happy. But my understanding, which could be wrong, is that the EF Core team is stretched fairly thin. Where is it on the priority list? As I hopefully established earlier this is a critical problem with EF Core 3.X and I hope it is prioritized as such.

I also want to give some design feedback. Query splitting is the more performant solution in the overwhelming majority of situations; often by hundreds or thousands of times. So the default should likely be reverted to query splitting. Then syntactic sugar should be added to use the cartesian product approach as needed to address the problems you initially set out to solve.

Here's a breakdown of the benefits of this approach from my perspective as a user.

2.X users will not need to change their queries to account for it.
2.X users will not have massive release-blocking performance drops.
3.X users who split their queries would not need to change their code as it would still work as well.
3.X users who had smaller queries that were performant enough as a cartesian product are more likely to receive a performance gain than they are a loss.
When 3.X users actually do receive a performance loss from going back to query splitting it will be small and with your syntactic sugar, easily fixed.
Defaulting to the certesian product approach creates a massive performance pitfall for new users to discover. Some of whom will not search for a solution and instead just write off EF Core as unusably slow.

I hope I've made a productive contribution to the discussion of this Issue. While I believe your decision was incorrect I also acknowledge that: I am not a member of your team; I do not see the situation from the more informed position of someone who works on the project daily; I was not at the design meetings; I was not at the meetings discussing the feedback on this problem; and I have not even looked at the code in any detail. I am frustrated but have made my absolute best attempt to not let that affect my feedback in any way. Please forgive me if any emotion has leaked into my comments as I attempt to make my business case.

TylerDM on 12 Feb 2020

👍28

@roji I think the frustration comes from having completely rewrite and rework a large number of queries that were working and providing business value.

Given, I knew there would be some items we would need to change in order to use 3.0+, since it's a major version update, but I did not expect that we would need to completely rewrite all our linq statements. As @popcatalin81 pointed out, for a large or complex application this can be quite an undertaking. It's hard to justify to product owners/business owners, that we now need to "refactor" and re-regression test all applications (many microservices).

It's an effort just to go thru and make everything compatible to work 3.0, but what is so disappointing is when you get there you realize that your application no longer works, I feel a better approach could have been provided than the “all or nothing”. Just provide a feature flag (like you did for other behaviours) so we have time to refactor this in phases. And not the response of “just rewrite all your linq/queries”.

I understand you feel your decision was ‘right’, But it’s the delivery and execution that were done poorly.

jwisener on 12 Feb 2020

👍8

@TylerDM I definitely see your comment as a productive contribution, and there's no need to apologize for anything at all.

First, it's true that this issue has attracted a lot of attention, but I'd be a bit more careful inferring from this that the community as a whole is asking for a return to the previous behavior; scrolling up shows people arguing the opposite and wanting single-query to remain the default (as well simply people requesting help rewriting their queries). This isn't an argument against anything you said really, just a small note I wanted to make about the difficulty of gauging "what the community wants".

More to the point, one thing that's somehow gotten lost in this discussion, is that single-query vs. split-query isn't just about performance - it's also about correctness/consistency. Split-query is problematic since the multiple queries can actually see different states of the database as it is being modified concurrently, resulting in inconsistent results; note that this isn't a theoretical problem - we got actual, concrete complaints from users about this happening (https://github.com/dotnet/efcore/issues/9014 is an example). I do feel strongly that our default behavior here should tend towards correctness and consistency, and for any perf trade-off that could compromise correctness to be a user opt-in.

Aside from that, after discussions both internally and with users, I don't really agree there's evidence that's split query is "the more performant solution in the overwhelming majority of situations" - this really is one of those cases where the answer purely depends on the query (how many includes), the actual cardinality of the data in your particular database, and many other factors. I've personally seen various cases where additional round trips can be devastating to performance, especially as latency to the the database increases with the cloud, and while single-query can be streamed, split-query cannot (without MARS), affecting memory usage. However, I do recognize that single-query cartesian explosion may cause a more dramatic degradation than the above, and will obviously cause frustration to anyone porting an application from 2.x to 3.x which uses many Includes per query.

I do hope the above adds a bit more information on why the decision was made to switch to single-query. Please understand we do recognize the problem and intend to provide an easy way to opt into split-query behavior (this is what most people seem to be asking for), but at this point I don't think we're considering changing the default behavior back (doing that in a 3.1 patch release is probably not possible in any case). We're also aware that 3.0 was a release with a lot of user-impacting changes - I'm notably thinking about client evaluation which also caused a lot of upgrade pains. Our intention is to not do such a user-breaking release again.

Finally, keep in mind that all of the above is just the opinion of one engineer on the team...!

roji on 14 Feb 2020

👍5

My 2 cents doesn't matter if the old behavior is default and opt in to the new, or vice versa - the most important thing is that there be a way to do the old style queries. Many people have working applications using 2.2 that are blocked from upgrading to 3.1 because of this change. The reason we are blocked is we don't have the time to rewrite the code and retest it - the effort involved is significant for many apps. So we need a way to keep up with EF, without major rewrites. It's urgent. It there a time frame for the fix?

johnkwaters on 14 Feb 2020

👍12

First, it's true that this issue has attracted a lot of attention

I'd rather say the attention was created by EF 3.0 performance regressions that were kind of a surprise for users.

but I'd be a bit more careful inferring from this that the community as a whole is asking for a return to the previous behavior; scrolling up shows people arguing the opposite and wanting single-query to remain the default (as well simply people requesting help rewriting their queries)

The query rewrite is a bandaid. It's not the ideal scenario. Graph Loading is potentially a performance time bomb now. If your data doubles in DB you mays see a 10x performance drop in Production due to cartesian joins potentially being quadratic, and the performance behavior is not evident with small amounts of data but explodes on usage.

More to the point, one thing that's somehow gotten lost in this discussion is that single-query vs. split-query isn't just about performance - it's also about correctness/consistency. Split-query is problematic since the multiple queries can actually see different states of the database as it is being modified concurrently

The query split that, users affected by performance problem need to do now, can also affect correctness/consistency. In general, almost anything in Db world can be affected by concurrent modifications, unless you're always in a serializable transaction. Load with joins has in no way solved the concurrency issue, it just moved the problem into the user's yard (Which is the place where it always was. The "user" is responsible with proper isolation and consistency). But I understand the reasoning, an error in user's code vs library code is more telling to the user where the fix should be added.

I do feel strongly that our default behavior here should tend towards correctness and consistency, and for any perf trade-off that could compromise correctness to be a user opt-in.

I don't think this should be a tradeoff that the EF team needs to make. This is by definition a tradeoff the library user needs to make. The user is the one controlling isolation levels and therefore correctness in the face of concurrency anyway.,

Aside from that, after discussions both internally and with users, I don't really agree there's evidence that's split query is "the more performant solution in the overwhelming majority of situations"

For web site type applications where small amounts of data are loaded due to display limitations. The performance benefit from reducing roundtrips by using joins is undisputable. This is where milliseconds are shaved from a page due to reduced roundtrips.

For enterprise-type or data-intensive applications where large graphs are loaded for thousands of entities, this is most certainly a strategy that can add from seconds to minutes to load times if the graphs are large.

I'm in both groups, and I don't think the pain in the second case can be reduced by the milliseconds gained in the first case.

while single-query can be streamed, split-query cannot (without MARS), affecting memory usage

The impact on memory in a web application cannot be that great because typically web applications do not load big amounts of data per request. Also, it's fairly rare that applications stream the materialized entities instead of placing them in a List, projecting and manipulating them. If a user is worried about the memory he usually does not use EF's include to load entity graphs anyway.

Please understand we do recognize the problem and intend to provide an easy way to opt into split-query behavior

Yes, I think this is the correct fix for users on both sides of the spectrum.

but at this point, I don't think we're considering changing the default behavior back

As long as the default behavior can also be set at context level, this should pose no issues.

popcatalin81 on 14 Feb 2020

👍9

We (the team) have been discussing this issue a lot recently. We have a tentative plan for both 5.0 and 3.1.x.

In 5.0 we plan to:

Provide a mechanism to load big graphs using multiple includes. We're not entirely sure what the API surface looks like yet, but the idea is to make it easy to opt in to this without writing a lot of code/changing your queries significantly. (It will almost certainly be an opt-in, not the default behavior.)
Generate a warning at runtime if the code includes two or more collection navigations. (Two might not be the correct number here, but it's something like >= 2 and <= 4.) (Ideally we would do this in a code analyzer as well, but analyzers come with their own set of compatibility issues and maintenance overhead.)

For 3.1.x we plan to:

Implement the runtime warning described above in a patch. (We will need approval for this, but I expect we will get it.) (#19933)
Implement the loading mechanism described above (or something similar) as an "extension" that we can deliver now and use with 3.1.x code. "Deliver" here might mean copy/paste an extension method we give you into your code. It might mean we ship it in a NuGet package. (#19934)

ajcvickers on 15 Feb 2020

👍22 ❤9 🎉6

Perhaps this is obvious but I just wanted to say that while a lot of discussion here seems to centre around Include, this also affects Select/projections with navigation properties too, which don't need explicit Includes to trigger joins.

AutoMapper's ProjectTo extension produces a translatable projection and is quite handy for generic queries. A mapping between an entity and DTO can be configured, referring to multiple, perhaps nested, navigation properties, and the code at the point of querying becomes something as simple asSet<TEntity>().ProjectTo<TDto>(config).ToList(). I'm not sure if this use of AutoMapper is considered bad practice, but the behaviour in 2.2 obscured that - as you want in an abstraction I would argue?

I mention this scenario because the advice to simply rewrite queries to manually split the query doesn't work in quite the same way here as it does for a lot of the other examples here. Many other examples are quite long, explicit queries that would need to be converted into a similar length query. In the scenario I describe, it can mean introducing many more lines of code, it's much harder to use in any generic helper methods, might require more fundamental refactoring of the app.

snappyfoo on 2 Mar 2020

👍9

Agreed - most of the slowdowns preventing me from moving forwards belong to this class of problems - relatively large object graphs that are projects to DTOs with ProjectTo and AutoMapper.

johnkwaters on 2 Mar 2020

👍3

FWIW, Django's ORM solved this problem with select_related and prefetch_related functions many years ago.

I really don't see why this is so difficult for the EF team to solve in the same or similar fashion.

mikeroque on 9 Mar 2020

😕2 😄1

@mikeroque as far as I can tell, select_related does a simple join, which is exactly what EF Core does by default at the moment when Include is used. prefetch_related seems to be equivalent to query splitting, which we've shown how to manually perform above, and this issue is about adding a shortcut to make that easier.

roji on 10 Mar 2020

I am having the same issue as @snappyfoo my SQLite integration tests now time out because it cant handle all the LEFT JOINs. I also use generic Set<TEntity>() calls and I use AutoMapper's ProjectTo so refactoring in not an option for me.

VictorioBerra on 11 Mar 2020

👍3

Just 'upgraded' to core 3.1 from 4.7. After working out the ridiculous amount of Include()/ThenInclude() we have an unworkable solution with minutes to compile the expression followed by SQL timeouts. The old solution was clean and fast, please revert.

mtinnes on 17 Mar 2020

👍3

Just stumbled across this today. I was using a query in IdentityServer to get a Client, RedirectUris, PostRedirectUris, and CorsOrigins.

As 4 separate queries, it returns a total of 170 rows. But as a single query, it timed out. Executing directly in SSMS shows upward of 148,000 rows. That's a bit of a difference!

EF Core 3.1 generates the following query (cleaned up for brevity):

SELECT *
FROM [Clients] AS [c]
    LEFT JOIN [ClientPostLogoutRedirectUris] AS [c0] ON [c].[Id] = [c0].[ClientId]
    LEFT JOIN [ClientRedirectUris] AS [c1] ON [c].[Id] = [c1].[ClientId]
    LEFT JOIN [ClientCorsOrigins] AS [c2] ON [c].[Id] = [c2].[ClientId]
WHERE c.Id = 2
ORDER BY [c].[Id], [c0].[Id], [c1].[Id], [c2].[Id]

For today, breaking this back into 4 separate queries will suffice for my needs, but ti was quite a surprise!

DaleyKD on 26 Mar 2020

I also hit this issue recently when trying to migrate from 2.2 to 3.1
Indeed this change requires really alot of time to go trough all business logic and find all queries that need to be updated. (And the proposed solutions doesn't look pretty)

I think that https://github.com/dotnet/efcore/issues/18022#issuecomment-576193879 explains well difference between single and multiple queries impact.
And my idea of solutions is similar to https://github.com/dotnet/efcore/issues/18022#issuecomment-558574926 and can be even smarter.

When we have Include that is 1-1, the best solution is to have the join. The call would ask all the columns related to includes from multiple tables in one call, then split the data on client side to related objects.
When we have Includes that are 1 to many, the best solutions would be to have separate call. If we include those related tables into the same call with joins, it will create columns that will be filled with same data over and over and increase size of the payload. (And probably calculation time on the server to consolidate the response, and client side to manage the split of the data)

This should be the default behavior, and then we should have different Include methods if we want to override it:

.Include() - If the relationship is 1-1, use the JOIN and include it in the main query, if 1-n, make separate call to get the related data.
.IncludeJoin() - Can be used on 1-n relationships to force the builder to include this as additional JOIN into the main query.
.IncludeLoad() - Can be used on 1-1 relationships to force the builder to execute it as separate call to the database.

Because it should be simple for the novice user who doesn't know about those subtile things. They should simply use Include(). Now for those who need advanced uses, different types of Includes can exists.
In v1 the split queries were announced as a feature and I think in most cases it is helping simple users and we're used to it since few years now. I agree it's better in some cases to load includes it single query, but it looks like that in majority of cases it is just generating overcomplicated queries that kills database servers. (That I guess you tried to avoid when designing EF Core after EF6)

Glad that something will be done about it https://github.com/dotnet/efcore/issues/18022#issuecomment-586521749 and hopefully for 3.1.x to save all those who plan on migrating from 2.2

peter-bozovic on 28 Mar 2020

👍9

Did anybody manage to create the extension method for something like AsMultipleSqlStatements() or AsSeparateQuery ? This turned out to be a major issue after upgrading to 3.1 and we would rather not have to rewrite all the queries at least for now.

gsaadeh on 15 Apr 2020

Please correct me if I'm wrong. All suggested workarounds are not working together with AsNoTracking(), so, even if we rewrite our queries, we are not able to achieve the performance of EF Core 2.1.

Here is my personal opinion on it. The tried and tested behavior of EF Core 2.1 should be restored. This emerges from a simple risk analysis. With the logic of EF Core 3.1, it is much more likely to accidentally write a query that is not noticeable, but still significantly slower than the one that is executed in several split database queries.

Rather, I'm in favor of a function like AsSingleQuery() that allows any database query to be executed as a single query. In this case it would also be more consistent (from my point of view), since the AsNoTracking() function also gains more performance, if it really matters and tracking is irrelevant.

Since EF Core 3.0 you get an exception, if the query cannot be translated. However, you don't even get a warning, if the query, which took less than five seconds to execute, now takes about half an hour. That is also not understandable for me.

mamazgut on 16 Apr 2020

👍6

We wanted to upgrade from 2.2 to 3.1 too, because 2.2 is end of life. We are using multiple linq queries in our DAL layer using include statements. We will not be able to explain to our product owner that the migration to 3.1 will take a lot more time because we will have to rewrite all our queries because of such a breaking change.

Please include an extension that allows using the old 2.X behaviour. Otherwise we will have to downgrade from 2.2 to 2.1, which does have Long Term Support.

@ajcvickers mentioned above that something is in the work. The "loading mechanism" you are talking about - will this return the old 2.2 behaviour? And when will this nuget package be available? We planned in our current sprint to upgrade to 3.1. And this change in EF Core is a major blocking issue for us to go on with the migration.

Joehannus on 16 Apr 2020

@dragnilar when do you use the select statements on the includeoptimized statements? We might try this out to see whether it works as a temporary workaround until the new nuget package that @ajcvickers mentioned is available.

        var clone = dbContext.Projects
            .IncludeOptimized(z => z.Financial)
            .IncludeOptimized(z => z.ProjectProtocol)
            .IncludeOptimized(z => z.ReportCategories)
            .IncludeOptimized(z => z.ReportSubCategories)
            .IncludeOptimized(z => z.SubProject)
            .IncludeOptimized(z => z.SubProject.ValidationFlag)
            .IncludeOptimized(z => z.SubProject.ExcelFileJson)
            .IncludeOptimized(z => z.SubProject.EnergyAuditUtilities)
            .IncludeOptimized(z => z.SubProject.EnergyAuditData)
            .IncludeOptimized(z => z.SubProject.EnergyAuditData.EnergyAuditAreas)
            .IncludeOptimized(z => z.SubProject.Address)
            .IncludeOptimized(z => z.SubProject.Utilities)
            .IncludeOptimized(z => z.SubProject.UtilityTypes)
            .IncludeOptimized(z => z.SubProject.Units)
            .IncludeOptimized(z => z.SubProject.Units.Select(y => y.Building))
            .IncludeOptimized(z => z.SubProject.BuildingTypes)
            .IncludeOptimized(z => z.SubProject.Buildings)
            .IncludeOptimized(z => z.SubProject.Buildings.Select(y => y.BuildingType))
            .IncludeOptimized(z => z.SubProject.Buildings.Select(y => y.Site))
            .IncludeOptimized(z => z.SubProject.Sites)
            .IncludeOptimized(z => z.SubProject.Sites.Select(y => y.Address))
            .IncludeOptimized(z => z.SubProject.Participants)
            .IncludeOptimized(z => z.SubProject.Participants.Select(y => y.Address))
            .IncludeOptimized(z => z.SubProject.InspectedUnits)
            .IncludeOptimized(z => z.SubProject.InspectedUnits.Select(y => y.Building))
            .IncludeOptimized(z => z.SubProject.InspectedUnits.Select(y => y.UnitType))
            .IncludeOptimized(z => z.SubProject.UnitTypes)
            .IncludeOptimized(z => z.SubProject.CommonAreas)
            .IncludeOptimized(z => z.SubProject.CommonAreas.Select(y => y.Building))
            .IncludeOptimized(z => z.SubProject.CommonAreas.Select(y => y.Building).Select(z => z.Units))
            .IncludeOptimized(z => z.SubProject.FixtureAreas)
            .IncludeOptimized(z => z.SubProject.FixtureAreas.Select(y => y.Fixtures))
            .IncludeOptimized(x => x.SubProject.LineItems)
            .IncludeOptimized(x => x.SubProject.LineItems.Select(y => y.EnergyOneItem))
            .IncludeOptimized(x => x.SubProject.LineItems.Select(y => y.EnergyTwoTwoItem))
            .IncludeOptimized(x => x.SubProject.LineItems.Select(y => y.TraditionalItem))
            .FirstOrDefault(x => x.Id == project.Id);

        if (clone != null)
        {
            foreach (var entityEntry in dbContext.ChangeTracker.Entries())
            {
                if (entityEntry.Entity != null)
                {
                    entityEntry.State = EntityState.Detached;
                }
            }
            return clone;
        }

```

Joehannus on 16 Apr 2020

Like everyone else, we've had the same problem with query performance when trying to upgrade from EFCore 2.2 to 3.X. Going through all the code and re-writing/splitting all the queries and re-testing them all to ensure they come back with the correct data is not really an option. So upgrading is effectively blocked.

I'm pretty shocked the decision was made to completely change the underlying behavior. Sure it fixes a problem that has been there since day one, that affected certain applications that have many writers/readers of the same data rows, but breaks all of the most common use cases! The use-case you were trying to fix is easier fixed through use of a view or SP; or as suggested, by adding an extension method .AsSingleQuery() rather than changing the behavior.

I'm incredibly disappointed by this, as many of the other features in EFCore are well thought out and designed.

A work-around of some kind has been talked about for a rather long time since this issue was introduced, but, to the best of my knowledge, nothing is available, 7 months later. I do not understand the great reluctance to revert this behavior that breaks so many applications, and go about implementing the single query/Cartesian product methodology a different way.

macdonald-keith-vmware on 16 Apr 2020

👍10

Did anybody use this form of .IncludeOptimized(z => z.SubProject.Units.Select(y => y.Building)) as a workaround? Any confirmation that this fixes the performance issue?

We've spent much time and effort doing the upgrade to 3.1 and it is so unfortunate that we are stuck with the last obstacle of performance. This is a complete show stopper and we might just revert back to 2.2 until this issue is resolved. I feel that many of us are loosing faith in the tooling in light of all these ground breaking changes that keep occurring and considering other alternatives.

gsaadeh on 19 Apr 2020

@dragnilar when do you use the select statements on the includeoptimized statements? We might try this out to see whether it works as a temporary workaround until the new nuget package that @ajcvickers mentioned is available.

        var clone = dbContext.Projects
            .IncludeOptimized(z => z.Financial)
            .IncludeOptimized(z => z.ProjectProtocol)
            .IncludeOptimized(z => z.ReportCategories)
            .IncludeOptimized(z => z.ReportSubCategories)
            .IncludeOptimized(z => z.SubProject)
            .IncludeOptimized(z => z.SubProject.ValidationFlag)
            .IncludeOptimized(z => z.SubProject.ExcelFileJson)
            .IncludeOptimized(z => z.SubProject.EnergyAuditUtilities)
            .IncludeOptimized(z => z.SubProject.EnergyAuditData)
            .IncludeOptimized(z => z.SubProject.EnergyAuditData.EnergyAuditAreas)
            .IncludeOptimized(z => z.SubProject.Address)
            .IncludeOptimized(z => z.SubProject.Utilities)
            .IncludeOptimized(z => z.SubProject.UtilityTypes)
            .IncludeOptimized(z => z.SubProject.Units)
            .IncludeOptimized(z => z.SubProject.Units.Select(y => y.Building))
            .IncludeOptimized(z => z.SubProject.BuildingTypes)
            .IncludeOptimized(z => z.SubProject.Buildings)
            .IncludeOptimized(z => z.SubProject.Buildings.Select(y => y.BuildingType))
            .IncludeOptimized(z => z.SubProject.Buildings.Select(y => y.Site))
            .IncludeOptimized(z => z.SubProject.Sites)
            .IncludeOptimized(z => z.SubProject.Sites.Select(y => y.Address))
            .IncludeOptimized(z => z.SubProject.Participants)
            .IncludeOptimized(z => z.SubProject.Participants.Select(y => y.Address))
            .IncludeOptimized(z => z.SubProject.InspectedUnits)
            .IncludeOptimized(z => z.SubProject.InspectedUnits.Select(y => y.Building))
            .IncludeOptimized(z => z.SubProject.InspectedUnits.Select(y => y.UnitType))
            .IncludeOptimized(z => z.SubProject.UnitTypes)
            .IncludeOptimized(z => z.SubProject.CommonAreas)
            .IncludeOptimized(z => z.SubProject.CommonAreas.Select(y => y.Building))
            .IncludeOptimized(z => z.SubProject.CommonAreas.Select(y => y.Building).Select(z => z.Units))
            .IncludeOptimized(z => z.SubProject.FixtureAreas)
            .IncludeOptimized(z => z.SubProject.FixtureAreas.Select(y => y.Fixtures))
            .IncludeOptimized(x => x.SubProject.LineItems)
            .IncludeOptimized(x => x.SubProject.LineItems.Select(y => y.EnergyOneItem))
            .IncludeOptimized(x => x.SubProject.LineItems.Select(y => y.EnergyTwoTwoItem))
            .IncludeOptimized(x => x.SubProject.LineItems.Select(y => y.TraditionalItem))
            .FirstOrDefault(x => x.Id == project.Id);

        if (clone != null)
        {
            foreach (var entityEntry in dbContext.ChangeTracker.Entries())
            {
                if (entityEntry.Entity != null)
                {
                    entityEntry.State = EntityState.Detached;
                }
            }
            return clone;
        }

I'm not sure I understand your question completely. What do you mean by select statements?

dragnilar on 19 Apr 2020

@dragnilar I think @Joehannus means this .IncludeOptimized(z => z.SubProject.Units.Select(y => y.Building))

Notice the Select statement after Units that is supposed to load the Building related data.

gsaadeh on 19 Apr 2020

👍1

@gsaadeh Thanks for pointing that out, probably shouldn't be trying to answer these questions at 4:00 in the morning...

@Joehannus The select is used to filter the related entities that are needed for the whole "Project" clone. It gets executed along with the rest of the large batch of multiple statements that get sent when you use IncludeOptimized. I'd paste the actual SQL that's emitted here, but its 99,305 lines. Needless to say it's actually very efficient... and surprisingly faster than what we could achieve even with pre 3.0's multiple query behavior. I'm not advocating you should use the EF Plus library (even though it is free) but for our case it was the most pragmatic choice we could make.

I hope that answers your question, but if you need more details let me know.

dragnilar on 19 Apr 2020

Thank you very much @dragnilar this is really helpful and I appreciate you answering so early in the morning

gsaadeh on 19 Apr 2020

For owned entity types (.OwnsMany()) the loading of the related entities is implied. You don't need to call .Include() for them to be included in the initial query.

What if we want to control the loading behavior of owned entity types? Is there a way to disable the initial load so they can be loaded separately like this: query.SelectMany(x => x.Addresses).Load()?

I think not, because of this exception message when trying to load the owned entity types:

A tracking query projects owned entity without corresponding owner in result. Owned entities cannot be tracked without their owner. Either include the owner entity in the result or make query non-tracking using AsNoTracking().

If it is not possible to delay the loading of owned entity types, we better not use them if there is a change of multiple joins slowing down our queries. Is this correct?

leksts on 19 Apr 2020

@dragnilar Thanks for that reply. I did change all our include en theninclude statements by using the EF Plus library last friday. It works great. I had one small issue though - in one of the theninclude statements the resulting object also had some one to one relationship objects. For some reason these were loaded without a theninclude in EF 2.1 and 2.2. With EF Plus I had to do an extra Select. Once we've upgraded all our projects, we had planned on thorough regression testing, and now it's even more necessary.

Joehannus on 20 Apr 2020

Did anybody use this form of .IncludeOptimized(z => z.SubProject.Units.Select(y => y.Building)) as a workaround? Any confirmation that this fixes the performance issue?

We've spent much time and effort doing the upgrade to 3.1 and it is so unfortunate that we are stuck with the last obstacle of performance. This is a complete show stopper and we might just revert back to 2.2 until this issue is resolved. I feel that many of us are loosing faith in the tooling in light of all these ground breaking changes that keep occurring and considering other alternatives.

I have to agree with @gsaadeh (and others in this post). It's quite a nuisance that they made all threse breaking changes. One other we came upon was the removal of DbQuery. Which we use for database views. There is a workaround using a new attribute on DbSet entities. But only using the model builder syntax. And as it so happens we used data annotations everywhere. I'm not keen on rewriting all the data annotations to model builder variants. And I don't like using a mixed variant either. Microsoft will introduce a new attribute for this. But only in 5.0.0-preview1. The latter I found out this morning. I would have assumed that they'd introduce this attribute in 3.1 as well. Unfortunately they will not.

Joehannus on 20 Apr 2020

Problem with using IncludeOptimized as a workaround is that if you use another Include after using IncludeOptimized it does not work. I was gonna use it as extension methods to segregate reponsabilities, but with this behaviour I can't abstract the order of query fragments.

iuridosanjos on 22 Apr 2020

Why does .NET Core so tightly couple itself to EFCore like this? Most upgrade paths involve "how did EF break everything this time" in my experience, and it leads to scenarios like this. Where you literally can't upgrade to the latest version of a framework, because it obligates you to upgrade your database-layer package and there's no real way around it.

Let's sever this awful dependency! I understand you can't always do that, but I find it hard to believe that EFCore has to be upgraded every time .NET Core is (does compiling against .NET Core 3 assemblies really necessitate such breaking changes under the hood that consumers can't change, opt out of, or at least get notified of prior to getting stuck with it?)

Like others in this thread, we have show-stoppers on long-invested upgrade paths not because of anything in .NET Core, but because of this tightly-clung dependency that cannot be avoided. Bare minimum - give people a choice. Maintain old behavior. Upgrade older versions of EFCore to be workable with newer .NET Core. Don't force upgrades that break things so hard that people can't upgrade .NET Core. It's one thing to upgrade so people use your tooling "better." It's another to break existing code for no reason.

sdepouw on 24 Apr 2020

👀1 👍1

EFCore has to be upgraded every time .NET Core is

That is not the case. Old versions of EF Core can be used with all newer versions of .NET Core, since .NET Core is backwards-compatible. For example, you can use .NET Core 3.1 with EF Core 2.1.

In general, you are not forced to upgrade to newer versions of EF Core, but have the option of doing so. If you do upgrade to a new major version, then you typically have to deal with some breaking changes - the number and impact of these in 3.0 was indeed quite large, and future major versions are supposed to be much less extreme.

roji on 24 Apr 2020

👍5

That is not the case. Old versions of EF Core can be used with all newer versions of .NET Core, since .NET Core is backwards-compatible. For example, you can use .NET Core 3.1 with EF Core 2.1.

I'm sorry, but I do not entirely agree with your statement. Although it is technically possible, the usefulness of such a solution remains very questionable, because in this case the problem is only postponed and not completely solved.
As support for the current LTS version 2.1 ends on August 21, 2021, developers are faced with even greater challenges when migrating to the current .NET Core version (some breaking changes in .NET 5.0 are also to be expected).

Please don't get me wrong. You are doing a fantastic job and I am happy to be able to use the results of your work. But I think your decision to put everything in one query was not correct. The advantage of EF Core was that it always worked out-of-the-box and delivered the best possible performance. If you need clinically clean SQL statements, you can write them by hand. The strength of EF had always been productivity.

mamazgut on 27 Apr 2020

👍2

@mamazgut I wasn't saying anything about single-query, or in fact about the direction EF Core should take. I was merely responding to @sdepouw's statement that newer versions of .NET Core require newer versions of EF Core. While newer versions of EF Core may depend on newer versions of .NET Core, the opposite is not the case, and there's nothing stopping you from using EF Core 2.1 on .NET Core 3.1, for example. It is also high unlikely that .NET 5.0 will introduce breaking changes to a degree that would prevent EF Core 2.1 from running on it.

roji on 27 Apr 2020

👍2

This is a very disappointing change and discourage the use the EF Core 3.1. We have a large project and how MS can expect us to change entire code to in single shot which will involve regression as well. Ideally these should have been a additional feature and let the app teams decide if they want to leverage it. This is big hurdle to adoption of EF Core 3.1.
What is more disappointing is the fact that EF team is not talking about rollback, no sign of consideration of this pain.

pravinchandankhede on 5 May 2020

👍1

Having lived in OSS and other ecosystems for a while, I'm used to major versions introducing breaking changes.

I was also disappointed in migrating from .NET Core 2.2 to 3.1 that I didn't realize how EFCore 3.1 broke this. However, the workaround of using EFCore 2.2 with .NET Core 3.1 is working for me. I've been updating queries in a branch (changing huge Includes to multiple statements) and it's not so bad. It was supposedly what was happening before anyway!

I understand this is painful and you may disagree with the decision but asking for rollbacks and refusing to accept any breaking changes is what kept .NET Framework down. There is a path forward, use it but also be mindful that all code needs maintainance and regular revisiting.

adamhathcock on 5 May 2020

👍3

I understand this is painful and you may disagree with the decision but asking for rollbacks and refusing to accept any breaking changes is what kept .NET Framework down.

This is a straw man argument.

No one was saying the change should be reverted because we don't want changes or breaking changes for that matter. Breaking changes for the right cause, are good.

This change, in particular, was bad because it affects a large class of consumers in a dramatic way, with no clear benefits in other places.

popcatalin81 on 5 May 2020

👍3

No one was saying the change should be reverted because we don't want changes or breaking changes for that matter. Breaking changes for the right cause, are good.

The post above mine literally asked for that.

This change, in particular, was bad because it affects a large class of consumers in a dramatic way, with no clear benefits in other places.

You're against it because it affects you in a way you consider bad. I read why the change was done and it accelerates other changes. It make sense to me. Having the ORM not be too magical (hiding extra queries) and simplifying the EFCore code base are wins for everyone.

I was just offering an alternative view that showed it also affected me in a non-trival way but I still agree with what happened and that the workaround does work.

adamhathcock on 5 May 2020

The scope of this decision was probably not clear to everyone. During a development, it is simply a matter of taking a look behind and analyzing the work done. This helps a lot not to lose sight of the right way.

Having the ORM not be too magical (hiding extra queries) and simplifying the EFCore code base are wins for everyone.

Many-to-many navigation properties (a.k.a "skip navigations") Here comes the magic...

mamazgut on 5 May 2020

Many-to-many navigation properties (a.k.a "skip navigations") Here comes the magic...

😬

adamhathcock on 5 May 2020

The scope of this decision was probably not clear to everyone. During a development, it is simply a matter of taking a look behind and analyzing the work done. This helps a lot not to lose sight of the right way.

This reminds me... I am not sure I said this earlier on, but to me it seems like a breaking change of this level should have been vetted with a larger feedback ground beyond the halls of Github. I am not sure if @roji or @ajcvickers can comment, but to me in the future it would be cool to get MS to issue feedback surveys for EF like they do with so many other parts of .NET.

I think that way we would at least have felt less surprised/confused if we were made aware that this was being considered beforehand than having to discover it during post-upgrade testing and then have to come here to Github or infer the change from the documentation. I know there's no perfect way to make everyone aware and allow them to voice their feedback, but I think this is definitely something that could be improved.

dragnilar on 6 May 2020

Here's my anecdotal evidence situation:

I have a complex model with multiple level in inheritance and 16 include() calls. Some list to includes only exists on some derived classes. I made a benchmark software loading my whole database (dev, so less data that production) 30 times in a loop. Using EFCore 2.2, it took in average 19 seconds, which was 2.5x faster than EF6. Using EFCore 3.1 and the same code, I'm back to the same performance as with EF6, which is around 50seconds.

I tried to use the IncludeOptimized library, but it don't support inheritance (see link to issue just above).

I then tried to follow the MultiLevelIncludeQueries.cs sample posted by smitpatel to separate my include() calls into multiple subqueries. Unfortunately, no matter how I try to split it or separate it, it only get worse. I'm now in the average of 4minutes 30seconds for loading the same data.

I tried to open SQL Profiler and look at the queries sent by EFC2.2 and re-create them manually with the manual include splitting technique, but I can't seem to manage to do it. In fact, every time I add a new include() to the query and call Load(), it seems to do another SELECT again of all base properties that were previously loaded in the first ToList() call. I'm also not perfectly clear of his usage of SelectMany() in the sample instead of just Include().

Dunge on 6 May 2020

I found the issue. For anyone reading this message in the future who is also trying to do query splitting with collections on derived entities using inheritance:

EFC2.2 was intelligent enough not to do the query if there was no element of that type in the collection. EFC3.1 mega query always join them, and even if you try to load them manually afterward it will do it even if there's no need. What you need to do is check for Any() before launching relation loading call with Load(). Example:

var query = context.Container
var result = query.ToList();
query.Include("Labels").SelectMany(t => t.Labels).Load();
if (result.OfType<Message>().Any())
    query.Include("Graphics").OfType<Message>().SelectMany(t => t.Graphics).Load();

Dunge on 7 May 2020

👍1

var query = context.Container
var result = query.ToList();
query.Include("Labels").SelectMany(t => t.Labels).Load();
if (result.OfType\ query.Include("Graphics").OfType\

First of all, I have nothing against your suggestion. But now imagine, you must implement this fix in 100+ places for multiple collections in your code. What does your code look like in the end?

It was so simple...
var result = context.Container
.Include(c => c.Labels)
.Include(c => (c as Message).Graphics)
.AsNoTracking()
.ToList();

It could be solved so elegant with an opt-in behavior to the single query logic (not for this specific case, but in general. For this case the old behavior is the right choice)...
var result = context.Container
.Include(c => c.Labels)
.Include(c => (c as Message).Graphics)
.AsSingleQuery()
.AsNoTracking()
.ToList();

But now we just have, what we have...

mamazgut on 8 May 2020

@mamazgut
I don't think the problem with the current approach is more code (which can be modified in a matter of minutes, if you know exactly what to do), the real problem with the current approach is that every query with multiple Inclues on collections is a potential performance time bomb, which can blow up in production as the system gathers more data. System performance degradation is the real danger if you're not extra careful with your queries, and this is not always obvious because up to a certain point joins perform really really well (better than multiple queries), but then you suddenly hit a wall once the dreaded cartesian join of multiple collections includes hits.

popcatalin81 on 8 May 2020

@popcatalin81
Agree with your statement, regarting to potential performance degradation (I already mentioned that in one of my previous posts).

Regarding to statement to more code - it's just a matter of taste. I personally prefer less code because, among other things, it is less error proned.

mamazgut on 8 May 2020

@mamazgut
I don't think the problem with the current approach is more code (which can be modified in a matter of minutes, if you know exactly what to do), the real problem with the current approach is that every query with multiple Inclues on collections is a potential performance time bomb, which can blow up in production as the system gathers more data. System performance degradation is the real danger if you're not extra careful with your queries, and this is not always obvious because up to a certain point joins perform really really well (better than multiple queries), but then you suddenly hit a wall once the dreaded cartesian join of multiple collections includes hits.

Given the stated benefits to joins in some/many cases, doesn’t the responsibility lie with the developer to get their query right? If you hit the Cartesian explosion problem in a stored procedure you might have to rewrite to do multiple selects. Same applies to EF LINQ queries. I don’t know if there is a way to solve the leaky abstraction. Therefore, if it falls on the developer to make the right call, what needs to be provided is a way to easily switch between the two approaches in the same query, which otherwise is written the same.

snappyfoo on 8 May 2020

Given the stated benefits to joins in some/many cases, doesn’t the responsibility lie with the developer to get their query right?

Yes and no. The best rule of the thumb for generally best performance across the board is to load related entities with a join (N:1 and 1:1) and collections separately (1:N). This should be de default.
Any performance gains from loading collections with a join are specific and the user needs to opt-in because he knows that:

There won't be a cartesian join explosion in time
The dataset will "remain" sufficiently small so that performance won't degrade.

popcatalin81 on 8 May 2020

👍4

@popcatalin81 and others, one of the point that constantly gets missed/ignored in this discussion, is that multiple queries suffer from a severe consistency issue. The two queries can see a different state of the database as it is being updated in between the two queries. This can result in extremely problematic discrepancies, as dependents in the 2nd query reference a principal that no longer exists, or worse, whose state has changed since in an incompatible way. This can be worked around by enclosing the split queries in a serializable (or snapshot) transaction, but that also creates its own problems and should not be done implicitly by EF.

In my personal opinion, this makes split queries unsuitable as a default loading strategy - safety and consistency should be the most important goal here, especially as long as alternatives do exist. In other words, you should definitely be able to opt into split queries if you're willing to assume the responsibility for possible consistency issues (by enclosing in a serializable transaction, or just because you know there's no concurrency).

Having said that, the team is currently actively discussing a more 1st-class split query option to simplify the workarounds that @smitpatel has already shown above. Expect more details soon.

roji on 8 May 2020

👍2 ❤1

@roji

[O]ne of the point that constantly gets missed/ignored in this discussion, is that multiple queries suffer from a severe consistency issue

I think we understand the consistency/safety issue. It's that it's just not a problem for many or perhaps most applications. On the other hand, unacceptable performance of commonplace 1:M queries by default is a HUGE problem for all applications.

@popcatalin81 has it right. I suspect that for all but the most trivial applications, M2M cardinality is large enough that split queries will perform better than single queries, so forcing us to use single queries (with no easy solution for split queries) to solve an edge-case problem at the cost of performance for the vast majority of applications seems like the wrong solution.

_This is why it's hard for us to understand the EF team's position on this._

@roji

[Y]ou should definitely be able to opt into split queries if you're willing to assume the responsibility for possible consistency issues (by enclosing in a serializable transaction, or just because you know there's no concurrency).

Right! So why is it not feasible to support transactions to provide isolation and/or implement something like EF+'s IncludeOptimized API to give us a choice?

@smitpatel

We decided that single query mode is right decision we have made and we will keep the behavior
We will look into ways if we can improve perf of single SQL scenario.

You can only do so much to optimize a cartesian explosion, and duplicate rows/cells will never be as efficient as split queries with no duplication.

Respectfully, I think this is the wrong approach to the problem.

Adding a hook which allows EF to generate split queries is infra costly and it has same bugs which made us change the behavior in first place. So we are not looking to add such a hook.

Is it really that costly that it's worth the impact to our application performance (and forcing us to just work around it anyway)?

Entity Framework Plus's IncludeOptimized seems to be a performant solution, but it's not compatible with AutoMapper on EF3 (with no plans to implement it any time soon as of yesterday), which makes it effectively useless if you need to use DTOs or otherwise use projection.

But if EF worked the way devs need and expect, EF+ wouldn't exist in the first place, so at the very least, please make it so we can easily use single or split queries as the situation warrants instead of forcing us to code up nontrivial workarounds to get acceptable performance for the most common use case.

@smitpatel (September 2019)

Depending on complexity of above, we may look into provide some API to user where they can pass multiple queries and we would stitch the result.

Why make us write two separate queries?

As I mentioned earlier in this issue, the Django team solved this problem in 2012 with select_related (single query) and prefetch_related (split queries), which I guess would look like .Include() and .IncludeOptimized() in EF.

@roji (May 2020)

Having said that, the team is currently actively discussing a more 1st-class split query option to simplify the workarounds that @smitpatel has already shown above. Expect more details soon.

Yes, that would be great. The framework should handle this common problem instead of making us fix it ourselves in every app. I think that's what we're asking for.

It shouldn't have taken 7 months of discussion to get the EF team to what seems to be the obvious solution.

mikeroque on 8 May 2020

This can be worked around by enclosing the split queries in a serializable (or snapshot) transaction, but that also creates its own problems and should not be done implicitly by EF.

Using the mechanisms specifically designed to solve these types of problems isn't acceptable, but using a hack that can bring applications to their knees is? To my (limited) knowledge snapshot transactions don't even lock aside from recovery scenarios. What was wrong with that approach? @roji

TylerDM on 8 May 2020

👍1

Filed #20892 to track first iteration of the solution we are going to provide in 5.0 release.

smitpatel on 8 May 2020

👍6 🎉2

@roji

@popcatalin81 and others, one of the point that constantly gets missed/ignored in this discussion, is that multiple queries suffer from a severe consistency issue. The two queries can see a different state of the database as it is being updated in between the two queries. This can result in extremely problematic discrepancies, as dependents in the 2nd query reference a principal that no longer exists, or worse, whose state has changed since in an incompatible way. This can be worked around by enclosing the split queries in a serializable (or snapshot) transaction, but that also creates its own problems and should not be done implicitly by EF.

The database can always be updated in between DML operations, and also in the time between the query is executed and the query is consumed. Heck, during a select on a set of rows in a database which uses MVCC (like oracle) it's not going to include the updated data but when the query is finished the state of the table has changed. So you'll have to work with stale data that might be out of date in any case, no matter what method you're using.

When querying a database and consuming data outside the database in a graph, you'll work with stale data, it doesn't matter how you fetch it: one big query might take longer and updates might be applied because of the read locks after the fetch has been completed (so the db is different at the time of consumption), and multiple queries might result in a graph that's different from the graph you'd get when you'd have fetched it in one go.

That last point is however not something that's fixable with a client consuming data in a graph form outside the database: because at one point can you say 'this is consistent with the database state at this time' ? The answer to that is ... never. So you have to accept data is stale when you consume it, and it's a reflection of a state that has passed, no matter what you do.

(edit) I do recall an old debate about this same topic when ObjectSpaces was in beta and they introduced the 'span' api (include / prefetch / eager load), where some people were against it as it would lead to inconsistencies and Warren argued it was stale data anyway (I remember as I then realized it was undoable to avoid stale data and thus inconsistency between client and server side data, one of these aha moments in a novice ORM developer :P ). It was in a newsgroup back in 2002/3, not sure if it's available still.

FransBouma on 9 May 2020

👍2

@FransBouma the point here is not that data can change at any time and become stale - that's indeed true. The problem is that when one LINQ query accessing navigations is executed via two SQL queries, EF needs to wire up the instances coming out of the first query to instances coming out of the second.

To give a concrete example, consider the following simple example with a Blog that has many Posts:

```c#
var results = ctx.Blogs
.Include(b => b.Posts)
.Where(b => b.Name.StartsWith("B"))
.ToList();

In EF Core 2.2, this would be executed via the following two queries (SQL is from PostgreSQL):

```sql
SELECT b."Id", b."Name"
FROM "Blogs" AS b
WHERE b."Name" LIKE 'B%'
ORDER BY b."Id";

SELECT "b.Posts"."Id", "b.Posts"."BlogId", "b.Posts"."Title"
FROM "Posts" AS "b.Posts"
INNER JOIN (
    SELECT b0."Id"
    FROM "Blogs" AS b0
    WHERE b0."Name" LIKE 'B%'
) AS t ON "b.Posts"."BlogId" = t."Id"
ORDER BY t."Id";

Now, if a certain Blog's Name started with B when the first query runs, but was renamed in between, the query results would include that blog, but with an empty collection of posts - although at no point did the blog post have zero posts in the database. This isn't stale data - it's false data, and can lead to quite severe corruption issues depending on the application.

In other words, again, the problem isn't that users get "a reflection of a state that has passed", but that they get a state that never existed. We still fully intend to provide the option to do this, but don't think it's right for the default behavior to contain such hidden, subtle concurrency issues that only manifest themselves under load etc.

roji on 9 May 2020

👍2

The example is indeed one that might give data that's not there. However you'll always have that. A select issues a readlock, which blocks writes till the readlock is lifted (by default at the end of the select). Consider this scenario (where the query is fetched using a join and thus in '1 go'.

connection 1 does the blogs join posts query and issues readlocks on both posts and blogs.
connection 2 wants to delete a post, which is part of the resultset of the select of connection 1. This delete is stalled
connection 1 now executes the select
connection 2 can proceed once the select is completed as the readlock will be lifted.

Now the data for connection 1 will contain a post that's not in the db. In both your and my example timing issues might cause the end result not match what's in the db: so in the end 'consistent' what does that mean: consistent with what exactly? the state of the db at the time of fetching? or when consuming the data?

Btw, you can optimize the 2nd query with an in clause using blog id's, not having the problem (but nitpick, I get the point you're trying to make, I just think the joined single query isn't free of consistency issues as well).

FransBouma on 9 May 2020

@FransBouma - Something like #12776

smitpatel on 9 May 2020

@smitpatel yes :)

FransBouma on 9 May 2020

@FransBouma you're still describing the situation as a staleness problem, whereas I don't think it is; there's a very big difference between a state that is no longer there and a state that never was there.

For the example above, if the application somehow reacts in a different way for a blog with no posts - for example delete the blog - then you end up deleting a blog thinking it had no posts, whereas in fact that was never the case. This isn't a timing/staleness issue - it's a data correctness issue.

To concentrate more narrowly on consistency, I mean consistency within the results returned by the LINQ query, internally - not consistency between the results and the latest database state (that's staleness). For example, say that for performance reasons, we denormalize some data that exists on the posts, and put it in a field on blog. Since the blogs and the posts come back in two queries, the denormalized data on blog can be completely inconsistent with the data in posts, which it is supposed to represent. The consistency I'm concerned about is between the blogs and the posts within the same LINQ query - and split queries don't provide that.

(just in case it isn't clear from the above - I'm very happy to be having this exchange and the fact we're being challenged on our reasoning!)

roji on 9 May 2020

you're still describing the situation as a staleness problem, whereas I don't think it is; there's a very big difference between a state that is no longer there and a state that never was there.
For the example above, if the application somehow reacts in a different way for a blog with no posts - for example delete the blog - then you end up deleting a blog thinking it had no posts, whereas in fact that was never the case. This isn't a timing/staleness issue - it's a data correctness issue.

It is a timing related issue, as in my example the update should have taken place but it was postponed and therefore the query reports 1 post too many that shouldn't have been there. To be fair, in both examples, they can occur because of a timing critical situation. I'm not denying the problem you describing btw, it's there, but I think you'll run into these kind of situations with a single query as well.

Your denormalized example can still lead to inconsistencies, as the denormalization itself can always lead to inconsistencies (same situation as yours: single query, an update has to take place on the posts and the blog row, the update on the blog row is done, the update on the same data in the posts rows is postponed due to a read lock of the joined fetch. If you have a count field on blog for the # of posts, you still can get inconsistencies in that single linq query:

connection 1: update blog B row's Count field -1
connection 2: perform select on blogs, fetching B too
connection 2: starts the select on posts to fetch the posts of the blogs, setting read / shared locks
connection 1: wants to delete the post of B but has to wait for its X lock due to the read lock on the post row
connection 2: performs the actual fetch of the post rows, results are joined by the database and returned as 1 resultset.
connection 1: gets the x lock, deletes the post.

you then have a blog row with a count that's not consistent with the # of posts merged into it.

One way to solve this if people are concerned about this edge case is using a transaction, that's what they're for. Not sure if that's possible to do within ef core's api (explicitly start a transaction when fetching).

FransBouma on 10 May 2020

@FransBouma if I'm understanding your example above, you're simply showing an update and a delete which aren't wrapped in a transaction, and which lead to an inconsistency. That's obviously true, but it's two separate database modifications. A user program doing this obviously contains a bug in its code and needs to use a transaction (if they need consistency). What I'm talking about is a situation where only a single update has occurred, causing a single LINQ query to return inconsistent results; that's a very different situation.

To put it differently, as per your example, multiple updates need to be wrapped in a transaction - this is a basic, known fact in any kind of database programming. One also doesn't need to think much about isolation level anywhere, as any parallel operations (queries or updates) will typically run with at least ReadCommitted by default. On the other hand, I'm interested in what happens on the querying side. if your LINQ query is executed via two SQL queries, then those queries also need to be wrapped by a transaction in order to return consistent results - whereas a single SQL query with JOIN does not. Furthermore, ReadCommitted (the default) is insufficient for multiple reads - you need Serializable (or Snapshot) to ensure that no concurrent updates leak into your selects.

So to reach the same consistency guarantees that a single-query (with JOINs) provides naturally, you need to wrap all your queries with serializable/snapshot transactions.

roji on 10 May 2020

The example I made can be run in a transaction or not, a transaction doesn't lock rows up front in tables it might touch, that's the thing: the read/shared lock set by the select on another connection will stall the update/delete. Also it's a single update/delete (still a transaction, as any insert/delete is transactional but semantics). It's not a bug in the client software, it's a result of how 2 parallel connections interfere with each other, which is the same with the problem you illustrated with multiple queries issued for a single linq statement.

So to reach the same consistency guarantees that a single-query (with JOINs) provides naturally, you need to wrap all your queries with serializable/snapshot transactions.

the transaction requirements are pretty steep, true, but you won't get consistency with your single query either: the data in a row in blogs can become inconsistent with a set of rows in posts as they're not fetched together: the locks aren't set at the same time. So as there are multiple tables involved, there can be a situation where a parallel connection might affect rows in posts. That's the same problem as you illustrated with the split query.

(edit). So IMHO, there are multiple ways to do this: each have their pro/cons, and for consistency reasons you can't avoid issues, but they can occur in different situations: so you might want to choose one over the other for different situations.

However, in practice these are all moot: the data you'll be consuming will be 'a' state, and you in all cases have to assume the rows related to the root rows (and rows related to the rows related to the root rows etc. etc.) might be rows fetched later on. If you define the eager loading as a snapshot isolated fetch, as you seem to do as I understand that's your argument to do a single joined version, then I think you run into problems as you can't guarantee that, unless you first set all the row locks and then fetch all the data.

FransBouma on 10 May 2020

I may be dense - or there may be an actual behavior difference here between PostgreSQL and SQL Server. In any case I'm genuinely interested in understanding exactly what you're saying and would be happy to be proven wrong.

The PostgreSQL docs say the following:

Read Committed is the default isolation level in PostgreSQL. When a transaction uses this isolation level, a SELECT query (without a FOR UPDATE/SHARE clause) sees only data committed before the query began; it never sees either uncommitted data or changes committed during query execution by concurrent transactions. In effect, a SELECT query sees a snapshot of the database as of the instant the query begins to run. However, SELECT does see the effects of previous updates executed within its own transaction, even though they are not yet committed. Also note that two successive SELECT commands can see different data, even though they are within a single transaction, if other transactions commit changes after the first SELECT starts and before the second SELECT starts.

Without discussing any locks or implementation details, to me this means that no matter what else is happening in the database, a single query will never return inconsistent results - regardless of whether you're querying one table, joining, or whatever. Note the contrast in the paragraph with "two successive SELECT commands" which can see different data (in Read Committed). I'm reading this to mean that a single query has no consistency issues, whereas split queries do.

Are we maybe talking about something that works differently across databases? Can you point to some docs if so?

roji on 10 May 2020

I wonder if PostgreSQL effectively implements the same behavior for Read Committed as SQL Server does when READ_COMMITTED_SNAPSHOT is on?

Edit: This article seems to confirm this:

To summarize, each statement within an RCSI transaction sees a static committed data set, but that set can change between statements inside the same transaction.

Generally very useful series about the isolation levels in SQL Server, and the two read committed modes in particular: https://sqlperformance.com/2014/07/t-sql-queries/isolation-levels

roji on 10 May 2020

👍1

In SQL Server, readers block writers and writers block readers, however in other databases this isn't the case, (I think postgresql indeed does things differently, due to it uses MVCC like oracle ? ), but that said, what you quoted that's about an update in progress, like a transaction T1 on connection C1 has updated a row R1 and a select of another connection C2 then reads R1 (or tries to) and sees the old results. In SQL Server, the C2 connection's read of R1 waits till T1 is committed/rolled back as the x lock on r1 is then lifted, in the default transaction isolation read committed.

Rereading my action order in my example (Please be aware I find this discussion pretty academic :P, as to me it's impossible to guarantee a single linq query is an atomic operation without a transaction ), I do realize the update+delete, if ran in a transaction, won't show the behavior as the select on blogs will stall as the transaction has an x lock on blogs. If the isolation level is readuncommitted, then the inconsistency can occur, but you asked for it.

FransBouma on 10 May 2020

👍1

Great, I'm very happy we've had this conversation - I've definitely learned a lot, thanks for that. PostgreSQL is indeed an MVCC database, and this seems to be exactly where it matters.

to me it's impossible to guarantee a single linq query is an atomic operation without a transaction

I now understand how that may be the case by default in SQL Server. However, is it correct to say that when SQL Server is used with READ_COMMITTED_SNAPSHOT (RCSI), that statement is no longer true, and all single queries are implicitly consistent/atomic (i.e. never see any updates - committed or uncommitted - during the course of their execution)?

roji on 10 May 2020

Correct, that's what I can make up from the docs too, it's basically enforcing what postgresql and oracle and other mvcc using databases do by default :)

And thank you for pointing out a situation with split queries. Luckily for me I define eager loading as a series of operations one can define together with a root query fetch (so not an atomic operation), but indeed if you define it as an atomic fetch then split queries might not be consistent unless RCSI is used (which is more costly).

It boils down a bit to a definition problem, perhaps? whether 'include' is seen as atomic or not? If you see it as 'include in the resultset' but don't define it as an atomic operation (I'd be careful with these as it's a client operation) you don't need to take measures. That does require the ability to explicitly enforce a start of a transaction tho (like ctx.StartTransaction(...) which then also has to explicitly be committed).

FransBouma on 10 May 2020

👍1

Yeah, I think we're completely aligned now in understanding the situation...

It's a fair point about this being a question of definition. I think we already agree that both single and split query modes should be made available, so this is more a question of what the default should be. I do lean towards guaranteeing that a single LINQ query return consistent results by default (even if it may not be on SQL Server by default), because I believe the default should be as safe as possible.. I prefer the potential perf issues of a single query with JOINs (which are hopefully more easily visible/discoverable) to subtle concurrency-related consistency issues.

A related question is whether when split queries are done (whether opt-in or by default), an ORM should wrap them in a serializable/snapshot transaction to again guarantee consistency even in that scenario. I'm pretty wary of implicitly starting a transaction with such a high isolation level, so it may be better left to users to do themselves - though again there's value in providing consistency by default without forcing users to think about all this complicated stuff.

Thanks again for this great conversation @FransBouma.

roji on 10 May 2020

👍4

BTW EF Core (as well as EF6) sets READ_COMMITTED_SNAPSHOT when it creates the database (see https://github.com/dotnet/efcore/issues/4809#issuecomment-283330842).

roji on 10 May 2020

👍2

Filed #20892 to track first iteration of the solution we are going to provide in 5.0 release.

Why is this fix postponed for the 5.0 release? We're having problems right now with the 3.0 and 3.1 releases.

We've just upgraded to .net Core 3.1, including EF Core 3.1, in the last month. Everything is running in our acceptance environment now. Initially when we started migrating a lot queries with includes got really bad performance after the upgrade due to the behavioral change of EF Core 3.0 and 3.1 for include statements. We thought we had fixed these issues by using Entity Framework Plus. Everything seemed okay in our Development environment. But we're now getting errors quite frequently in our Acceptance environment regarding the request limit on our azure databases (which is 90). Also specific azure functions are running into timeouts due to the default maximum time of 5 minutes. These issues all started exactly on the day that we upgraded our Acceptance environment.

It's not acceptable that the release of a fix in which we get back the old behaviour will only happen in the 5.0 release. With this breaking change in EF Core 3.x many systems just can not be upgraded without major performance issues.

Joehannus on 11 May 2020

👍2

We've just upgraded to .net Core 3.1, including EF Core 3.1, in the last month. Everything is running in our acceptance environment now. Initially when we started migrating a lot queries with includes got really bad performance after the upgrade due to the behavioral change of EF Core 3.0 and 3.1 for include statements. We thought we had fixed these issues by using Entity Framework Plus. Everything seemed okay in our Development environment. But we're now getting errors quite frequently in our Acceptance environment regarding the request limit on our azure databases (which is 90). Also specific azure functions are running into timeouts due to the default maximum time of 5 minutes. These issues all started exactly on the day that we upgraded our Acceptance environment.

Glad you found your issue before it got to production. Rollback! Then get on with the workarounds mentioned in this thread.

Unfortunately, you'll have to remove and change some includes to be multiple statements (they were always multiple SQL queries but just hidden).

adamhathcock on 11 May 2020

We've just upgraded to .net Core 3.1, including EF Core 3.1, in the last month. Everything is running in our acceptance environment now. Initially when we started migrating a lot queries with includes got really bad performance after the upgrade due to the behavioral change of EF Core 3.0 and 3.1 for include statements. We thought we had fixed these issues by using Entity Framework Plus. Everything seemed okay in our Development environment. But we're now getting errors quite frequently in our Acceptance environment regarding the request limit on our azure databases (which is 90). Also specific azure functions are running into timeouts due to the default maximum time of 5 minutes. These issues all started exactly on the day that we upgraded our Acceptance environment.

Glad you found your issue before it got to production. Rollback! Then get on with the workarounds mentioned in this thread.

Unfortunately, you'll have to remove and change some includes to be multiple statements (they were always multiple SQL queries but just hidden).

After the upgrade we also started doing functional changes. And a roll back to 2.1 is not an option, because we had already migrated to 2.2, which is now end of life. This is quite a hassle we got ourselves into now.

I hope increasing the DTU's on our databases, which were S1 until now, might solve some of the issues.

Joehannus on 11 May 2020

After the upgrade we also started doing functional changes. And a roll back to 2.1 is not an option, because we had already migrated to 2.2, which is now end of life. This is quite a hassle we got ourselves into now.

Non-sense. You can use EFCore 2.2 (or 2.1 is still LTS if you're that bothered) on .NET Core 3.1 as explained above.

You own your application, not Microsoft. If you're paying them directly for EFCore support, then this wouldn't be the channel to ask for paid help. It seems like you should pay for support to a vendor or switch to an ORM that has paid support.

adamhathcock on 11 May 2020

👎10

It looks like the majority of this discussion is around Include and navigation properties. Is there any workaround or upcoming feature related to splitting queries that would work for owned types? The initial design for #20892 doesn't seem like it would cover owned types (except maybe AsSplitQuery).

dandenton on 21 May 2020

his strive for the right choice seems like a strive for purity rather than what the consumers of EF Core 3.0 desired.

Yes, welcome to dealing with Microsoft, where they do what they want regardless of what the community wants.

dcb99 on 27 May 2020

👎4 👍1

The following is quoted from https://github.com/dotnet/efcore/issues/18022#issuecomment-586521749

Implement the loading mechanism described above (or something similar) as an "extension" that we can deliver now and use with 3.1.x code. "Deliver" here might mean copy/paste an extension method we give you into your code. It might mean we ship it in a NuGet package. (#19934)

The propose-punt label was just added, does that mean the 3.1.x helper will no longer be offered?

mguinness on 2 Jun 2020

😕2

@mguinness After much analysis on what we could do as an add-on for 3.1 verses implementing this properly in 5.0, we decided to direct our efforts into the 5.0 feature. This was for two main reasons:

There wouldn't be that much reuse between the two, and we wanted to focus resources on a solid, long-term solution that we can get into previews for people to use ASAP.
Anything we could do as an add-on would have limitations. After looking at what EF Plus provides, we decided that anything we add is likely to have similar limitations to the EF Plus implementation.

Therefore, we believe that people who need something to work with EF Core 3.1 should look into using EF Plus, which has a free version available. At the same time, the working feature for EF Core 5.0 will be merged soon.

Sorry for not providing this update sooner.

ajcvickers on 3 Jun 2020

👍9

If multiple SQL query statements were put in a single roundtrip and the queries were done within an appropriate transaction to ensure consistency, EF could consume the multiple result sets from that single roundtrip. What downsides would that have compared to the single query approach?

No more single query was the top reason I've been looking forward to porting to EF Core. For EF6, I gained a 30-50x throughput boost to many queries by querying through a helper class that generates and stitches the EF queries in multiple roundtrips. What would be ideal is a single roundtrip containing a query statement per table.

jnm2 on 8 Jun 2020

@jnm2 all this has already been addressed above. For example, ensuring consistency would mean using the serializable (or snapshot) isolation level, which comes with its own price and complications - which also vary across databases. I really believe it should be up to the user to make that decision, rather than it being done magically by EF. The plan is already to provide a way for you to opt into ~single query~ multiple queries for 5.0 - but not to make that the default.

roji on 8 Jun 2020

👍4

@roji I agree with that. The part I missed was where it was noted that multiple roundtrips are not required in order to do multiple queries.

jnm2 on 8 Jun 2020

👍1

Now that #21189 has been merged in, a new AsSplitQuery operator has been introduced and I was thinking that it would be nice to introduce it's counterpart e.g. AsSingleQuery and the ability to configure the default behavior at the context level e.g. context.QueryOptions.DefaultQueryLoadingBehavior = QueryLoadingBehavior.SplitQuery;
in order to be able to opt-in/opt-out easily without the need to add a AsSplitQuery to every query that eagerly loads related entities because it's a bit tedious and prone to errors IMO

ilmax on 12 Jun 2020

👍8

Preview6 introduces AsSplitQuery API to load collections using multiple queries. Details of the API has been posted https://github.com/dotnet/efcore/issues/20892#issuecomment-644456029
The API is available in nightly builds.

smitpatel on 16 Jun 2020

👍5

Preview6 introduces AsSplitQuery API to load collections using multiple queries. Details of the API has been posted #20892 (comment)
The API is available in nightly builds.

Thank you @smitpatel! Hopefully I can test it out soon with the same code from before.

dragnilar on 17 Jun 2020

When the preview6 will be available on NuGet, so I can install it?

eddyzhu1980 on 20 Jun 2020

@eddyzhu1980 It's in the daily builds now.

ajcvickers on 20 Jun 2020

Is it possible to configure the default query behavior of the DbContext to execute all queries as split queries? If not, I would love to see this added. Something like this:

services.AddDbContext<ApplicationContext>(options =>
{
    options.UseSqlServer(this.configuration.GetConnectionString("DefaultConnection"));
    options.UseQueryBehavior(QueryBehavior.SplitQuery);
});

and adding an overload to AsSplitQuery to allow you to disable this behavior on demand via .AsSplitQuery(false) or adding another quarriable extension AsSingleQuery?

ChristopherHaws on 10 Jul 2020

👍1

@ChristopherHaws - See https://github.com/dotnet/efcore/issues/21355
We have already added methods. They should be available in preview8

smitpatel on 10 Jul 2020

🎉6 👍2

For anyone interested in the new split query functionality, some new docs are now live: https://docs.microsoft.com/en-us/ef/core/querying/related-data#single-and-split-queries

roji on 10 Jul 2020

👍4 🎉3 🚀2

@roji Have you entertained trying to eliminate the cartesian explosion by doing a single join but to a concatenation of tables? For example,

using (var context = new BloggingContext())
{
    var people = context.Set<Person>()
        .Include(p => p.AuthoredPosts)
        .Include(p => p.OwnedBlogs)
        .ToList();
}

could translate to something like

SELECT [p].[PersonId], [p].[Name], [p].[PhotoId], [u].[p0_PostId], [u].[p0_AuthorId], [u].[p0_BlogId], [u].[p0_Content], [u].[p0_Rating], [u].[p0_Title], [u].[b_BlogId], [u].[b_OwnerId], [u].[b_Rating], [u].[b_Url]
FROM [Person] AS [p]
LEFT JOIN (
    SELECT [p0].[AuthorId] as [PersonId], [p0].[PostId] AS [p0_PostId], [p0].[AuthorId] AS [p0_AuthorId], [p0].[BlogId] AS [p0_BlogId], [p0].[Content] AS [p0_Content], [p0].[Rating] AS [p0_Rating], [p0].[Title] AS [p0_Title], NULL AS [b_BlogId], NULL AS [b_OwnerId], NULL AS [b_Rating], NULL AS [b_Url]
    FROM [Posts] AS [p0]
    UNION ALL
    SELECT [b].[OwnerId] as [PersonId], NULL AS [p0_PostId], NULL AS [p0_AuthorId], NULL AS [p0_BlogId], NULL AS [p0_Content], NULL AS [p0_Rating], NULL AS [p0_Title], [b].[BlogId] AS [b_BlogId], [b].[OwnerId] AS [b_OwnerId], [b].[Rating] AS [b_Rating], [b].[Url] AS [b_Url]
    FROM [Blogs] AS [b]
) AS [u] ON [p].[PersonId] = [u].[PersonId]
ORDER BY [p].[PersonId], [u].[p0_PostId], [u].[b_BlogId]

brunom on 13 Jul 2020

@brunom it's an interesting suggestion... However, the principal table's columns (Person) are still duplicated for each dependent row. Also, nulls are getting sent across the wire for all columns on each side of the union; although the data impact is likely to be small (because null), as you add more includes the number of nulls needlessly getting sent grows very quickly... So I wouldn't consider this a very good general-purpose alternative. Have you had much experience with this pattern, any concrete perf data to share?

roji on 13 Jul 2020

Usage of Union can also break index usage for join.

smitpatel on 13 Jul 2020

👍2

IIRC EF6 was doing something similar when including multiple collections.

The union strategy would probably result in an order of magnitude fewer cells for non-trivial row counts. Column count is the same: all columns in all extents (+marker), but row count would be sum of row counts vs product of row counts.
Do some providers support sparse columns on the wire?

It's only my experience but I believe major dbs are capable of pushing down the join predicate to the extents of the union. For the simple FK case at least.

bachratyg on 13 Jul 2020

I've seen this issue as well. My version is much more reasonable than the OP's:

        public IEnumerable<User> GetCurrentUsers() => _context.Users
                                                         .Include(u => u.QuickNotes)
                                                         .Include(u => u.UserQuickNoteSets)
                                                         .ThenInclude(uqn => uqn.QuickNote)
                                                         .Include(us => us.UserSetting) 
                                                         .Include(u => u.RoleMemberships)
                                                         .ThenInclude(m => m.Role)
                                                         .ThenInclude(r => r.RightsAssignments)
                                                         .ThenInclude(a => a.Right)
.Where(u => u.IsOnDuty).ToList()

Ef 2.2 run's this as 3 queries. The entire operation takes less than 1 sec and returns 8 rows. Running on 3.1, 1 query, takes 8 minutes and returns 110863 rows that then have to be mapped down into 8 entities

We were planning on flatting our queries anyway, but this difference is remarkable.

JasonDWilson on 13 Jul 2020

😄7 ❤2 👀1

I've seen this issue as well. My version is much more reasonable than the OP's:
        public IEnumerable<User> GetCurrentUsers() => _context.Users
                                                         .Include(u => u.QuickNotes)
                                                         .Include(u => u.UserQuickNoteSets)
                                                         .ThenInclude(uqn => uqn.QuickNote)
                                                         .Include(us => us.UserSetting) 
                                                         .Include(u => u.RoleMemberships)
                                                         .ThenInclude(m => m.Role)
                                                         .ThenInclude(r => r.RightsAssignments)
                                                         .ThenInclude(a => a.Right)
.Where(u => u.IsOnDuty).ToList()
Ef 2.2 run's this as 3 queries. The entire operation takes less than 1 sec and returns 8 rows. Running on 3.1, 1 query, takes 8 minutes and returns 110863 rows that then have to be mapped down into 8 entities

We were planning on flatting our queries anyway, but this difference is remarkable.

Hehe, I never said my example was "reasonable". 😄

I haven't had a chance to try it with the release preview since we're pretty swamped at the moment due to some other circumstances, and frankly, the work around we found with EF Plus has been working great. If I do have a chance, I will try out the release preview of Net 5 to see if its any faster than EF Plus' implementation.

dragnilar on 13 Jul 2020

👍1

Sorry no offense meant. Just mean that it happens with a fewer number of joins as well.

The first I've heard of EF Plus. IncludeOptimized breaks the call down to seperate queries again?

JasonDWilson on 13 Jul 2020

@roji Yes, null and duplicate cells persist, but it stops the row explosion. Joining to a union may still be slower than AsSplitQuery() sometimes, but it's always faster than the default join cascade.

I got no experience with the join to union pattern. I came up with it and haven't seen it elsewhere.

Usage of Union can also break index usage for join.

@smitpatel Actually, it improves index usage.

Let's create 100.000 Person records, each with 10 Blogs and 10 Posts:

context.Database.ExecuteSqlRaw(@"
    INSERT INTO Person(Name)
    select top(100*1000) 'xx'
    FROM sys.objects s1
    CROSS JOIN sys.objects s2 
    CROSS JOIN sys.objects s3 
    CROSS JOIN sys.objects s4 
    CROSS JOIN sys.objects s5");
for (int i = 0; i < 10; ++i)
{
    context.Database.ExecuteSqlRaw(@"
        insert into Blogs(OwnerId)
        select PersonId
        from Person");
}
context.Database.ExecuteSqlRaw(@"
    insert into Posts(AuthorId, BlogId, Rating)
    select OwnerId, BlogId, 42
    from Blogs");

And then have a query that needs to scan the whole Person table to return only 10 records:

context.Set<Person>()
.OrderBy(p => p.Name).Take(10)
.Include(p => p.AuthoredPosts)
.Include(p => p.OwnedBlogs)
.ToList();

AsSplitQuery() needs to scan the Person table 3 times, in each sub query, and does 486+(486+345)+(486+345)=2148 logical reads in total. Here are the plans

https://www.brentozar.com/pastetheplan/?id=rJi6Ar9yv
https://www.brentozar.com/pastetheplan/?id=ByJNyUq1P
https://www.brentozar.com/pastetheplan/?id=BylH185yP

Without AsSplitQuery(), logical reads climb to 4161 because of the cartesian explosion. SQL Server not only sends each Blog 10 times, it reads it 10 times as well:

https://www.brentozar.com/pastetheplan/?id=H1Gty8cJD

When joining to a union all, the logical reads are only 1146 and the plan is perfect https://www.brentozar.com/pastetheplan/?id=BkTnxI91P

Here's the SQL

SELECT [t].[PersonId], [t].[Name], [t].[PhotoId], [u].[p0_PostId], [u].[p0_AuthorId], [u].[p0_BlogId], [u].[p0_Content], [u].[p0_Rating], [u].[p0_Title], [u].[b_BlogId], [u].[b_OwnerId], [u].[b_Rating], [u].[b_Url]
FROM (
    SELECT TOP(10) [p].[PersonId], [p].[Name], [p].[PhotoId]
    FROM [Person] AS [p]
    ORDER BY [p].[Name]
) AS [t]
LEFT JOIN (
    SELECT [p0].[AuthorId] as [PersonId], [p0].[PostId] AS [p0_PostId], [p0].[AuthorId] AS [p0_AuthorId], [p0].[BlogId] AS [p0_BlogId], [p0].[Content] AS [p0_Content], [p0].[Rating] AS [p0_Rating], [p0].[Title] AS [p0_Title], NULL AS [b_BlogId], NULL AS [b_OwnerId], NULL AS [b_Rating], NULL AS [b_Url]
    FROM [Posts] AS [p0]
    UNION ALL
    SELECT [b].[OwnerId] as [PersonId], NULL AS [p0_PostId], NULL AS [p0_AuthorId], NULL AS [p0_BlogId], NULL AS [p0_Content], NULL AS [p0_Rating], NULL AS [p0_Title], [b].[BlogId] AS [b_BlogId], [b].[OwnerId] AS [b_OwnerId], [b].[Rating] AS [b_Rating], [b].[Url] AS [b_Url]
    FROM [Blogs] AS [b]
) AS [u] ON [T].[PersonId] = [u].[PersonId]
ORDER BY [t].[Name], [t].[PersonId], [u].[p0_PostId], [u].[b_BlogId]

By the way, EF Core can't run a query to force a join to a union all:

context.Set<Person>()
.OrderBy(p => p.Name).Take(10)
.Select(p => new { p, u = p.AuthoredPosts.Select(ap => new { ap, ob = (Blog)null }).Concat(p.OwnedBlogs.Select(ob => new { ap = (Post)null, ob })) })
.AsEnumerable()
.Select(x => x.p)
.ToList();

It throws 'When performing a set operation, both operands must have the same Include operations.'

brunom on 13 Jul 2020

@brunom I've taken another look at your proposal and here are a few thoughts.

First, your proposal seems to work well as long as we're dealing with a shallow graph where the collection includes are at the same level (i.e. with the dependents referencing the same principal); in your example, Post seems to reference both Blog and Person directly, whereas in the typical example it would only reference Blog:

c# context.Set<Person>() .Include(b => b.Blogs) .ThenInclude(p => p.Posts) .ToList();

Unless I'm missing something (would be happy to be corrected!), for this case your proposal isn't possible (query splitting, on the other hand, can handle this without any issues). Another unrelated issue as already mentioned, is the duplication of principal row data just like in the regular single query scenario.

To summarize, if the above is correct, then this doesn't seem like a better alternative to query splitting in the general case, but could indeed be an optimization to single query, when loading multiple collection navigations at the same level. This would still require a more thorough perf investigation; for one thing, the interaction between the set operation and index usage needs to be investigated in other databases - not just SQL Server - I'm actually going to look into this as part of an unrelated investigation (#21509).

Can you please open a new issue with your proposal so we can look into it in the future? Unfortunately this is too late for the the 5.0 release though.

@smitpatel any further thoughts here?

roji on 14 Jul 2020

Just a quick question about this: I arrived here from an SO question about why 3.x was slowing down on a query that involved Include and the reason for the performance loss was cited as "an order by has been added to the query". Note: I've just taken that at face value and not done any investigation as to what cost to performance the ordering operation has introduced.

It made me wonder; when efcore is resolving Cartesian explosions on the client side, does it do it as a "order by in the db, then a 'if the current row's X PK column values are different to the previous row's PK column values, make a new parent entity, else reuse the current parent entity' as it processes the data row by row" ?

If this is the case, and an order by is a performance hit, is there any scope for removing the order by on the db side and instead performing a lookup/keeping a dictionary style indexer of the entity in the client side so the db doesn't have to do an order by?

If it's not the case, what purpose does the orderby serve, and why does it include more than just ID columns sometimes?

CaiusJard on 29 Jul 2020

@CaiusJard your description of how EF Core loads related entities in 3.1 is pretty accurate - but we haven't necessarily seen evidence that the ORDER BY clauses are the cause of significant perf degradation, as opposed to the cartesian explosion itself. See #19571 for a discussion specifically on this, and note that #19828 is about removing the last ORDER BY, which indeed isn't necessary.

To keep this already huge thread focused, I propose that if you want to continue discussing this, please post on one of these two issues rather than here.

roji on 29 Jul 2020

👍1

I am not sure this is the correct issue for this question, however this issue is why I am facing this problem. I need to query a table with multiple joins without tracking. Since splitting query manually is not possible without tracking entity, I did some workaround (executing query with tracking enabled and detach the entity). But our dbContext is scoped and I can't safely clear every tracked entity.
c# public void DetachEntity(TEntity entity){ _dbContext.ChangeTracker.TrackGraph(entity, EntityState.Detached, c => { if (c.Entry.State == EntityState.Detached) return false; c.Entry.State = EntityState.Detached; return true; }); }
Above method clears the entity and all of it's relational entities. But there is warning in TrackGraph method which led me here.
...This overload, on the other hand, allows the callback to decide when traversal will end, but the onus is then on the caller to ensure that traversal will not enter an infinite loop.
Is the above method safe to use for detaching tracked entity?

mtnoztrk on 3 Sep 2020

Since splitting query manually is not possible without tracking entity

It is possible as written in the gist.

smitpatel on 3 Sep 2020

Since splitting query manually is not possible without tracking entity

It is possible as written in the gist.

Do you mean the opening post? Because afaik EF Plus needs tracking enabled.

mtnoztrk on 3 Sep 2020

This link https://gist.github.com/smitpatel/d4cb3619e5b33e8d9ea24d3f2a88333a

smitpatel on 3 Sep 2020

👍1

In #20892 we are getting the AsSplitQuery method. Will that work when you use own DbParameter and multiples Includes? If we reusing the same SqlParameter in EfCore3.1 in multiple queries an The SqlParameter is already contained by another SqlParameterCollection. exception is throw. This is described in #11370 and is marked as solved with the #12098, but is this case also solved when using the new AsSplitQuer function?

For EfCore 3.1 I'm using this as a workaround to fix this

using System;
using System.Data.Common;
using System.Linq;
using System.Reflection;
using Microsoft.Data.SqlClient;
using Microsoft.EntityFrameworkCore.Storage;
using Microsoft.EntityFrameworkCore.Storage.Internal;

namespace Test.Application.Data.EfCoreOverrides
{
    // register this as IRelationalCommandBuilderFactory in the service provider
    internal class ClonableRelationalCommandBuilderFactory : RelationalCommandBuilderFactory
    {
        public ClonableRelationalCommandBuilderFactory(
            RelationalCommandBuilderDependencies dependencies)
            : base(dependencies)
        {
        }

        public override IRelationalCommandBuilder Create()
        {
            return new ClonableRelationalCommandBuilder(Dependencies);
        }
    }

    [System.Diagnostics.CodeAnalysis.SuppressMessage("Usage", "EF1001:Internal EF Core API usage.", Justification = "<Pending>")]
    internal class ClonableRelationalCommandBuilder : RelationalCommandBuilder
    {
        private static readonly FieldInfo _parameterFieldInfo = typeof(RawRelationalParameter)
            .GetField("_parameter", BindingFlags.Instance | BindingFlags.NonPublic);

        public ClonableRelationalCommandBuilder(
            RelationalCommandBuilderDependencies dependencies) : base(dependencies)
        {
        }

        public override IRelationalCommandBuilder AddParameter(IRelationalParameter parameter)
        {
            if (parameter is CompositeRelationalParameter compositParameter)
            {
                var clonedParameters = new CompositeRelationalParameter(compositParameter.InvariantName, compositParameter.RelationalParameters
                    .Select(x =>
                    {
                        if (x is RawRelationalParameter rawParameter)
                        {
                            var dbParameter = (DbParameter)_parameterFieldInfo.GetValue(rawParameter);
                            return new ClonableRawRelationalParameter(x.InvariantName, dbParameter);
                        }
                        return x;
                    })
                    .ToList());

                return base.AddParameter(clonedParameters);
            }

            return base.AddParameter(parameter);
        }
    }

    [System.Diagnostics.CodeAnalysis.SuppressMessage("Usage", "EF1001:Internal EF Core API usage.", Justification = "<Pending>")]
    internal class ClonableRawRelationalParameter : RawRelationalParameter
    {
        private static readonly FieldInfo _parentFieldInfo = typeof(SqlParameter)
            .GetField("_parent", BindingFlags.Instance | BindingFlags.NonPublic);

        public ClonableRawRelationalParameter(string invariantName, DbParameter parameter) : base(invariantName, parameter)
        {
        }

        public override void AddDbParameter(DbCommand command, object value)
        {
            if (value is SqlParameter
                && !(_parentFieldInfo?.GetValue(value) is null))
            {
                base.AddDbParameter(command, (DbParameter)((ICloneable)value).Clone());
                return;
            }

            base.AddDbParameter(command, value);
        }
    }
}

@smitpatel

Tasteful on 10 Sep 2020

@Tasteful - Filed #22483

smitpatel on 10 Sep 2020

👍1

I think the new issue-number is wrong, should be #22483

Tasteful on 10 Sep 2020

Yes, my 2 key is faulty.

smitpatel on 10 Sep 2020

😄2

This link https://gist.github.com/smitpatel/d4cb3619e5b33e8d9ea24d3f2a88333a

As a side effect of this workaround I have noticed that the List<> properties are now null, where earlier they would be just empty Lists. This means I need to check against null in many other places where i would just do an Any()

Gio2k on 11 Sep 2020

I'm surprised to see so few mentions of the increased memory usage caused by the "single query / no automatic splitting" feature in EF Core 3. I've just hunted down a tenfold memory increase after moving to EF Core 3.1.10 (from 2.2.6) and the cause was that the query was no longer automatically split into 5 simpler queries.

EF Core 2 - ~120 MB used
big asset import - efcore2

EF Core 3 - ~1.3 GB used
big asset import - efcore3

This has a huge impact.

FYI, the EF Core 2 query looked like:

var company = await _dbContext.Companies
                .Include(c => c.Sites)
                .Include(c => c.Categories)
                .Include(c => c.Assets).ThenInclude(a => a.Site)
                .Include(c => c.Assets).ThenInclude(a => a.Tasks)
                .AsTracking()
                .SingleOrDefaultAsync(c => c.Id == companyId);

dstj on 24 Nov 2020

👍1

@dstj Please open a new issue and attach a small, runnable project or post a small, runnable code listing that reproduces what you are seeing so that we can investigate.

ajcvickers on 24 Nov 2020

Closing this discussion issue since 5.0 has now shipped with split-query support for most scenarios. Remaining work is tracked by #21234.

ajcvickers on 24 Nov 2020

@ajcvickers @smitpatel

FYI, we had a chance to test this today with the original query and 5.0 did resolve the performance problem (we had to enable Split Query mode obviously). I am noticing side effects of #22283, but we have our work around with EF Plus for now.

Thanks for addressing this and keeping the 🦄 magic going! 👍

dragnilar on 25 Nov 2020

❤3

Updated the pending item issue number to #21234

smitpatel on 25 Nov 2020

Efcore: Significant Query Slowdown When Using Multiple Joins Due To Changes In 3.0

* This older issue has been re-purposed to be discussion about different approaches, etc. It will not be locked because it is important to us that we don’t shut down channels of communication with the community.

Steps to reproduce

Results:

Further technical details

Most helpful comment

All 236 comments

12098 captures the discussion over the design which should satisfy anyone's interest.

Loading data for a single entity requires storing the entity and the query

A manual check before calling LoadAsync is necessary to prevent empty queries

AsNoTracking is not usable anymore

Relying on `null` for non-loaded collection properties is no longer possible.

Generic paging / sorting methods are more complex

This rewrite might also be necessary later when performance problems arrise

??? Switching to a document DB like Cosmos requires reverting to the old code

Related issues

Efcore: Significant Query Slowdown When Using Multiple Joins Due To Changes In 3.0

* This older issue has been re-purposed to be discussion about different approaches, etc. It will not be locked because it is important to us that we don’t shut down channels of communication with the community.

Steps to reproduce

Results:

Further technical details

Most helpful comment

All 236 comments

12098 captures the discussion over the design which should satisfy anyone's interest.

Loading data for a single entity requires storing the entity and the query

A manual check before calling LoadAsync is necessary to prevent empty queries

AsNoTracking is not usable anymore

Relying on null for non-loaded collection properties is no longer possible.

Generic paging / sorting methods are more complex

This rewrite might also be necessary later when performance problems arrise

??? Switching to a document DB like Cosmos requires reverting to the old code

Related issues

Relying on `null` for non-loaded collection properties is no longer possible.