dotnet / efcore

EF Core is a modern object-database mapper for .NET. It supports LINQ queries, change tracking, updates, and schema migrations.
https://docs.microsoft.com/ef/
MIT License
13.8k stars 3.2k forks source link

Projections With Full Types Not Fixing Up Navigations on Sqlite and SQL Server #7131

Closed julielerman closed 7 years ago

julielerman commented 8 years ago

I've searched for a related issue as well as blog posts/docs and am not finding anything. I know it's out there somewhere but have to stop looking!

Also related to https://github.com/aspnet/EntityFramework/issues/4007

Steps to reproduce

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

namespace ProjectionProblem
{
  public class Parent
  {
    public Parent()
    {
      Children = new List<Child>();
    }

    public int Id { get; set; }
    public string Description { get; set; }
    public List<Child> Children { get; set; }
  }

  public class Child
  {
    public int Id { get; set; }
    public string Description { get; set; }
  }

  public class MyContext : DbContext
  {
    public DbSet<Parent> Parents { get; set; }

    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
    {
      optionsBuilder.UseSqlite("Data source=ProjectionProblem.db");
    }
  }

  public class Program
  {
    private static void Main(string[] args)
    {
      using (var context = new MyContext())
      {
        context.Database.EnsureDeleted();
        context.Database.EnsureCreated();
        var parent = new Parent {Description = "Parent1"};
        parent.Children.Add(new Child {Description = "Child1"});
        parent.Children.Add(new Child {Description = "Child2"});
        context.Parents.Add(parent);
        context.SaveChanges();
      }
      using (var context2 = new MyContext())
      {
        var newtype = context2.Parents.Select(p => new {Parent = p, p.Children}).ToList();
      }
    }
  }
}

The issue

Navigations not being fixed up in results. newtype.Children has the child objects, but newtype.Parent.Children is empty.

Further technical details

EF Core version: 1.1 Operating system: Visual Studio version: vs2015 but really not applicable

Other details about my project setup:

julielerman commented 8 years ago

Just to add to this...other projections tests are also quite messy. Again, this is probably noted somewhere that I'm just not finding ...not mentioned in the roadmap or in this repo but that could be affected by how I'm searching.

Filtered projections .. e.g. var newtype = context3.Parents.Select(p => new { Parent = p, Children=p.Children.Where(c=>c.Description=="Child2") }).ToList();

The queries are strange. Non of them have any reference to the children. The results are strange. What's in the Children property is a ParameterInjector

Are we supposed to be avoiding projections for the time being? Besides for reasons of the N+1 issues? So, suggest other strategies ala multiple queries with fixup, or explicit loads, and of course log & test to see what's best? (or wait :) ) Thanks

maumar commented 8 years ago

Try:

var newtype = context2.Parents.Select(p => new {Parent = p, p.Children.ToList()}).ToList(); }

Otherwise the child collections won't be automatically enumerated. Perhaps that's the problem.

julielerman commented 7 years ago

Thanks for the suggestion but no you can't even do that ...p.Children is not a queryable. And it is enumerated in that it is getting recognized and there is a query for it and they are being returned, just not fixed up.

And although I know EF Core is not EF6, that would be a very strange change from how LINQ has always worked.

maumar commented 7 years ago

It's worth pointing out that the query plan will contain all the queries necessary to execute a given Linq statement, regardless of whether those queries actually going to be enumerated or not. Best way to ensure that the queries actually run is to look at Profiler in SSMS

julielerman commented 7 years ago

Definitely looked at what was happening in the database

Sent from my *** Phone

On Nov 26, 2016, at 8:15 PM, Maurycy Markowski notifications@github.com<mailto:notifications@github.com> wrote:

It's worth pointing out that the query plan will contain all the queries necessary to execute a given Linq statement, regardless of whether those queries actually going to be enumerated or not. Best way to ensure that the queries actually run is to look at Profiler in SSMS

- You are receiving this because you authored the thread. Reply to this email directly, view it on GitHubhttps://github.com/aspnet/EntityFramework/issues/7131#issuecomment-263096029, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AExnEGyORmZoID1OCw7DK7LHQw9QD9DFks5rCNmhgaJpZM4K8pHn.

tonkajuris commented 7 years ago

This seems to work with the Include statement using sqlite: var types2 = context2.Parents.Include(x => x.Children).Select(p => new { Parent = p, p.Children }).ToList();

SQL Server returns the child items without requiring the include: var types = context2.Parents.Select(p => new { Parent = p, p.Children }).ToList();

maumar commented 7 years ago

Ahh missed the part about Sqlite. Looks like a bug then. I know for a fact that it works fine on SqlServer that's why I made all those suggestions :)

julielerman commented 7 years ago

Thanks guys. I think you are focused on the wrong thing. The issue (original,not follow up) is about the fact that EF is not fixing up the navigations when a projection query pulls back child objects. The query is executed and the proper data is returned using both SQL server and SQLite. Yes there are workarounds. But the fact that the navigation fix up is not being performed is surprising and I felt it justified raising an issue. The follow up issue is the example where I'm filtering the navigation and that's a different problem which have seen with both providers as well.

Sent from my *** Phone

On Nov 27, 2016, at 3:03 AM, Maurycy Markowski notifications@github.com<mailto:notifications@github.com> wrote:

Ahh missed the part about Sqlite. Looks like a bug then. I know for a fact that it works fine on SqlServer that's why I made all those suggestions :)

- You are receiving this because you authored the thread. Reply to this email directly, view it on GitHubhttps://github.com/aspnet/EntityFramework/issues/7131#issuecomment-263107735, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AExnEJGP8djRFBDo_35YfX4Z3KcHvbGXks5rCTlmgaJpZM4K8pHn.

maumar commented 7 years ago

I see, the fixup should be happening for sure. I will look into it.

julielerman commented 7 years ago

Thanks!

Here's a repo with logging using SQLite that tests (with console, not tests): https://github.com/julielerman/EF110ProjectionProblem/

fixup from separate queries (fixup works) fixup from projection (fixup doesn't work) attempt to filter children on projection (no query for children!) filter when using load (works as expected)

Should I open a separate issue for the second problem I raised? Where the filtered projection on children

var newtype = context3.Parents.Select(p =>
new { Parent = p, Children=p.Children.Where(c=>c.Description=="Child2") }).ToList();

didn't even send a query for the children to the database? Again, workarounds exist, but I would expect this to work the way it's always worked in EF.

tonkajuris commented 7 years ago

I was able to verify SQL Server profiler was returning a query. Haven't found a good way to profile SQLLite. I've used Explain and Timer with selects without success. var newtype = context3.Parents.Select(p => new { Parent = p, Children=p.Children.Where(c=>c.Description=="Child2") }).ToList(); I was able to profile the following: exec sp_executesql N'SELECT [e].[Id], [e].[Description], [e].[ParentId] FROM [Children] AS [e] WHERE ([e].[ParentId] = @__get_Item_0) AND ([e].[Description] = N''Child2'')',N'@__get_Item_0 int',@__get_Item_0=1

julielerman commented 7 years ago

Whoa .. okay I'm using Contains in my filter in Sql Server but == in the sample I built up to isolate that uses SQLite. Going to look for the 101st time at my log ...mmm maybe not at my log. I'll use sql server with sqlprofiler.

julielerman commented 7 years ago

No difference on my end. Model may be different. Note (above) that I do NOT have a FK property in the child. This does not impede the fixup when I do separate queries. It may be impeding the fixup in the first projection (p, p.Children) and it may be impeding the QUERY in the 2nd projection . I'll modify my model. (Though if that's the case, it's still a bug ..fixup works in some cases but not others) BRB!

julielerman commented 7 years ago

Still no difference with the explicit FK in child - no fixups, no additional query. So now i'm really confused. You are running my repo? Are you using a nightly build? I'm using 1.1.0 (released). my sqlserver tests are in a completely different project/model. I will change the repo to use SQL Server and profile that, but on the other project, my log and sqlprofiler are matching

julielerman commented 7 years ago

Still no luck with the filtered projection. Only thing coming through profiler/log is SELECT [p].[Id], [p].[Description] FROM [Parents] AS [p]

I cannot see what we could be doing differently. Are you sure you're not seeing the output from the query using "context4"?

maumar commented 7 years ago

Currently the filtered case shouldn't automatically yield the results for child chollection, only if you add ToList or enumerate the collection explicitly

maumar commented 7 years ago

It is a known issue although I don't think we have a separate item for it. I just added some additional context to #4007 that was suggested by Diego in another thread, which might be how we untlimately tackle the problem

julielerman commented 7 years ago

@maumar wrt the fixup issue, I wanted to mention that the child objects are not being tracked when the results are materialized which is why the fixup isn't happening.

maumar commented 7 years ago

I see, we do some special handling of the result collections (wrapping them in a method IIRC) this might be what's preventing the entities to be tracked

julielerman commented 7 years ago

@maumar yes, the ToList workaround does trigger the query ...still no fixup though. ;) But that just goes back to teh same problem as this issue is about. It's notable only because I need to make sure EF6 & earlier users are aware since its different behavior. Telling them "it's not Ef6, don't expect it to work the same" doesn't work very well. So I 'm just trying to point out what to watch out for. :)

maumar commented 7 years ago

This is partially addressed by #8584 - if the collection navigation is not composed on, we re-use include pipeline which handles tracking. If the collection is filtered however, we still use the old rewrite and results are not being tracked.

maumar commented 7 years ago

assigning to @anpete since he is reworking this entire area

anpete commented 7 years ago

@maumar We should be good here now, right? @julielerman Are you able to give this a try on the latest nightly or Preview1?

julielerman commented 7 years ago

@anpete I'll run through my various scenarios and report back

anpete commented 7 years ago

Thanks!

julielerman commented 7 years ago

My integration tests using SQL Server LocalDb (without the supporting classes, but hopefully that's enough) are failing with preview 1. Will check with nightly next. I've seeded my db so I know that there are 3 quotes total in there.

   [TestMethod] //PASSES
        public void EagerLoadViaProjectionRetrievesRelatedData()
        {
            using (var context = new SamuraiContext())
            {
                var samuraiList = context.Samurais
              .Select(s => new { Samurai = s, Quotes = s.Quotes })
              .ToList();
                Assert.AreEqual(3, samuraiList.Sum(s => s.Quotes.Count()));

            }
         }

        [TestMethod]  //FAILS Expected <3>, Actual <0>
        public void EagerLoadViaProjectionTracksRelatedData()
        {
            using (var context = new SamuraiContext())
            {
                context.Samurais
                .Select(s => new { Samurai = s, Quotes = s.Quotes })
                .ToList();
                Assert.AreEqual(3, context.ChangeTracker.Entries<Quote>().Count());
            }
        }
        [TestMethod]  FAILS: Expected <2>, Actual <1>
        public void EagerLoadViaProjectionTracksRelatedDataWhenModified()
        {
            using (var context = new SamuraiContext())
            {
                var samuraiGraph = context.Samurais
              .Select(s => new { Samurai = s, Quotes = s.Quotes })
              .First();
                samuraiGraph.Samurai.Name = "make name different";
                samuraiGraph.Quotes[0].Text = "make quote different";

                Assert.AreEqual(2, context.ChangeTracker.Entries().Count(e => e.State == EntityState.Modified));
            }
        }
julielerman commented 7 years ago

[Update: these problems were due to confusion over SDKs etc. A clean VM with VS2017preview3 and new dotnet core run time and SDK solved the problem] Status: Using EF Core v2.0.0-preview3-25895 Also explicitly installed NETStandard.Library.NETFramework.2.0.0-preview3-25415-01 And had to install tuples library :) which is v4.4.0-preview3-25415-01 Still getting errors about tuple library missing. I see a new version of vs2017 preview just got pushed up a few hours ago. Will try that. I'm avoiding using .net core for this because that adds more layers of problems to the nightly builds. (I learned the hard way)
-aha! .NET 4.7 wasn't even installed

julielerman commented 7 years ago

SUCCESS!!

Finally got things sorted out so that I can run my tests against the nightly EF Core 2.0 build. And my tests for projections are passing:

image

There are two things I am checking for in my tests:

1) Related data retrieved via projection is being tracked. Earlier, it was getting retrieved and in memory but unknown by the change tracker. 2) Filtering related data in a projection is working as expected. Earlier, the filter was ignored. In my test, I'm querying for samurais with their quotes but ONLY those quotes with a particular word in it. Of the 3 total quotes, only 2 have that word. The filter is now only retrieving those quotes.

THANK YOU!! This is an important fix.

I will go now back and do this with EF Core 2 Preview 2

julielerman commented 7 years ago

Preview 2 in netstandard20 project tested via an MSTest netcoreapp2.0 project (just CLI + VSCode for this ...so much simpler :) ) All tests are passing

image

anpete commented 7 years ago

Thanks for taking the time to try this out!

anpete commented 7 years ago

Closing as fixed in 2.0