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

Unwanted materialization of intermediate query in some cases #13900

Closed
ac10n opened this issue Nov 6, 2018 · 2 comments
Closed

Unwanted materialization of intermediate query in some cases #13900

ac10n opened this issue Nov 6, 2018 · 2 comments
Assignees
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported type-enhancement
Milestone

Comments

@ac10n
Copy link

ac10n commented Nov 6, 2018

In the code below, if we uncomment the line

 .Where(x => x.LastOrder != null)

We'll see materialization of LastOrders. Even worse, that happens in a select N+1 manner, one by one in queries like:

exec sp_executesql N'SELECT TOP(1) [o].[Id], [o].[Date], [o].[PersonId], [o].[Price]
FROM [Orders] AS [o]
WHERE @_outer_Id = [o].[PersonId]
ORDER BY [o].[Date] DESC',N'@_outer_Id int',@_outer_Id=1

Steps to reproduce

using Microsoft.EntityFrameworkCore;
using System;
using System.Collections.Generic;
using System.Linq;

namespace EfCoreBugs
{
  class Program
  {
    static void Main(string[] args)
    {
      using (var dbContext = new MyDbContext())
      {
        var data = dbContext.People
          .Select(x => new
          {
            Person = x,
            LastOrder = x.Orders.OrderByDescending(o => o.Date).FirstOrDefault()
          })
          //.Where(x => x.LastOrder != null)
          .Select(x => new
          {
            x.Person.Name,
            x.LastOrder.Price,
            x.LastOrder.Date
          }).ToList();
        Console.WriteLine(data.Count);
      }
      Console.ReadLine();
    }

    public class MyDbContext : DbContext
    {
      protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
      {
        optionsBuilder.UseSqlServer(@"Server=.;Database=EfCoreBugs;Trusted_Connection=True;MultipleActiveResultSets=True;");
      }
      public DbSet<Person> People { get; set; }
      public DbSet<Order> Orders { get; set; }
    }

    public class Person
    {
      public int Id { get; set; }
      public string Name { get; set; }

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

    public class Order
    {
      public int Id { get; set; }
      public DateTime Date { get; set; }
      public decimal Price { get; set; }

      public int PersonId { get; set; }
      public Person Person { get; set; }
    }
  }
}

Further technical details

EF Core version: 2.1.4
Database Provider: Microsoft.EntityFrameworkCore.SqlServer
Operating system: Win10
IDE: Vs2017 15.8.9

@smitpatel
Copy link
Contributor

#10001 Would help this scenario to be condensed in single query rather than N+1.
That being said, 3.0 query changes may make the materialization unnecessary and reduces the columns brought back.

@maumar
Copy link
Contributor

maumar commented May 22, 2019

This works in the new pipeline. We simplify the intermediate projection and access the properties directly. Also entity we convert the x.LastOrder != null to key equality.

equivalent query on northwind:

ctx.Customers
                    .Select(c => new
                    {
                        Person = c,
                        LastOrder = c.Orders.OrderByDescending(o => o.OrderDate).FirstOrDefault()
                    }
                    ).Where(x => x.LastOrder != null)
                    .Select(x => new { x.Person.CustomerID, x.LastOrder.OrderID })

produces the following sql:

SELECT [c].[CustomerID], (SELECT TOP(1) [o].[OrderID]
    FROM [Orders] AS [o]
    WHERE [c].[CustomerID] = [o].[CustomerID]
    ORDER BY [o].[OrderDate] DESC)
FROM [Customers] AS [c]
WHERE (SELECT TOP(1) [o0].[OrderID]
    FROM [Orders] AS [o0]
    WHERE [c].[CustomerID] = [o0].[CustomerID]
    ORDER BY [o0].[OrderDate] DESC) IS NOT NULL

@smitpatel smitpatel added the verify-fixed This issue is likely fixed in new query pipeline. label Jun 13, 2019
@maumar maumar added closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. and removed verify-fixed This issue is likely fixed in new query pipeline. labels Jun 20, 2019
@maumar maumar closed this as completed Jun 20, 2019
@ajcvickers ajcvickers modified the milestones: 3.0.0, 3.0.0-preview7 Jul 2, 2019
smitpatel added a commit that referenced this issue Jul 11, 2019
@ajcvickers ajcvickers modified the milestones: 3.0.0-preview7, 3.0.0 Nov 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported type-enhancement
Projects
None yet
Development

No branches or pull requests

4 participants