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

Global query filter generates SQL with unnecessary columns in projections #20758

Closed
DanielBlazevic opened this issue Apr 27, 2020 · 10 comments
Closed
Assignees
Labels
area-perf area-query 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

@DanielBlazevic
Copy link

DanielBlazevic commented Apr 27, 2020

Steps to reproduce

When using projection without global query filter, generated SQL is as expected. Only two columns are selected in this example:

_ = context.Projects.Select(x => new { ProjectName = x.Name, CityName = x.City.Name }).ToList();

And generated SQL is as expected:

SELECT [p].[Name] AS [ProjectName], [c].[Name] AS [CityName]
FROM [pm].[Project] AS [p]
LEFT JOIN [City] AS [c] ON [p].[CityId] = [c].[Id]

If I add global query filter:

partial void OnModelCreatingPartial(ModelBuilder modelBuilder)
{
    modelBuilder.Entity<Project>().HasQueryFilter(p => p.IsDeleted == false);
    modelBuilder.Entity<City>().HasQueryFilter(p => p.IsDeleted == false);
}

Generated SQL looks like this:

SELECT [p].[Name] AS [ProjectName], [t].[Name] AS [CityName]
FROM [pm].[Project] AS [p]
LEFT JOIN (
          SELECT [c].[Id], [c].[CountryEntityId], [c].[CountyId], [c].[CreateDate], [c].[CreateUser], [c].[IsActive], [c].[IsDeleted], [c].[LastEditDate], [c].[LastEditUser], [c].[Name], [c].[StateId]
          FROM [City] AS [c]
          WHERE [c].[IsDeleted] = CAST(0 AS bit)
      ) AS [t] ON [p].[CityId] = [t].[Id]
      WHERE [p].[IsDeleted] = CAST(0 AS bit)

So, my questions are:

  • why are those extra columns included in LEFT JOIN when they are not needed in projection?
  • is this expected behavior of EF Core ?
  • what can I do to evade those extra columns ?

I am asking because I measured performance degradation of complex queries with global query filter enabled and unnecessary columns in left joins.

Further technical details

EF Core version: 3.1.3
Database provider: Microsoft.EntityFrameworkCore.SqlServer
Target framework: .NET Core 3.1
Operating system: Windows 10
IDE: Visual Studio 2019 16.5.4

@roji
Copy link
Member

roji commented Apr 27, 2020

I am asking because I measured performance degradation of complex queries with global query filter enabled and unnecessary columns in left joins.

Are you saying you're seeing a perf difference between those two specific SQL queries, i.e. with and without the added columns in the inner query? If so, can you share the benchmark and results?

Even if there's no perf issue here (which is what I'd expect), we could still generate tighter SQL (but that would be a lower priority).

@DanielBlazevic
Copy link
Author

After retesting with other tools, performance of two queries is similar. Difference is less than 2%. Sorry about this, I obviously used wrong tool for benchmark.

But tighter SQL would be nice, especially for tables with lot of columns.

@roji
Copy link
Member

roji commented Apr 28, 2020

@DanielBlazevic thanks for confirming. Am reopening the issue and putting in the backlog as an SQL improvement issue (low priority).

@stevendarby
Copy link
Contributor

stevendarby commented Oct 7, 2020

Just to add a data point to this...

I have a query which selects from 10 tables and produces a SQL query 4,167 characters long.

If I add a common global filter on each of the tables, the total query is 23,375 characters. The filters themselves only add about 90 characters per table, 900 in total, so most of that is coming from the long lists of columns that are now being selected in sub-queries.

I understand it's not really a perf issue, but it does make reviewing queries that little bit more difficult and I just wanted to give a "real world" example of how big the difference can be for when this gets reviewed again.

@stevendarby
Copy link
Contributor

Another little annoyance to add- Application Insights seems to limit the logged SQL to 8kb, so with the above example it's the difference between seeing the full query and only seeing a third of it.

@stevendarby
Copy link
Contributor

stevendarby commented Aug 16, 2021

So, finally upgraded a big solution to EF Core 5.0 and... this is fixed? Sub-queries now only select the columns that are used in the final projection or as part of the join. Great news.

Quick example with the BloggingContext, with some filters:

modelBuilder.Entity<Blog>().HasQueryFilter(x => x.Rating > 4);
modelBuilder.Entity<Post>().HasQueryFilter(x => x.Title.StartsWith("The"));

and this query:

_ = db.Posts.Select(x => new
    {
        x.Blog.Url,
        x.Content
    }).FirstOrDefault();

5.0:

 SELECT TOP(1) [t].[Url], [p].[Content]
 FROM [Posts] AS [p]
 INNER JOIN (
     SELECT [b].[BlogId], [b].[Url]
     FROM [Blogs] AS [b]
     WHERE [b].[Rating] > 4
 ) AS [t] ON [p].[BlogId] = [t].[BlogId]
 WHERE [p].[Title] IS NOT NULL AND ([p].[Title] LIKE N'The%')

3.1:

SELECT TOP(1) [t].[Url], [p].[Content]
FROM [Posts] AS [p]
INNER JOIN (
    SELECT [b].[BlogId], [b].[Rating], [b].[Url]
    FROM [Blogs] AS [b]
    WHERE [b].[Rating] > 4
) AS [t] ON [p].[BlogId] = [t].[BlogId]
WHERE [p].[Title] IS NOT NULL AND ([p].[Title] LIKE N'The%')

3.1 selected Rating in the subquery. The difference is small in this example, but on a big table, 3.1 selecting all columns made it very verbose and harder to read.

The issue of whether it should do a subquery at all is another issue imo. It has been mentioned in other places that filtering in a subquery may be perf beneficial compared to a simple join with the filters on the outer where. If this issue is just about selecting "unnecessary columns", as per the title, then it's fixed for me!

@smitpatel
Copy link
Member

Fixed in #21992

Thanks @stevendarby for testing out.

@smitpatel smitpatel self-assigned this Aug 16, 2021
@smitpatel smitpatel modified the milestones: Backlog, 5.0.0 Aug 16, 2021
@smitpatel smitpatel added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Aug 16, 2021
@stevendarby
Copy link
Contributor

@smitpatel I noticed an exception to this where unused columns are still selected when it's part of a set operation that produces a cross apply. Not sure of the exact conditions for it but I have a repro.

Obviously this is a fairly minor issue, mostly about tighter/nicer looking SQL, but would be nice to have.

using Microsoft.EntityFrameworkCore;
using Microsoft.Extensions.Logging;
using System.Collections.Generic;
using System.Diagnostics;
using System.Linq;

{
    using var context = new MyContext();

    context.Database.EnsureCreated();

    // Only necessary columns are selected in sub query on Images
    _ = context.Posts
        .Select(x => new { Type = "Image", Detail = x.Image.Title })
        .ToList();

    // All columns are selected in sub query on Images
    _ = context.Blogs
        .SelectMany(
            blog => blog.Posts.Select(x => new { Type = "Post", Detail = x.Title })
                .Concat(blog.Posts.Select(x => new { Type = "Image", Detail = x.Image.Title })),
            (blog, detail) =>
                new BlogDetailsDto
                {
                    BlogName = blog.Name,
                    Type = detail.Type,
                    Detail = detail.Detail
                })
        .ToList();
}

public class MyContext : DbContext
{
    public DbSet<Blog> Blogs { get; set; }
    public DbSet<Post> Posts { get; set; }

    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
        => optionsBuilder
            .UseSqlServer("Server=.;Database=Join;Trusted_Connection=True;")
            .LogTo(x => Debug.WriteLine(x), LogLevel.Information);

    protected override void OnModelCreating(ModelBuilder modelBuilder)
        => modelBuilder.Entity<Image>().HasQueryFilter(x => !x.IsDeleted);
}

public class BlogDetailsDto
{
    public string BlogName { get; set; }
    public string Type { get; set; }
    public string Detail { get; set; }
}

public class Blog
{
    public int Id { get; set; }
    public string Name { get; set; }
    public ICollection<Post> Posts { get; set; }
}

public class Post
{
    public int Id { get; set; }
    public string Title { get; set; }
    public string Content { get; set; }
    public int BlogId { get; set; }
    public int ImageId { get; set; }
    public Blog Blog { get; set; }
    public Image Image { get; set; }
}

public class Image
{
    public int Id { get; set; }
    public string Title { get; set; }
    public string Url { get; set; }
    public bool IsDeleted { get; set; }
    public Post Post { get; set; }
}

Second query SQL:

      SELECT [b].[Name] AS [BlogName], [t].[Type], [t].[Detail]
      FROM [Blogs] AS [b]
      CROSS APPLY (
          SELECT N'Post' AS [Type], [p].[Title] AS [Detail]
          FROM [Posts] AS [p]
          WHERE [b].[Id] = [p].[BlogId]
          UNION ALL
          SELECT N'Image' AS [Type], [t0].[Title] AS [Detail]
          FROM [Posts] AS [p0]
          INNER JOIN (
              SELECT [i].[Id], [i].[IsDeleted], [i].[Title], [i].[Url] -- Here
              FROM [Image] AS [i]
              WHERE [i].[IsDeleted] = CAST(0 AS bit)
          ) AS [t0] ON [p0].[ImageId] = [t0].[Id]
          WHERE [b].[Id] = [p0].[BlogId]
      ) AS [t]

@smitpatel
Copy link
Member

Pruning not being applied to set operations was fixed in #28142

@stevendarby
Copy link
Contributor

Oh great, thank you and apologies for not checking more recent builds.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-perf area-query 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

5 participants