Skip to content
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

After upgrading to EFCore 6 from EFCore 5 Included data are missing when using pagination (take/skip) #27193

Closed
ghost opened this issue Jan 15, 2022 · 11 comments

Comments

@ghost
Copy link

ghost commented Jan 15, 2022

Hi guys,

we've upgraded our project to EFCore 6 and our database services started to struggle with loading related data using Eager Loading meaning that when loading multiple records from DB most of the records have empty Navigation Properties. On EFCore 5 everything works OK, after upgrading to EFCore 6 data are missing. After further investigation we found out that our pagination (LINQ combination of skip + take) is causing this problem.

Since we use several levels of inheritance and some abstractions it will be easier to post resulting LINQ query from Debug view as it looks exactly as if it was written directly in code:

  DbSet<TimeTable>()
      .AsNoTracking()
      .Where(pdm => pdm.NotAfter == null || pdm.NotAfter >= <>c__DisplayClass6_0.effectiveDateTime)
      .Include(timeTable => timeTable.WeekdayDefinitions)
      .Include(timeTable => timeTable.SaturdayDefinitions)
      .Include(timeTable => timeTable.SundayDefinitions)
      .Include(timeTable => timeTable.PublicHolidayDefinitions)
      .AsSplitQuery()
      .OrderByDescending(x => x.NotBefore)
      .Skip(0)
      .Take(10);

When this query executes the service will return collection of 10 records where 9 have empty Navigation Properties and the 10th has them populated. In EFCore 5 this query works fine and all records have populated all Navigation Properties but in EFCore 6 it doesn't work as intended anymore.

As we have exhausted all our possible ideas to fix this issue we decided to write here. Could you please point us where the problem might be?

Thank you.

EF Core version: 6.0.1
Database provider: Microsoft.EntityFrameworkCore.Sqlite
Target framework: .NET 6.0
Operating system: Windows 10
IDE: Visual Studio 2022

@ghost ghost added the customer-reported label Jan 15, 2022
@roji
Copy link
Member

roji commented Jan 16, 2022

@DanVUT it seems like your query is non-deterministic: you're ordering by a boolean property (x.NotBefore), which means that the order of all rows with the same NotBefore value is non-deterministic. When using split query, it's especially important for queries to be 100% deterministic, since the two split SQL queries may return different orderings.

Can you try to add a ThenBy operator after your OrderByDescending, in order to make the ordering fully deterministic? For example, try ordering by the ID.

However, I'm not sure this should have changed from EF Core 5 to 6. Also, we may want to add a doc note about deterministm to the split query doc page.

@ghost
Copy link
Author

ghost commented Jan 16, 2022

@roji Hi. The property NotBefore is DateTime, not Boolean. Also the property NotAfter is DateTime. So I think that it should be pretty deterministic. But we will try to add more ordering so it would be as much deterministic as possible.

But as I mentioned before. On EFCore 5 it worked fine. And nothing has changed except for EFCore version.

@roji
Copy link
Member

roji commented Jan 16, 2022

Sorry, I misread the property as being boolean - but what I wrote applies. If you have multiple rows with the same NotAfter timestamp, their order is non-deterministic and could cause the issues above. I'm again not sure why it worked in EF Core 5 - there's a possibility it did only by chance (/cc @smitpatel in case I've forgotten a change we did here).

@ghost
Copy link
Author

ghost commented Jan 17, 2022

@roji Hi, I’ve tried adding one more ordering by PK which is UUID column and it did the trick :) thank you guys for helping.

@ghost ghost closed this as completed Jan 17, 2022
@ajcvickers
Copy link
Member

@roji Could Removed last ORDER BY when joining for collections have had an impact here?

@ajcvickers ajcvickers reopened this Jan 17, 2022
@roji
Copy link
Member

roji commented Jan 17, 2022

Yeah, it could - good catch. Though we specifically discussed this and agreed that we're OK with user queries breaking, if they implicitly relied on this ordering which wasn't explicitly stated in the query.

@ajcvickers
Copy link
Member

ajcvickers commented Jan 17, 2022

@roji Agreed. Just a reason why the behavior could have changed, and maybe something to add to the breaking change as a place where ordering matters? Although we do warn already.

@roji
Copy link
Member

roji commented Jan 17, 2022

Yeah...

I do think it's worth adding a note in the split query docs, and possibly move the determinism warning in the new pagination docs up to be more front and center. This is a tricky requirement that's clearly easy to overlook...

@ajcvickers
Copy link
Member

Discussed in triage: @roji to make sure docs contain warning or issue is filed.

@smitpatel
Copy link
Member

Removing ordering should have no impact here as it should be removed in both queries. Need to look at generated SQL.

roji added a commit to roji/EntityFramework.Docs that referenced this issue Jan 19, 2022
In split query, and also a stronger note in the pagination page.

See dotnet/efcore#27193
@roji
Copy link
Member

roji commented Jan 19, 2022

Docs issue: dotnet/EntityFramework.Docs#3242

@ajcvickers ajcvickers reopened this Oct 16, 2022
@ajcvickers ajcvickers closed this as not planned Won't fix, can't repro, duplicate, stale Oct 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants