Correlated subquery (N+1 fix) missing ORDER BY clause inside nested query

[simeyla]

I am trying to do a simple query to get a list of Club rows with associated customer Order rows. (This is for a subscription model where a customer has a Club membership).

With the new EFCore 2.1 functionality only two queries are generated to avoid N+1 issues. I get back the correct results, until I add OrderByDescending in which case no data is returned for the 'child' query.

It is clear what is happening by looking at the generated SQL. The first query here gets the top @_p_0 clubs, and the second query gets the corresponding orders.

  SELECT TOP(@__p_0) [c].[ClubId] AS [ClubId], [c].[NextShipmentDt]
  FROM [Club] AS [c]
  ORDER BY [c].[NextShipmentDt] DESC, [c].[ClubId]


  SELECT [t].[NextShipmentDt], [t].[ClubId], 
		 [c.OrderNavigation].[CompletedOrderId] AS [OrderID], 
		 [c.OrderNavigation].[OrderDt] AS [OrderDate], 
		 [c.OrderNavigation].[ClubId]
  FROM [Order] AS [c.OrderNavigation]
  INNER JOIN (
      SELECT TOP(@__p_0) [c0].[NextShipmentDt], [c0].[ClubId]
      FROM [Club] AS [c0]
  ) AS [t] ON [c.OrderNavigation].[ClubId] = [t].[ClubId]
  ORDER BY [t].[NextShipmentDt] DESC, [t].[ClubId]

This is great SQL except for one thing! The ORDER BY [t].[NextShipmentDt] DESC should be INSIDE the INNER query because it uses SELECT TOP. This results in completely the wrong orders being returned for the club memberships. Then EFCore fails to match them up and returns me zero results.

Expected correct SQL (one line added)

  SELECT [t].[NextShipmentDt], [t].[ClubId], 
		 [c.OrderNavigation].[CompletedOrderId] AS [OrderID], 
		 [c.OrderNavigation].[OrderDt] AS [OrderDate], 
		 [c.OrderNavigation].[ClubId]
  FROM [Order] AS [c.OrderNavigation]
  INNER JOIN (
      SELECT TOP(@__p_0) [c0].[NextShipmentDt], [c0].[ClubId]
      FROM [Club] AS [c0]
      ORDER BY [t].[NextShipmentDt] DESC
  ) AS [t] ON [c.OrderNavigation].[ClubId] = [t].[ClubId]
  ORDER BY [t].[NextShipmentDt] DESC, [t].[ClubId]

Steps to reproduce

The code is very simple, uses ToList() to force the new behavior as described in the release notes. I was able to repeat this result with any query containing OrderByDescending.

var clubsAndOrders = _context.Clubs
                    .Select(c => new
                    {
                        ClubID = c.ID,
                        NextShipment = c.NextShipmentDt,

                        Orders = c.OrderNavigation.Select(o => new
                        {
                            OrderID = o.OrderId,
                            ShippingAddress = o.Address

                        }).ToList()
                    })
                    .OrderByDescending(x => x.NextShipmentDt)
                    .Take(20)
                    .ToList();

(This code differs slightly from the code to generate the SQL, but the structure is the same).

OrderNavigation is defined in the Club entity as:

    public ICollection<Order> OrderNavigation { get; set; }

with the following constraint:

            entity.HasOne(d => d.Club)
                .WithMany(p => p.OrderNavigation)
                .HasForeignKey(d => d.ClubId)
                .HasConstraintName("FK_Order_Club");

Further technical details

EF Core version: Entity Framework Core 2.1.0-preview1-28290
Database Provider: Microsoft.EntityFrameworkCore.SqlServer
Operating system: Win 10
IDE: Visual Studio 2017 15.6.0 Preview 7

[maumar]

this is already fixed in d787c13

[maumar]

we produce the following SQL now:

SELECT TOP(@__p_0) [c].[ID] AS [ClubID], [c].[NextShipmentDt] AS [NextShipment]
FROM [Clubs] AS [c]
ORDER BY [NextShipment] DESC, [c].[ID]

SELECT [t].[NextShipmentDt], [t].[ID], [c.OrderNavigation].[OrderId] AS [OrderID], [c.OrderNavigation].[Address] AS [ShippingAddress], [c.OrderNavigation].[ClubID]
FROM [Orders] AS [c.OrderNavigation]
INNER JOIN (
	SELECT TOP(@__p_0) [c0].[NextShipmentDt], [c0].[ID]
	FROM [Clubs] AS [c0]
	ORDER BY [c0].[NextShipmentDt] DESC, [c0].[ID]
) AS [t] ON [c.OrderNavigation].[ClubID] = [t].[ID]
ORDER BY [t].[NextShipmentDt] DESC, [t].[ID]

[simeyla]

@maumar Thanks for taking the time to provide the code. Tried to search for this issue because I figured it was more than likely already in the pipeline, but wanted to make sure :-)