-
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
EF CORE 5.0 ThenInclude OrderBy, uses PK before OrderBy field, rendering it useless #26343
Comments
This issue is lacking enough information for us to effectively reproduce. Please post a runnable project/solution or complete code listing that demonstrates the behavior you are seeing. |
Hey, This is essentially a duplicate of #9067 Thanks |
@smitpatel - My project is too big to share any kind of runnable code. Like @Bitz mentioned this is an ongoing issue and #9067 is closed so I opened this one. The issue is that when EF Core generates sql at the end it uses PK's to create the 'ORDER BY' sql script. I am guessing that the parser is prioritizing PK's over what the client supplies in the context request. So Include / ThenInclude's that have OrderBy chain the user supplied field to the end of the generated sql going to SQL server. Example in the case of @Bitz supplied snip in #9067 var form = _db.Forms
.Include(x => x.Sections)
.ThenInclude(x => x.Rows.OrderBy(y => y.SortOrderofRows)) // <===== Issue: this gets added to the end of generated Sql after it's PK
.ThenInclude(x => x.Fields.OrderBy(y => y.SortOrderOfFields)) // <====== Issue: same as above
.ThenInclude(x => x.FieldType)
.AsSplitQuery()
.SingleOrDefaultAsync(x =>
x.Id == formId) [SortOrderofRows] and [SortOrderOfFields] will get added to the end of the generated sql right after its PK, so SQL server sorts first on PK then the user supplied fields in .OrderBy(y => y.USERSUPPLIEDFIELD). I used SQL profiler to see what is be generated. Then I cut paste the generated sql as a query, if I switch the PK and my OrderBy Field around in the generated sql it all works as expected. But as it is now the sort happens first on the PK then on other fields so it makes OrderBy somewhat useless in ThenInclude / Include . If you like I can do a video chat of the problem. But can not post any runnable code since this would require a good deal of setup. Thanks |
This is not duplicate of #9067 using OrderBy inside Include, tries to populate collection in the order specified inside collection, support for which was added in #1833 We have several test verifying that it works correctly. At the same time it only works if the collection type has a order preserving behavior, like List, but something like HashSet may not preserve the order in which items are added. #9067 tracks ordered collection which is about ability to preserve the index of item in collection while saving them, so when querying them the collection will have items in same order (this is not the order specified by OrderBy but rather ordering in which user initially populated collection.) The question still remains, we need runnable repro code to see what you are seeing otherwise we cannot investigate what is going wrong here. |
I don't have way to share my whole repo publically, is there a way I can show you without showing my client's entire project publically? Here is the offending code - I have tried many, many, combinations (a Weeks worth!!) - so this is what my latest looks like which starts off with a Many-to-Many. I've tried it with one side of the MTM navigation too. i.e. NavArea or NavBlock as starting point. All the same results, include/theninclude deeper in the chain will not play well:
Please let me know if need anything else. I really won't be able to make a whole repo just to show that this is broken. Please read my prior post, you can ignore the references to #9067 and not get hung up on semantics. Two people (me and Biz) are telling you the same thing, I explained where to look in my previous post. Not sure how much more I can do to help the MS EF team. Also, I can do a Teams call if you are really interested in fixing this issue. Also, if I am doing something wrong or expecting too much from EF I am open to a plain "it won't work" Thanks Again, for all your efforts! |
@waywardcode I've tried to recreate the issue based on the partial information above, and I've not succeeded; I'm using your original LINQ query above, with a minimal model reconstructed the best I could. The SQL queries I'm seeing are the following: -- This selects the forms, simple
SELECT TOP(2) [f].[Id]
FROM [Forms] AS [f]
WHERE [f].[Id] = @__formId_0
ORDER BY [f].[Id]
-- This selects the sections, simple
SELECT [s].[SectionId], [s].[FormId], [t].[Id]
FROM (
SELECT TOP(1) [f].[Id]
FROM [Forms] AS [f]
WHERE [f].[Id] = @__formId_0
) AS [t]
INNER JOIN [Section] AS [s] ON [t].[Id] = [s].[FormId]
ORDER BY [t].[Id], [s].[SectionId]
-- This selects the rows. Note we first order by Form then Section, and then by the Row's Order
SELECT [t0].[RowId], [t0].[Order], [t0].[SectionId], [t].[Id], [s].[SectionId]
FROM (
SELECT TOP(1) [f].[Id]
FROM [Forms] AS [f]
WHERE [f].[Id] = @__formId_0
) AS [t]
INNER JOIN [Section] AS [s] ON [t].[Id] = [s].[FormId]
INNER JOIN (
SELECT [r].[RowId], [r].[Order], [r].[SectionId]
FROM [Row] AS [r]
) AS [t0] ON [s].[SectionId] = [t0].[SectionId]
ORDER BY [t].[Id], [s].[SectionId], [t0].[Order], [t0].[RowId]
-- This selects the fields. We order by the Form, then the Section, then by the Row's order (same as above), and finally by the field
SELECT [t1].[FieldId], [t1].[FieldTypeId], [t1].[Order], [t1].[RowId], [t1].[FieldTypeId0], [t].[Id], [s].[SectionId], [t0].[RowId]
FROM (
SELECT TOP(1) [f].[Id]
FROM [Forms] AS [f]
WHERE [f].[Id] = @__formId_0
) AS [t]
INNER JOIN [Section] AS [s] ON [t].[Id] = [s].[FormId]
INNER JOIN (
SELECT [r].[RowId], [r].[Order], [r].[SectionId]
FROM [Row] AS [r]
) AS [t0] ON [s].[SectionId] = [t0].[SectionId]
INNER JOIN (
SELECT [f0].[FieldId], [f0].[FieldTypeId], [f0].[Order], [f0].[RowId], [f1].[FieldTypeId] AS [FieldTypeId0]
FROM [Field] AS [f0]
LEFT JOIN [FieldType] AS [f1] ON [f0].[FieldTypeId] = [f1].[FieldTypeId]
) AS [t1] ON [t0].[RowId] = [t1].[RowId]
ORDER BY [t].[Id], [s].[SectionId], [t0].[Order], [t0].[RowId], [t1].[Order] This kind of minimal code sample is what we're looking for when we're asking for a repro - definitely not your entire project. If you can tweak my code sample to reproduce what is failing, that would provide us with something we can investigate. Attempted reproawait using (var ctx = new BlogContext())
{
await ctx.Database.EnsureDeletedAsync();
await ctx.Database.EnsureCreatedAsync();
ctx.Forms.Add(new()
{
Sections = new()
{
new Section
{
Rows = new()
{
new Row
{
Fields = new()
{
new Field { FieldType = new() }
}
}
}
}
}
});
await ctx.SaveChangesAsync();
}
await using (var ctx = new BlogContext())
{
var formId = 1;
_ = await ctx.Forms
.Include(x => x.Sections)
.ThenInclude(x => x.Rows.OrderBy(y => y.Order)) //Nope!
.ThenInclude(x => x.Fields.OrderBy(y => y.Order)) //Nope here too!
.ThenInclude(x => x.FieldType)
.AsSplitQuery()
.SingleOrDefaultAsync(x =>
x.Id == formId);
}
public class BlogContext : DbContext
{
public DbSet<Form> Forms { get; set; }
protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
=> optionsBuilder
.UseSqlServer(@"Server=localhost;Database=test;User=SA;Password=Abcd5678;Connect Timeout=60;ConnectRetryCount=0")
.LogTo(Console.WriteLine, LogLevel.Information)
.EnableSensitiveDataLogging();
}
public class Form
{
public int Id { get; set; }
public List<Section> Sections { get; set; }
}
public class Section
{
public int SectionId { get; set; }
public List<Row> Rows { get; set; }
}
public class Row
{
public int RowId { get; set; }
public int Order { get; set; }
public List<Field> Fields { get; set; }
public Section Section { get; set; }
}
public class Field
{
public int FieldId { get; set; }
public FieldType FieldType { get; set; }
public int Order { get; set; }
}
public class FieldType
{
public int FieldTypeId { get; set; }
} |
@roji and @smitpatel thank you for the assistance - After some looking around and thanks for your code sample, I figured it out. I was using Split Queries in Startup.cs as an option .UseQuerySplittingBehavior This caused the generated ORDER BY to flip the sequence of fields where PK's come first and then OrderBy fields afterwards. After removing this option in I will post here if I find any more details. Here is my tweaked version
using Microsoft.EntityFrameworkCore;
using Microsoft.Extensions.Logging;
using System;
using System.Collections.Generic;
using System.ComponentModel.DataAnnotations;
using System.ComponentModel.DataAnnotations.Schema;
using System.Linq;
using System.Text.Json.Serialization;
await using (var ctx = new NavContext())
{
await ctx.Database.EnsureDeletedAsync();
await ctx.Database.EnsureCreatedAsync();
ctx.AreaToBlockManyToMany.Add(
new AreaToBlockManyToMany
{
NavBlock = new NavBlock
{
NavGroups = new List<NavGroup>
{
new NavGroup {
NavItems = new List<NavItem> { }
}
}
},
NavArea = new NavArea { },
NavBlockSortOrder = 0
});
await ctx.SaveChangesAsync();
}
await using (var ctx = new NavContext())
{
_ = await ctx.AreaToBlockManyToMany
.AsNoTracking()
.OrderBy(ot => ot.AreaId).ThenBy(o => o.NavBlockSortOrder) // this part is working
.Include(a => a.NavArea)
.Include(b => b.NavBlock)
.ThenInclude(g => g.NavGroups.OrderBy(sg => sg.GroupSortOrder)) // This is broken
.ThenInclude(i => i.NavItems.OrderBy(si => si.ItemSortOrder)) // This is broken
// .AsSplitQuery() // see Connection String
.ToListAsync();
}
public class NavContext : DbContext
{
public virtual DbSet<NavBlock> NavBlocks { get; set; }
public virtual DbSet<NavGroup> NavGroups { get; set; }
public virtual DbSet<NavItem> NavItems { get; set; }
public virtual DbSet<NavArea> NavAreas { get; set; }
public virtual DbSet<AreaToBlockManyToMany> AreaToBlockManyToMany { get; set; } // needed for seeding, used to seed from CSV
protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
=>
optionsBuilder
.UseSqlServer(@"Server=(localdb)\MSSQLLocalDB;Database=test;User=SA;Password=Abcd5678;Trusted_Connection=True;Connect Timeout=5;ConnectRetryCount=0",
o => o.UseQuerySplittingBehavior(QuerySplittingBehavior.SplitQuery)) // << --- the problem
.LogTo(Console.WriteLine, LogLevel.Information)
.EnableSensitiveDataLogging();
protected override void OnModelCreating(ModelBuilder modelBuilder)
{
base.OnModelCreating(modelBuilder);
modelBuilder.Entity<NavArea>()
.HasMany(b => b.NavBlocks)
.WithMany(m => m.Areas)
.UsingEntity<AreaToBlockManyToMany>(
j => j
.HasOne(o => o.NavBlock)
.WithMany(m => m.AreaToBlockManyToMany)
.HasForeignKey(fk => fk.NavBlockId),
j => j
.HasOne(o => o.NavArea)
.WithMany(m => m.AreaToBlockManyToMany)
.HasForeignKey(fk => fk.AreaId),
j =>
{
j.HasKey(k => new { k.NavBlockId, k.AreaId });
}
);
}
}
public class NavArea
{
[Key]
public int AreaId { get; set; }
public string AreaName { get; set; }
public int AreaSortOrder { get; set; }
public ICollection<NavBlock> NavBlocks { get; set; }
public List<AreaToBlockManyToMany> AreaToBlockManyToMany { get; set; }
}
public class NavAreaDTO
{
public int AreaId { get; set; }
public string AreaName { get; set; }
public int AreaSortOrder { get; set; }
public List<NavBlock> NavBlocks { get; set; }
}
// join table for Area to Block M2M with Sort field for NavBlock
public class AreaToBlockManyToMany
{
public int AreaId { get; set; }
public NavArea NavArea { get; set; }
public int NavBlockId { get; set; }
public NavBlock NavBlock { get; set; }
public int NavBlockSortOrder { get; set; }
}
public class NavBlock
{
[Key]
public int NavBlockId { get; set; }
public string BlockName { get; set; }
public string BlockStyle { get; set; }
public int BlockSortOrder { get; set; }
public bool IsActive { get; set; }
[NotMapped]
public List<int> Areas0 { get; set; }
[JsonIgnore]
public ICollection<NavArea> Areas { get; set; }
public List<AreaToBlockManyToMany> AreaToBlockManyToMany { get; set; }
public List<NavGroup> NavGroups { get; set; }
}
public class NavGroup
{
[Key]
public int NavGroupId { get; set; }
public string NavGroupTitle { get; set; }
public bool Root { get; set; }
public string Link { get; set; }
public int GroupSortOrder { get; set; }
// relationships
public int NavBlockId { get; set; }
public NavBlock NavBlock { get; set; }
public List<NavItem> NavItems { get; set; }
}
public class NavItem
{
[Key]
public int NavItemId { get; set; }
public string ItemName { get; set; }
public string Link { get; set; }
public int ItemSortOrder { get; set; }
public string ImageUrl { get; set; }
// relationships
public int NavGroupId { get; set; }
public NavGroup NavGroup { get; set; }
}
|
There is no difference between specifying split query on a specific query (with Looking at the tweaked code sample, thing still look OK for me. Here's the last of the split queries: SELECT [t0].[NavItemId], [t0].[ImageUrl], [t0].[ItemName], [t0].[ItemSortOrder], [t0].[Link], [t0].[NavGroupId], [a].[NavBlockId], [a].[AreaId], [n].[AreaId], [n0].[NavBlockId], [t].[NavGroupId]
FROM [AreaToBlockManyToMany] AS [a]
INNER JOIN [NavAreas] AS [n] ON [a].[AreaId] = [n].[AreaId]
INNER JOIN [NavBlocks] AS [n0] ON [a].[NavBlockId] = [n0].[NavBlockId]
INNER JOIN (
SELECT [n1].[NavGroupId], [n1].[GroupSortOrder], [n1].[NavBlockId]
FROM [NavGroups] AS [n1]
) AS [t] ON [n0].[NavBlockId] = [t].[NavBlockId]
INNER JOIN (
SELECT [n2].[NavItemId], [n2].[ImageUrl], [n2].[ItemName], [n2].[ItemSortOrder], [n2].[Link], [n2].[NavGroupId]
FROM [NavItems] AS [n2]
) AS [t0] ON [t].[NavGroupId] = [t0].[NavGroupId]
ORDER BY [a].[AreaId], [a].[NavBlockSortOrder], [a].[NavBlockId], [n].[AreaId], [n0].[NavBlockId], [t].[GroupSortOrder], [t].[NavGroupId], [t0].[ItemSortOrder] This query loads the NavItems (aliased to I think the missing piece here may be that NavItems are returned sorted by ItemSortOrder, but within their specific NavItem; EF Core doesn't load just a list of NavItems without context - it populates them as lists within their NavGroups. |
I know there is no difference except for global. :) I was pointing out that with split queries active, // Split Query OFF
SELECT [a].[NavBlockId], [a].[AreaId], [a].[NavBlockSortOrder], [n].[AreaId], [n].[AreaName], [n].[AreaSortOrder], [n0].[NavBlockId], [n0].[BlockName], [n0].[BlockSortOrder], [n0].[BlockStyle], [n0].[IsActive], [t0].[NavGroupId], [t0].[GroupSortOrder], [t0].[Link], [t0].[NavBlockId], [t0].[NavGroupTitle], [t0].[Root], [t0].[NavItemId], [t0].[ImageUrl], [t0].[ItemName], [t0].[ItemSortOrder], [t0].[Link0], [t0].[NavGroupId0]
FROM [AreaToBlockManyToMany] AS [a]
INNER JOIN [NavAreas] AS [n] ON [a].[AreaId] = [n].[AreaId]
INNER JOIN [NavBlocks] AS [n0] ON [a].[NavBlockId] = [n0].[NavBlockId]
LEFT JOIN (
SELECT [n1].[NavGroupId], [n1].[GroupSortOrder], [n1].[Link], [n1].[NavBlockId], [n1].[NavGroupTitle], [n1].[Root], [t].[NavItemId], [t].[ImageUrl], [t].[ItemName], [t].[ItemSortOrder], [t].[Link] AS [Link0], [t].[NavGroupId] AS [NavGroupId0]
FROM [NavGroups] AS [n1]
LEFT JOIN (
SELECT [n2].[NavItemId], [n2].[ImageUrl], [n2].[ItemName], [n2].[ItemSortOrder], [n2].[Link], [n2].[NavGroupId]
FROM [NavItems] AS [n2]
) AS [t] ON [n1].[NavGroupId] = [t].[NavGroupId]
) AS [t0] ON [n0].[NavBlockId] = [t0].[NavBlockId]
ORDER BY [a].[AreaId], [a].[NavBlockSortOrder], [a].[NavBlockId], [n].[AreaId], [n0].[NavBlockId], [t0].[GroupSortOrder], [t0].[NavGroupId], [t0].[ItemSortOrder], [t0].[NavItemId]
// Split Query ON
// With .UseQuerySplittingBehavior or .AsSplitQuery()
//SPLIT 1
SELECT [a].[NavBlockId], [a].[AreaId], [a].[NavBlockSortOrder], [n].[AreaId], [n].[AreaName], [n].[AreaSortOrder], [n0].[NavBlockId], [n0].[BlockName], [n0].[BlockSortOrder], [n0].[BlockStyle], [n0].[IsActive]
FROM [AreaToBlockManyToMany] AS [a]
INNER JOIN [NavAreas] AS [n] ON [a].[AreaId] = [n].[AreaId]
INNER JOIN [NavBlocks] AS [n0] ON [a].[NavBlockId] = [n0].[NavBlockId]
ORDER BY [a].[AreaId], [a].[NavBlockSortOrder], [a].[NavBlockId], [n].[AreaId], [n0].[NavBlockId]
//SPLIT 2
SELECT [t0].[NavGroupId], [t0].[GroupSortOrder], [t0].[Link], [t0].[NavBlockId], [t0].[NavGroupTitle], [t0].[Root], [a].[NavBlockId], [a].[AreaId], [n].[AreaId], [n0].[NavBlockId]
FROM [AreaToBlockManyToMany] AS [a]
INNER JOIN [NavAreas] AS [n] ON [a].[AreaId] = [n].[AreaId]
INNER JOIN [NavBlocks] AS [n0] ON [a].[NavBlockId] = [n0].[NavBlockId]
INNER JOIN (
SELECT [t].[NavGroupId], [t].[GroupSortOrder], [t].[Link], [t].[NavBlockId], [t].[NavGroupTitle], [t].[Root]
FROM (
SELECT [n1].[NavGroupId], [n1].[GroupSortOrder], [n1].[Link], [n1].[NavBlockId], [n1].[NavGroupTitle], [n1].[Root], ROW_NUMBER() OVER(PARTITION BY [n1].[NavBlockId] ORDER BY [n1].[GroupSortOrder]) AS [row]
FROM [NavGroups] AS [n1]
) AS [t]
WHERE 0 < [t].[row]
) AS [t0] ON [n0].[NavBlockId] = [t0].[NavBlockId]
ORDER BY [a].[AreaId], [a].[NavBlockSortOrder], [a].[NavBlockId], [n].[AreaId], [n0].[NavBlockId], [t0].[NavGroupId], [t0].[NavBlockId], [t0].[GroupSortOrder]
//SPLIT 3
SELECT [t2].[NavItemId], [t2].[ImageUrl], [t2].[ItemName], [t2].[ItemSortOrder], [t2].[Link], [t2].[NavGroupId], [a].[NavBlockId], [a].[AreaId], [n].[AreaId], [n0].[NavBlockId], [t0].[NavGroupId]
FROM [AreaToBlockManyToMany] AS [a]
INNER JOIN [NavAreas] AS [n] ON [a].[AreaId] = [n].[AreaId]
INNER JOIN [NavBlocks] AS [n0] ON [a].[NavBlockId] = [n0].[NavBlockId]
INNER JOIN (
SELECT [t].[NavGroupId], [t].[GroupSortOrder], [t].[NavBlockId]
FROM (
SELECT [n1].[NavGroupId], [n1].[GroupSortOrder], [n1].[NavBlockId], ROW_NUMBER() OVER(PARTITION BY [n1].[NavBlockId] ORDER BY [n1].[GroupSortOrder]) AS [row]
FROM [NavGroups] AS [n1]
) AS [t]
WHERE 0 < [t].[row]
) AS [t0] ON [n0].[NavBlockId] = [t0].[NavBlockId]
INNER JOIN (
SELECT [t1].[NavItemId], [t1].[ImageUrl], [t1].[ItemName], [t1].[ItemSortOrder], [t1].[Link], [t1].[NavGroupId]
FROM (
SELECT [n2].[NavItemId], [n2].[ImageUrl], [n2].[ItemName], [n2].[ItemSortOrder], [n2].[Link], [n2].[NavGroupId], ROW_NUMBER() OVER(PARTITION BY [n2].[NavGroupId] ORDER BY [n2].[ItemSortOrder]) AS [row]
FROM [NavItems] AS [n2]
) AS [t1]
WHERE 0 < [t1].[row]
) AS [t2] ON [t0].[NavGroupId] = [t2].[NavGroupId]
ORDER BY [a].[AreaId], [a].[NavBlockSortOrder], [a].[NavBlockId], [t0].[NavBlockId], [t0].[GroupSortOrder], [t2].[NavGroupId], [t2].[ItemSortOrder]
So currently things are working fine with split queries OFF in my production with tables populated, however if I turn Split Queries ON - the ORDER BY inside Include / ThenInclude is not working as it should. Might have something to do with the multiple queries generated ??? I couldn't say. If you feel this needs further investigation I can assist in anyway I can, otherwise we can close this. As a Side Note: This project is a CMS based on .Net Core backend / React frontend - was going to Open Source it at some point. |
Issue only happens when having 2 ordered includes chained through ThenInclude. The middle split query has incorrect ordering of OrderBy terms. Need to see what is generated SQL in current bits. |
According to regression test Filtered_include_after_different_filtered_include_different_level the issue seems to be fixed in 6.0 release. Verified that the test had incorrect ordering as mentioned above in 5.0 codebase. |
Yeah, just to confirm that my test above was on 6.0... |
Fixed in #24706 |
Has there been any progress with this? Ordering navigation properties still does not translate into the expected ordering in EFC5.0.5
If there is no intention of ever fixing this, it might be best to show a warning when an attempt to order a navigation property is made to avoid confusion.
Originally posted by @Bitz in #9067 (comment)
Example output on ORDERBY inside ThenInclude from EF
ORDER BY [a].[AreaId], [a].[NavBlockSortOrder], [a].[NavBlockId], [n].[AreaId], [n0].[NavBlockId], [t0].[NavGroupId], [t0].[NavBlockId], [t0].[SortOrder]
Any solution to this, hidden unknown dark magic or otherwise? :)
The text was updated successfully, but these errors were encountered: