Efcore: Query Perf: Selecting entire table for navigation properties in projections

Created on 13 Jun 2016  路  19Comments  路  Source: dotnet/efcore

Steps to reproduce

public class Country
{        
    public string CountryCode { get; set; }
    public string CountryName { get; set; }
    public virtual ICollection<State> States { get; set; }
}

public class State 
{
    public string StateCode { get; set; }
    public string StateName { get; set; }
    public string CountryCode { get; set; }       
    public Country Country { get; set; }
}

Mapping on Country

builder.ToTable("Country");
builder.HasKey(pr => pr.CountryCode);
builder.HasMany(m => m.States).WithOne(i => i.Country).HasForeignKey(m => m.CountryCode);

Mapping on State

builder.ToTable("State");
builder.HasKey(pr => pr.StateCode);
builder.HasOne(m => m.Country).WithMany(m => m.States).HasForeignKey(m => m.CountryCode);

Now the Query

var query = _context.Countries
                    .Where(i => i.CountryCode == "USA")
                    .Select(m => new
                                  {
                                       m.CountryName,
                                       m.CountryCode,
                                       States = m.States.Select(x => new
                                                         {
                                                            x.StateCode,
                                                            x.StateName,
                                                            x.CountryCode
                                                         })
                                  }).AsQueryable();
  return query.ToList();

The issue

When I run SQL Profiler on this query I get following

SELECT [i].[CountryName], [i].[CountryCode]
FROM [Country] AS [i]
WHERE [i].[CountryCode] = N'USA'

and

SELECT [x].[CountryCode], [x].[StateCode], [x].[StateName]
FROM [State] AS [x]

I understand the two queries are not combined as in previous versions of EntityFramework. However, here the query on State is just select all without any where condition from the Country it is looking for. It does gives me States from 'USA' but it is doing so in the memory, not the SQL server.

Now make the query to include about 5 countries, it gives me 5 of these queries without any condition for Country

SELECT [x].[CountryCode], [x].[StateCode], [x].[StateName]
FROM [State] AS [x]

Further technical details

EF Core version: 1.0.0-rc2-final
Operating system: Windows 10
SQL Server 2008
Visual Studio version: VS 2015 update 2
Framework Target: net461

area-perf closed-fixed type-bug

Most helpful comment

Don't get me wrong, I really appreciate the team's hard work and all.
But EF might get the RTM title in couple of days, but it's not there yet.
A trivial query resulting in a full table retrieval, that's not RTM.

Many things have changed to the best in Microsoft in the last couple of years, including the open source and multi platform change, but "version 1 isn't reliable - wait for SP1" hasn't.
馃槩

All 19 comments

@anpete @maumar is this already tracked by another issue?

Is this a bug or work in progress?

Closing as a dupe

Reopening as we are going to track the lack of filtering issue separately from the n+1 optimization.

cc @maumar

Don't get me wrong, I really appreciate the team's hard work and all.
But EF might get the RTM title in couple of days, but it's not there yet.
A trivial query resulting in a full table retrieval, that's not RTM.

Many things have changed to the best in Microsoft in the last couple of years, including the open source and multi platform change, but "version 1 isn't reliable - wait for SP1" hasn't.
馃槩

@gdoron your feedback is fair, LINQ providers take time and EF Core is no exception. There are definitely some significant limitations in the initial RTM, but we are working to address these and I think it will mature quickly from this point on. Most of what we are doing is bug fixing now, though there are some notable things still to be implemented (such as GroupBy transalation). We've tried to be very transparent about what to expect when using EF Core https://docs.efproject.net/en/latest/efcore-vs-ef6/index.html.

I do think that v1 software in general tend to be less mature - not just from Microsoft. Historically this was amplified by our slow release cadence, think back to .NET being >1 year between releases. I'm hopeful that folks that are adopting new technology will be able to keep up-to-date as we get some releases out quickly that address the top issues folks are hitting.

@rowanmiller I have no complains about EF core not being as good as EF 6, it's obvious that a rewritten software, specially in EF's size will have many bugs in its early development, it's a temporary condition and I believe everyone is aware of this and accept it.

I was arguing that EF core was rushed and released too early, before it's ready.
People will use an RTM product in production, and they will surely get hit by the many bugs that EF core still has, their entire website can easily crash because of this bug for example.

It seems that the only reason EF core is being released now is to follow the same schedule as the rest of .NET Core and ASP.NET Core.

Each brick should be independent, just like WebSockets and SignalR that were left behind for future releases.
This is what I meant in this coment.

As a side note, as a consumer of the library, it's by far more important for us that EF Core's core features will work as they should, with good performance and reliability than having another nice to have feature (like scaffolding for instance).

With great appreciation to you and your team,
Doron

@gdoron I have to fundamentally disagree with you. Rowan is on point. Yes, it's ideal if enough people use your betas that v1.0.0 is flawless, but that may not be realistic. It's far worse if you get paralyzed into needing it to be perfect before you ship.

Version 1 Sucks, But Ship It Anyway is a great read by the co-founder of Stack Exchange.

Also, If you aren't embarrassed by v1.0 you didn't release it early enough.

@jnm2 I read Jeff's blog post briefly but I believe I got the general idea.
I agree with concept, but EF doesn't suck, it's broken.

There's no reason to ship version 1 when you are already fully aware of the huge issues your software has.

If your version 1 will take servers and applications down because of N+1 queries and full tables(with a capital S) retrieval in a single query (as I painfully experienced), and you're aware of it, DO NOT SHIP IT.

There's a huge difference between being perfect and being reliable.
If your product is a critical one, you can ship with subset of features, but they do need to be reliable.

I guess you wouldn't want your bank's R&D department to read that blog and say, awesome idea! so lets go live with "version 1 sucks" and see what the feedback will be.

I am very disappointed with EF Core and will move our company to use Dapper or EF 6.1 if things won't get fixed very soon.

I think EF somewhere on the road made a huge mistake at prioritizing the features (maybe because of someone else making a terrible decision and changing the schedule and roadmap midway, as I believe happened).
You can have

  • Shadow properties (that I have no idea what are they good for).
  • Reverse engineer an existing DB etc'
  • Etc.

but off top of my head, issues that we suffer from:

  • Async operations are tremendously slower than the sync API - it was improved by 70% in matter of minutes by @anpete, kudos for that! but it should have happened much earlier, I'm sure EF team have benchamrks so they knew about it (if they don't, that is...).
  • Simple queries return the entire table and evaluate on the client.
  • Simple queries are translated to N+1 queries.
  • Join queries are translated to multiple queries.
  • Views are not supported.
  • SP are not supported.
  • Raw SQL that doesn't return a single track entity isn't supported.
  • Many limitations with Many to Many (not sure if they are known or not).

So there are many issues, but there isn't even a way of overcome them with RawSql.

The team, as you can see in the issues and the pull requests, are working hard and being transparent with the community. But they must prioritize the basic features of ORM first.

But they must prioritize the basic features of ORM first.

But that's your list. I'd never use views and SPs directly with EF. I think many to many without a join entity is a bad idea personally. Shadow properties are just as much a basic feature of an ORM as view and SP support, if not more fundamentally so. I can hardly believe we're finally getting them! The only bugs that would affect my company are the perf ones which is why I'm holding out for 1.0.1.

EF Core is not broken. For some people, EF Core is working perfectly. The EF team made it clear that EF Core will not be at parity with EF 6 for a long time, so everything is up for grabs. I experimentally moved our stuff to EF Core so I could provide feedback, but ultimately the onus is on _you_ to verify whether taking the dive in production is a good idea. Every RTM release will have bugs and if servers went down because you didn't spend enough time testing your use cases before going to production that is _your_ responsibility. This is the case with every library.

If EF Core's feature priorities don't match your priorities and you need your EF6-ish features, that is why they clearly say that you should remain on EF6. All the information that you needed to make a good judgment call was available.

Realocating to 1.1.0 as the fix for this was pretty high risk.

fixed in https://github.com/aspnet/EntityFramework/pull/6022, merged to dev in 06693727e15e48ed62695d89fa703a4d6a8fe813

@rowanmiller Thanks for the fix and the update!
Is there an estimation on when 1.1.0 will be released?
The roadmap hasn't been updated for a month.

I used 1.1.0-alpha1-22107 and see no change in the query.

@ruchanb I just run the following code:

    class Program
    {
        static void Main(string[] args)
        {
            using (var ctx = new MyContext())
            {
                ctx.Database.EnsureDeleted();
                ctx.Database.EnsureCreated();

                var s11 = new State { CountryCode = "USA", StateCode = "WA", StateName = "Washington" };
                var s12 = new State { CountryCode = "USA", StateCode = "CA", StateName = "California" };
                var s21 = new State { CountryCode = "PL", StateCode = "DS", StateName = "Dolnoslaskie" };

                var c1 = new Country { CountryCode = "USA", CountryName = "United States", States = new List<State> { s11, s12 } };
                var c2 = new Country { CountryCode = "PL", CountryName = "Poland", States = new List<State> { s21 } };

                ctx.States.AddRange(s11, s12, s21);
                ctx.Countries.AddRange(c1, c2);
                ctx.SaveChanges();
            }

            using (var ctx = new MyContext())
            {
                var query = ctx.Countries
                    .Where(i => i.CountryCode == "USA")
                    .Select(m => new
                    {
                        m.CountryName,
                        m.CountryCode,
                        States = m.States.Select(x => new
                        {
                            x.StateCode,
                            x.StateName,
                            x.CountryCode
                        })
                    }).AsQueryable();
                var result = query.ToList();
                foreach (var r in result)
                {
                    var foo = r.States.ToList();
                    Console.WriteLine(foo);
                }
            }
        }
    }

    public class Country
    {
        public string CountryCode { get; set; }
        public string CountryName { get; set; }
        public virtual ICollection<State> States { get; set; }
    }

    public class State
    {
        public string StateCode { get; set; }
        public string StateName { get; set; }
        public string CountryCode { get; set; }
        public Country Country { get; set; }
    }

    public class MyContext : DbContext
    {
        public DbSet<Country> Countries { get; set; }
        public DbSet<State> States { get; set; }

        protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
        {
            optionsBuilder.UseSqlServer("Server=.;Database=Repro5738;Trusted_Connection=True;MultipleActiveResultSets=true");
        }

        protected override void OnModelCreating(ModelBuilder modelBuilder)
        {
            modelBuilder.Entity<Country>(b =>
            {
                b.ToTable("Country");
                b.HasKey(pr => pr.CountryCode);
                b.HasMany(m => m.States).WithOne(i => i.Country).HasForeignKey(m => m.CountryCode);
            });

            modelBuilder.Entity<State>(b =>
            {
                b.ToTable("State");
                b.HasKey(pr => pr.StateCode);
                b.HasOne(m => m.Country).WithMany(m => m.States).HasForeignKey(m => m.CountryCode);
            });
        }
    }

on our nightly build and got this (expected) sql:

SELECT [i].[CountryName], [i].[CountryCode]
FROM [Country] AS [i]
WHERE [i].[CountryCode] = N'USA'

exec sp_executesql N'SELECT [x].[StateCode], [x].[StateName], [x].[CountryCode]
FROM [State] AS [x]
WHERE @_outer_CountryCode = [x].[CountryCode]',N'@_outer_CountryCode nvarchar(450)',@_outer_CountryCode=N'USA'

Is there any difference between the code you have and the one above?

Hmm, I tried your code and the sql seems to be as you said. What about the no of queries on State though? State is queried each time as the no of countries listed..
Thanks.

@ruchanb N+1 queries for top level collections is tracked here: https://github.com/aspnet/EntityFramework/issues/4007

this fix aimed to mitigate it somewhat by only pulling in relevant entities, rather than entire DbSets

Was this page helpful?
0 / 5 - 0 ratings