Missing related data when inserting data into new item

[johanskoldekrans]

I fetch the data from the database into a new declared object and I dont get the related data like I did in version 2.0.*.

Example:

public class SomeItem
{
     public int Id {get; set;}
     public List<OtherTable1> OtherTable1 {get;set}
}

var data = ctx.SomeTable.AsNoTracking()

.Include(x => x.OtherTable1)
.ThenInclude(x => x.OtherTable2)
.ThenInclude(x => x.SomeOtherTable)

.Include(x => x.OtherTable1)
.ThenInclude(x => x.OtherTable2)
.ThenInclude(x => x.SomeTypeTable)

.Select(c => new SomeItem {
    Id = c.Id,
    OtherTabledata = OtherTable1.ToList() <--This is missing the data from OtherTable2
}).ToList();

In my real life example OtherTable1 has a lot of Include/ThenInclude/ThenIncludes.

should I be doing something different in 2.1 or is this a bug/known issue?

No exceptions... except null exceptions...

Further technical details

EF Core version: 2.1
.Net Core version: 2.1
Database Provider: Microsoft.EntityFrameworkCore.SqlServer
Operating system: Windows 2016
IDE: Visual Studio 2017 15.7.2

[ajcvickers]

@smitpatel @maumar Can you look into this as a possible regression from 2.0?
/cc @divega

[smitpatel]

Correlated subquery optimization causes the include annotations going through navigation being optimized to be ignored hence the navigation did not get populated.
Work-around is to remove ToList to have 2.0 behavior.

[johanskoldekrans]

@smitpatel Sorry the workaround didnt help. Made it into an ICollection but same result. Looking at the SQL Profiler it still seems to be optimized away. The data isn't read from the database.

[johanskoldekrans]

@smitpatel Sorry, just to be clear. I removed the .ToList() and changed the property in the class to an ICollection.

[smitpatel]

@johanskoldekrans - Thanks for pointing out. I verified that there is no work-around yet.

[maumar]

if SomeTable entity doesn't have too many properties, you can do something like this as a workaround:

var data = ctx.SomeTable.AsNoTracking()
.Include(x => x.OtherTable1)
.ThenInclude(x => x.OtherTable2)
.ThenInclude(x => x.SomeOtherTable)
.ToList()
.Select(c => new SomeItem {
    Id = c.Id,
    OtherTabledata = c.OtherTable1.ToList()
}).ToList();

[johanskoldekrans]

@maumar Yes I know. But it is a gazillion times slower and I would never do that even if my life depended on it. :-)

[johanskoldekrans]

I measured before and it was about 35 times slower..

[maumar]

There are multiple problems here. For cases when the root entity is not in the final projection (e.g. because Select/SelectMany is used on navigation), the recommended pattern should be to apply include after the projection, like its done here: #11053

However, if the Include is applied directly on projected collection (i.e. without the select/selectmany like in #11053), AsQueryable call is needed in order to compose include on top of it. That in turn blocks the correlated collection optimization (#12195), because AsQueryable is a result opearator - so we get N+1 queries in that case.

Moreover, if the included property is a collection, we generate invalid query plan because correlated collection and include don't work well together (both modifying and cloning the QM - that should potentially be addressed by the include unification - #10736

[maumar]

@johanskoldekrans the performance hit you are seeing is unexpected. Are you seeing 35x hit when comparing the workaround I suggested with the 2.0.2 query?

Strange thing is that 2.0.2 was essentially performing the workaround query under the covers. When the query was projecting a "naked" collection navigation we would convert that into include, which then merged with includes that you specified explicitly and thats why it all worked. So in theory, the performance between 2.0.2 and 2.2 using the include workaround should be very similar.

In 2.2 we delegated this into the correlated collection optimization pipeline and that currently doesn't play well with additional collection includes.

Could you provide a repro project (entity classes query as well as approximate size of the data - i.e. how many entities of each type) so that we can see whats going on?

[johanskoldekrans]

@maumar I have rewritten all of the affected viewcomponents. I removed the original includes and add them later on using ForEach on the List after I have executed the main query.

The tables arent that huge at all, maybe a couple of hundred thousand rows and good indices on them.

I think the performance hit I got was due to a mix of problems with 15.7.2 of Visual Studio and the Resharper extension I have. After upgrading to 15.7.3 everything is a lot smoother both debugging 2.1 as well as the performance of the queries. I could revert the code but I don't think that ef was the culprit.

I will try the pattern you suggested in #11053 and let you know.

But the 20-30 ms to 3,3 s hit a got was probably due to something else. :-)

[johanskoldekrans]

@maumar Can I pm you stuff? I tried the pattern from #11053 without luck (I think I did it correctly).

[johanskoldekrans]

model.Appointments = ctx.Appointments.AsNoTracking()
                    .Where(a => (a.CreateById == CurrentUser.UserId || a.UpdateById == CurrentUser.UserId || a.AppointmentUsers.Any(x => x.UserId == CurrentUser.UserId)) && a.DeletionDate == null && (customerid == null || a.CustomerId == customerid))
                    .Include(a => a.AppointmentAttachments)
                    .Include(a => a.AppointmentCustomers)
                    .ThenInclude(a => a.Customer)
                    .Include(a => a.AppointmentType)
                    .Include(a => a.AppointmentUsers)
                    .Include(a => a.CreateBy)
                    .Include(a => a.Customer)
                    .Include(a => a.DeletedBy)
                    .Include(a => a.UpdateBy)
                    
                    .Include(a => a.ParentAppointment)
                    .ThenInclude(a => a.Customer)
                    .Include(a => a.InverseParentAppointment)
                    .ThenInclude(a => a.Customer)
                    .Include(a => a.CustomerAdvices)
                    .Include(a => a.AppointmentUsers)
                    .ThenInclude(a => a.User)
                     
                    .Select(
                    a => new ActivityListItem
                    {
                        AppointmentDate = a.AppointmentDate,
                        
                        AppointmentDescription = a.AppointmentDescription,
                        AppointmentId = a.AppointmentId,
                        AppointmentAssignments = a.AppointmentAssignments.ToList(),
                        AppointmentType = a.AppointmentType.AppointmentType1,
                        AppointmentTypeDescription = a.AppointmentType.AppointmentTypeDescription,
                        Attachments = a.AppointmentAttachments.Select(aa => new ActivityListAttachmentItem { AppointmentAttachmentDescription = aa.AppointmentAttachmentDescription, AppointmentAttachmentId = aa.AppointmentAttachmentId }).ToList(),
                        CreateById = a.CreateById,
                        CreateByUserDescription = a.CreateBy.UserDescription,
                        CreateDate = a.CreateDate,
                        CustomerDescription = a.Customer.CustomerDescription,
                        CustomerId = a.CustomerId,
                        Customers = a.AppointmentCustomers.Select(aa => new ActivityListCustomerItem { CustomerId = aa.CustomerId, CustomerDescription = aa.Customer.CustomerDescription }).ToList(),
                        IsAdvisory = a.IsAdvisory,
                        IsContactChange = a.IsContactChange,
                        IsAddressChange = a.IsAddressChange,
                        IsDraft = a.IsDraft,
                        IsSMS = a.IsSms,
                        IsNewAccount = a.IsNewAccount,
                        IsOrderReceived = a.IsOrderReceived,
                        IsTransfer = a.IsTransfer,
                        IsVoteRegistration = a.IsVoteRegistration,
                        IsWithdrawal = a.IsWithdrawal,
                        ParentAppointment = a.ParentAppointmentId == null ? null as ActivityListParentAppointment : new ActivityListParentAppointment { AppointmentId = a.ParentAppointmentId.Value, CustomerId = a.ParentAppointment.CustomerId, CustomerDescription = a.ParentAppointment.Customer.CustomerDescription  },
                        UpdateByUserDescription = a.UpdateBy.UserDescription,
                        UpdateDate = a.UpdateDate,
                        CustomerAdvice = a.CustomerAdvices.ToList(),
                        AppointmentUserListDescription = a.AppointmentUsers.Select(x => x.User.UserDescription).ToList()
                    }
                    
                    )
                    .Include(x => x.AppointmentAssignments)
                    .ThenInclude(x => x.Assignment)
                    .ThenInclude(x => x.AssignmentWithdrawal)
                    .Include(x => x.AppointmentAssignments)
                    .ThenInclude(x => x.Assignment)
                    .ThenInclude(x => x.Decision)
                    .Include(x => x.AppointmentAssignments)
                    .ThenInclude(x => x.Assignment)
                    .ThenInclude(x => x.Customer)
                    .Include(x => x.AppointmentAssignments)
                    .ThenInclude(x => x.Assignment)
                    .ThenInclude(x => x.AssignmentNewAccounts)

                    .Include(x => x.AppointmentAssignments)
                    .ThenInclude(x => x.Assignment)
                    .ThenInclude(x => x.AssignmentType)

                    .Include(x => x.AppointmentAssignments)
                    .ThenInclude(x => x.Assignment)
                    .ThenInclude(x => x.DecisionByUser)

                    .Include(x => x.AppointmentAssignments)
                    .ThenInclude(x => x.Assignment)
                    .ThenInclude(x => x.AssignmentNewAccounts)
                    .OrderByDescending(o => o.AppointmentDate)
                    .Take(items ?? 100)
                    .ToList();

[maumar]

@johanskoldekrans sure, please send the project to [my github name] at microsoft d0t c0m

[johanskoldekrans]

EfCoreTest.zip

I couldn't share code with you from our project unfortunately. But I made a quick example. Hope you can use it.

[ajcvickers]

Notes from triage:

• It was a bug in 2.0 that the Includes were not ignored. We don't want to make this the accepted behavior going forward because it would further complicate the story around Include, which is already hard for people to understand.
• @smitpatel and @maumar to see if there are other reasonable ways to write this query that will work with 2.1.
• Since we already decided on a pattern that uses Include inside a projection (see Query: correlated collection optimization doesn't get applied for subqueries with AsQueryable #12195) then we should try to use that kind of pattern. However, there are API issues extending this to reference navigations.
• Another way to tackle this longer term is with rule-based eager loading (load patterns). See Rule-based eager load (Include) (load patterns/LoadWith) #2953

[smitpatel]

Query:

                var data = (from st in db.SomeTable.AsNoTracking()
                            join ot in db.Set<OtherTable1>()
                                         .Include(e => e.OtherTable2).ThenInclude(e => e.SomeOtherTable)
                                         .Include(e => e.OtherTable2).ThenInclude(e => e.SomeTypeTable)
                             on st.Id equals EF.Property<int>(ot, "SomeItemId") into grouping
                            select new SomeItem
                            {
                                Id = st.Id,
                                OtherTable1 = grouping.ToList()
                            }).ToList();

It loads requested navigations on OtherTable1

[maumar]

@johanskoldekrans I removed the non-EF code from the sample you provided and landed with the following repro:

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

                var someTables = new List<SomeTable>();
                for (var i = 0; i < 50; i++)
                {
                    var sometable = new SomeTable { Name = "SomeTable" + i };
                    someTables.Add(sometable);
                }

                ctx.SomeTable.AddRange(someTables);
                ctx.SaveChanges();

                foreach (var someTable in someTables)
                {
                    Console.WriteLine(someTable.Id);

                    var relatedTables = new List<RelatedTable>();
                    for (var i = 0; i < 100; i++)
                    {
                        var relatedTable = new RelatedTable { Name = "RelatedTable" + someTable.Id + "_" + i, SomeTable = someTable };
                        relatedTables.Add(relatedTable);
                    }

                    ctx.RelatedTable.AddRange(relatedTables);
                    ctx.SaveChanges();

                    var relatedDatas = new List<RelatedData>();
                    foreach (var relatedTable in relatedTables)
                    {
                        for (int i = 0; i < 5; i++)
                        {
                            var relatedData = new RelatedData { RelatedTable = relatedTable, RelatedData1 = "RelatedData" + relatedTable.Id + "_" + i };
                            relatedDatas.Add(relatedData);
                        }
                    }

                    ctx.RelatedData.AddRange(relatedDatas);
                    ctx.SaveChanges();
                }
            }

            using (var ctx = new TestDbContext())
            {
                // original include query - loads everything correctly
                var query1 = ctx.SomeTable
                        .Include(x => x.RelatedTable)
                        .ThenInclude(x => x.RelatedData)
                        .ToList();

                // original correlated collection query - doesn't load RelatedData
                var query2 = ctx.SomeTable
                    .Include(x => x.RelatedTable)
                    .ThenInclude(x => x.RelatedData)
                    .Select(x => new SomeTableItem
                    {
                        Name = x.Name,
                        RelatedTable = x.RelatedTable.ToList()
                    })
                    .ToList();

                // 2.1 workaround - loads everything correctly, produces same sql as query2 was in 2.0.2
                var query3 = ctx.SomeTable
                    .Include(x => x.RelatedTable)
                    .ThenInclude(x => x.RelatedData)
                    .ToList() // <- this is the changed bit
                    .Select(x => new SomeTableItem
                    {
                        Name = x.Name,
                        RelatedTable = x.RelatedTable.ToList()
                    })
                    .ToList();
            }
        }
    }

    public partial class TestDbContext : DbContext
    {
        public virtual DbSet<RelatedData> RelatedData { get; set; }
        public virtual DbSet<RelatedTable> RelatedTable { get; set; }
        public virtual DbSet<SomeTable> SomeTable { get; set; }

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

        protected override void OnModelCreating(ModelBuilder modelBuilder)
        {
            modelBuilder.Entity<RelatedData>(entity =>
            {
                entity.Property(e => e.RelatedData1)
                    .IsRequired()
                    .HasColumnName("RelatedData")
                    .HasMaxLength(50);

                entity.HasOne(d => d.RelatedTable)
                    .WithMany(p => p.RelatedData)
                    .HasForeignKey(d => d.RelatedTableId)
                    .OnDelete(DeleteBehavior.ClientSetNull)
                    .HasConstraintName("FK_RelatedData_RelatedTable");
            });

            modelBuilder.Entity<RelatedTable>(entity =>
            {
                entity.Property(e => e.Name)
                    .IsRequired()
                    .HasMaxLength(50);

                entity.HasOne(d => d.SomeTable)
                    .WithMany(p => p.RelatedTable)
                    .HasForeignKey(d => d.SomeTableId)
                    .OnDelete(DeleteBehavior.ClientSetNull)
                    .HasConstraintName("FK_RelatedTable_SomeTable");
            });

            modelBuilder.Entity<SomeTable>(entity =>
            {
                entity.Property(e => e.Name)
                    .IsRequired()
                    .HasMaxLength(50);
            });
        }
    }

    public class ErrorViewModel
    {
        public string RequestId { get; set; }

        public bool ShowRequestId => !string.IsNullOrEmpty(RequestId);
    }

    public partial class RelatedData
    {
        public int Id { get; set; }
        public string RelatedData1 { get; set; }
        public int RelatedTableId { get; set; }

        public RelatedTable RelatedTable { get; set; }
    }

    public partial class RelatedTable
    {
        public RelatedTable()
        {
            RelatedData = new HashSet<RelatedData>();
        }

        public int Id { get; set; }
        public string Name { get; set; }
        public int SomeTableId { get; set; }

        public SomeTable SomeTable { get; set; }
        public ICollection<RelatedData> RelatedData { get; set; }
    }

    public partial class SomeTable
    {
        public SomeTable()
        {
            RelatedTable = new HashSet<RelatedTable>();
        }

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

        public ICollection<RelatedTable> RelatedTable { get; set; }
    }

    public class TestViewModel
    {
        public List<SomeTableItem> SomeTable { get; set; }
    }

    public class SomeTableItem
    {
        public string Name { get; set; }
        public List<RelatedTable> RelatedTable { get; set; }
    }

I confirmed that the workaround (i.e. query3) loads all the data correctly and it produces identical SQL as your original query had in 2.0.2

generated sql:

SELECT [x].[Id], [x].[Name]
FROM [SomeTable] AS [x]
ORDER BY [x].[Id]

SELECT [x.RelatedTable].[Id], [x.RelatedTable].[Name], [x.RelatedTable].[SomeTableId]
FROM [RelatedTable] AS [x.RelatedTable]
INNER JOIN (
    SELECT [x0].[Id]
    FROM [SomeTable] AS [x0]
) AS [t] ON [x.RelatedTable].[SomeTableId] = [t].[Id]
ORDER BY [t].[Id], [x.RelatedTable].[Id]

SELECT [x.RelatedTable.RelatedData].[Id], [x.RelatedTable.RelatedData].[RelatedData], [x.RelatedTable.RelatedData].[RelatedTableId]
FROM [RelatedData] AS [x.RelatedTable.RelatedData]
INNER JOIN (
    SELECT DISTINCT [x.RelatedTable0].[Id], [t0].[Id] AS [Id0]
    FROM [RelatedTable] AS [x.RelatedTable0]
    INNER JOIN (
        SELECT [x1].[Id]
        FROM [SomeTable] AS [x1]
    ) AS [t0] ON [x.RelatedTable0].[SomeTableId] = [t0].[Id]
) AS [t1] ON [x.RelatedTable.RelatedData].[RelatedTableId] = [t1].[Id]
ORDER BY [t1].[Id0], [t1].[Id]

[johanskoldekrans]

Thanks for the effort on this one. I really appreciate it. ButI have a laaaarge project and I need to find all of the possible problems in it. It looks as sort of a breaking bugfixchangethingy that obviously you all didnt know about at first either. :-)

@maumar Query 1 and 3 in the example is not good for my scenario because they are too slow and produce way to much data. I have a large (not huge twss) database with a fair amount of transactions.

@smitpatel Your example is close to what I did in some of the places in the project (maybe not as clean... :-)) and on others where I know the end result is fewer records I just used the ForEach-extension on the List.

It would be nice if you warned about the optimization you do in the log/console if possible. At least when in debug so they can more easily be found.

All the best to you guys! And thanks again for your commitment and work on EfCore. I have been with you and aspnetcore since before 1.0 in production. It is hard to keep p with alla the changes that are done from version to version but in the end it is worth it. Anyone can see you are on the right track.

[divega]

A new workaround for this issue was posted by @maumar at #12678 (comment).