-
Notifications
You must be signed in to change notification settings - Fork 3.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
filtered include generates invalid SQL with First() but not with ToList() #26585
Comments
likely fixed by #24491 |
thanks, they all seem like 6.0.0 fixes, any knowledge of this being fixed for EF core 5.x? |
@thinkOfaNumber there is little chance of the fixes getting backported to 5.0 at this point; the change is non-trivial and would risky breaking other things, and with 6.0 now out, 5.0 will be out of support in 6 months anyway. I'd highly recommend considering upgrading to 6.0, which is a long-term support release (3 years). |
I will eventually upgrade to ef-core 6 but that requires .net core 6 which I don't quite have access to yet in production. Plus there are a bunch of breaking changes I have to get through. So if filtered includes with eager loading was never and is never going to be supported in 5.x there should at least be a note in the documentation stating as much. |
Hi @ajcvickers, which bug is this a duplicate of? Thanks. |
Duplicate of #17337 for the root cause. |
Thanks @smitpatel It looks like #17337 is fixed in 6.x? So does that mean this bug won't be fixed in the (albeit small) remaining support period for efcore 5? If that's the case I highly recommend this needs to be detailed in the documentation, as 5.x is still an officially supported version. Would I open an issue here for the documentation or how would I go about that? thanks |
The bug won't be fixed in 5.0 release. There are several scenarios with filtered include which works correctly in 5.0 release. Writing in documentation that it is not supported is misleading and incorrect. Given LINQ query has infinite permutation possible, we don't expect every single scenario to work in every release. We strive to make work and document if things don't work for major scenarios where multiple people are running into issue. This issue doesn't fall into such case (yet) and documentation relating to this will provide negative value (apart from the issue how would you explain customer that what doesn't work and how can they relate to it when they are writing queries from scratch). It is different from someone running into same issue and trying to figure out the root cause. This issue will serve purpose of figuring out for future customers if they run into same issue. If we see a lot of customers hitting this, we will consider documenting something. |
I disagree with almost everything you're saying. I'm not asking filtered includes to be marked as "not supported" in general! However you have a major limitation with it, and noting it could save hours of trying and testing, looking for bugs, stack overflow, etc. It seems you're trying to hide it. I get that software has bugs, I get that you have another major release which makes 5.x obsolete, but is sweeping it under the carpet the correct ethos?
Really? generating invalid SQL is ok? I've never been about to do that in many years of using EF... Not to say it's never been possible but I've never come across it till now.
I think it's a pretty major scenario that is documented to be able to be used like this. The documentation literally has the orderby ... skip example in it. Are you saying adding
You're making this too easy for me! This is a duplicate of #17337 which has these other issues mentioned by those non-existent other people finding the same or similar problems: #18738 #19763 #19947 #17809. I didn't even try but if I did I'm sure I could find more on the web in general.
Saving developers time by not making them hunt around on stack overflow, search and raise bugs, expect the feature to work, this is all negative value?
Easily. Perhaps under this paragraph:
Add
Or alternatively you could put it under the final I don't even care if you don't use the word "bug". You (docs.microsoft.com) have the capacity to mark documentation as relevant to specific versions, and I see this a lot, so this need not be even mentioned in the 6.x documentation. That's probably enough said by me, I don't need to argue this forever. I appreciate you reading this far. Thanks. |
File a bug
When I use a filtered include and then First() on the parent query, EF Core 5 generates invalid SQL. I am scaffolding from a db-first project, however I reproduced this on a code-first project as well, which I've detailed below. My repro is here: https://github.com/thinkOfaNumber/efcore-5-test (I will spend a few minutes making it smaller).
I have recently posted the same here: https://stackoverflow.com/questions/69880136/invalid-column-name-when-using-ef-core-filtered-includes
This seems a similar-but-different problem to this bug: #8823
Include your code
The simplified idea is that a device has:
IOW you can move a device around to locations (or no location) and keep track of that over time.
The code-first model I came up with to simulate this is as follows:
Running the following query to select all devices works fine. I'm using a filtered include to only select the most recent history for this "view":
that works fine, however when I try and select only one device by ID using the same includes:
I get the following error:
For completeness,
ToModel()
is just an extension method to return a POCO.Include verbose output
The debug output for queries is as follows:
.First(d => d.DeviceId == deviceId)
debug output:And the successful
.Select(d => d.ToModel()).ToList();
output:Include provider and version information
EF Core version: 5.0.12
Database provider: Microsoft.EntityFrameworkCore.SqlServer
Target framework: .NET Core 3.1
Operating system: Windows 10 Home 21H1 19043.1288
IDE: Microsoft Visual Studio Community 2019 Version 16.11.5
The text was updated successfully, but these errors were encountered: