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

Including reference navigation and projecting collection navigation throws #18090

Closed
fschlaef opened this issue Sep 27, 2019 · 16 comments · Fixed by #18625
Closed

Including reference navigation and projecting collection navigation throws #18090

fschlaef opened this issue Sep 27, 2019 · 16 comments · Fixed by #18625
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-bug
Milestone

Comments

@fschlaef
Copy link

fschlaef commented Sep 27, 2019

When using .Include().ThenInclude() and selecting a collection, EF throws an InvalidOperationException with error EF.Property called with wrong property name.

Note that also using .Include() on the selected collection still throws.

The only "workaround" is to avoid .ThenInclude() which is not a solution for me (I need SomeCustomerDataFkNavigation)

Steps to reproduce

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

namespace _18090Repro
{
    class Program
    {
        static void Main(string[] args)
        {
            using var context = new MyContext();

            context.Database.EnsureDeleted();
            context.Database.EnsureCreated();
            context.SomeCustomerData.Add(new SomeCustomerData { });
            context.SaveChanges();

            context.Customer.Add(new Customer() { SomeCustomerDataFk = 1 });
            context.Customer.Add(new Customer() { });
            context.SaveChanges();

            context.Invoice.Add(new Invoice { CustomerFk = 1 });
            context.SaveChanges();

            context.InvoiceLine.Add(new InvoiceLine { InvoiceFk = 1 });
            context.SaveChanges();

            var invoices = context.Invoice
                .Include(i => i.CustomerFkNavigation)
                    .ThenInclude(c => c.SomeCustomerDataFkNavigation)
                .Select(i => new Invoice()
                {
                    Id = i.Id,
                    CustomerFkNavigation = i.CustomerFkNavigation,
                    InvoiceLine = i.InvoiceLine // This throws
                })
                .ToList();
        }
    }

    public partial class MyContext : DbContext
    {
        public virtual DbSet<Customer> Customer { get; set; }
        public virtual DbSet<Invoice> Invoice { get; set; }
        public virtual DbSet<InvoiceLine> InvoiceLine { get; set; }
        public virtual DbSet<SomeCustomerData> SomeCustomerData { get; set; }

        private static readonly LoggerFactory Logger = new LoggerFactory(new[] { new DebugLoggerProvider() });

        protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
        {
            string connectionString = "Server=.;Database=Repro18090;Trusted_Connection=True;MultipleActiveResultSets=true";

            optionsBuilder.UseSqlServer(connectionString)
                .EnableSensitiveDataLogging()
                .UseLoggerFactory(Logger);
        }

        protected override void OnModelCreating(ModelBuilder modelBuilder)
        {
            modelBuilder.Entity<Customer>(entity =>
            {
                entity.HasIndex(e => e.SomeCustomerDataFk);

                entity.Property(e => e.Id).HasColumnName("id");

                entity.Property(e => e.SomeCustomerDataFk).HasColumnName("someCustomerData_fk");

                entity.HasOne(d => d.SomeCustomerDataFkNavigation)
                    .WithMany(p => p.Customer)
                    .HasForeignKey(d => d.SomeCustomerDataFk)
                    .HasConstraintName("FK_Customer_SomeCustomerData");
            });

            modelBuilder.Entity<Invoice>(entity =>
            {
                entity.HasIndex(e => e.CustomerFk);

                entity.Property(e => e.Id).HasColumnName("id");

                entity.Property(e => e.CustomerFk).HasColumnName("customer_fk");

                entity.HasOne(d => d.CustomerFkNavigation)
                    .WithMany(p => p.Invoice)
                    .HasForeignKey(d => d.CustomerFk)
                    .OnDelete(DeleteBehavior.ClientSetNull)
                    .HasConstraintName("FK_Invoice_Customer");
            });

            modelBuilder.Entity<InvoiceLine>(entity =>
            {
                entity.Property(e => e.Id).HasColumnName("id");

                entity.Property(e => e.InvoiceFk).HasColumnName("invoice_fk");

                entity.HasOne(d => d.InvoiceFkNavigation)
                    .WithMany(p => p.InvoiceLine)
                    .HasForeignKey(d => d.InvoiceFk)
                    .OnDelete(DeleteBehavior.ClientSetNull)
                    .HasConstraintName("FK_InvoiceLine_Invoice");
            });

            modelBuilder.Entity<SomeCustomerData>(entity =>
            {
                entity.Property(e => e.Id).HasColumnName("id");
            });
        }
    }

    public partial class Customer
    {
        public Customer()
        {
            Invoice = new HashSet<Invoice>();
        }

        public int Id { get; set; }
        public int? SomeCustomerDataFk { get; set; }

        public virtual SomeCustomerData SomeCustomerDataFkNavigation { get; set; }
        public virtual ICollection<Invoice> Invoice { get; set; }
    }

    public partial class Invoice
    {
        public Invoice()
        {
            InvoiceLine = new HashSet<InvoiceLine>();
        }

        public int Id { get; set; }
        public int CustomerFk { get; set; }

        public virtual Customer CustomerFkNavigation { get; set; }
        public virtual ICollection<InvoiceLine> InvoiceLine { get; set; }
    }

    public partial class InvoiceLine
    {
        public int Id { get; set; }
        public int InvoiceFk { get; set; }

        public virtual Invoice InvoiceFkNavigation { get; set; }
    }

    public partial class SomeCustomerData
    {
        public SomeCustomerData()
        {
            Customer = new HashSet<Customer>();
        }

        public int Id { get; set; }

        public virtual ICollection<Customer> Customer { get; set; }
    }
}

Further technical details

EF Core version:
Database provider: Microsoft.EntityFrameworkCore.SqlServer
Target framework: .NET Core 3.0
Operating system: Windows 10 x64
IDE: Visual Studio 2019 16.3.1

@guftall
Copy link

guftall commented Oct 16, 2019

I get same error in ubuntu18 and EFCore 3.0.0
there was a workaround for me and i think this should work for you too, first convert your IQueryable to IEnumerable and then call Select method on that:

var invoices = context.Invoice
                .Include(i => i.CustomerFkNavigation)
                    .ThenInclude(c => c.SomeCustomerDataFkNavigation)
                .ToList()
                .Select(i => new Invoice()
                {
                    Id = i.Id,
                    CustomerFkNavigation = i.CustomerFkNavigation,
                    InvoiceLine = i.InvoiceLine
                });

@maumar maumar assigned smitpatel and unassigned maumar Oct 17, 2019
@fschlaef
Copy link
Author

@guftall This only works for small datasets as all you're doing is storing the entire table in memory, which would result in very poor performance with a lot of data.

@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 Oct 28, 2019
@smitpatel smitpatel changed the title EF.Property called with wrong property name when selecting collection and using ThenInclude Including reference navigation and projecting collection navigation throws Oct 28, 2019
smitpatel added a commit that referenced this issue Oct 28, 2019
Issue: Earlier we expanded navigation in selector and include in separate phase. This causes issue because if the latter visitor expands a reference navigation then former visitor's collection expansions have incorrect references.
Fix: Fix is to make include expansion part of selector expansion as next phase. So by the time we apply include, the correlation predicate in collection hasn't been converted to actual reference. So when it gets converted, it takes correct reference.

Also apply pending selector inside lambda expression since it is a subquery. This caused issue when subquery has a projection which has navigation to expand, which we never visited.

Resolves #18127
Resolves #18090
Resolves #17852
Resolves #17756
smitpatel added a commit that referenced this issue Oct 28, 2019
Issue: Earlier we expanded navigation in selector and include in separate phase. This causes issue because if the latter visitor expands a reference navigation then former visitor's collection expansions have incorrect references.
Fix: Fix is to make include expansion part of selector expansion as next phase. So by the time we apply include, the correlation predicate in collection hasn't been converted to actual reference. So when it gets converted, it takes correct reference.

Also apply pending selector inside lambda expression since it is a subquery. This caused issue when subquery has a projection which has navigation to expand, which we never visited.

Resolves #18127
Resolves #18090
Resolves #17852
Resolves #17756
smitpatel added a commit that referenced this issue Oct 28, 2019
Issue: Earlier we expanded navigation in selector and include in separate phase. This causes issue because if the latter visitor expands a reference navigation then former visitor's collection expansions have incorrect references.
Fix: Fix is to make include expansion part of selector expansion as next phase. So by the time we apply include, the correlation predicate in collection hasn't been converted to actual reference. So when it gets converted, it takes correct reference.

Also apply pending selector inside lambda expression since it is a subquery. This caused issue when subquery has a projection which has navigation to expand, which we never visited.

Resolves #18127
Resolves #18090
Resolves #17852
Resolves #17756
smitpatel added a commit that referenced this issue Oct 29, 2019
Issue: Earlier we expanded navigation in selector and include in separate phase. This causes issue because if the latter visitor expands a reference navigation then former visitor's collection expansions have incorrect references.
Fix: Fix is to make include expansion part of selector expansion as next phase. So by the time we apply include, the correlation predicate in collection hasn't been converted to actual reference. So when it gets converted, it takes correct reference.

Also apply pending selector inside lambda expression since it is a subquery. This caused issue when subquery has a projection which has navigation to expand, which we never visited.

Resolves #18127
Resolves #18090
Resolves #17852
Resolves #17756
smitpatel added a commit that referenced this issue Oct 29, 2019
Issue: Earlier we expanded navigation in selector and include in separate phase. This causes issue because if the latter visitor expands a reference navigation then former visitor's collection expansions have incorrect references.
Fix: Fix is to make include expansion part of selector expansion as next phase. So by the time we apply include, the correlation predicate in collection hasn't been converted to actual reference. So when it gets converted, it takes correct reference.

Also apply pending selector inside lambda expression since it is a subquery. This caused issue when subquery has a projection which has navigation to expand, which we never visited.

Resolves #18127
Resolves #18090
Resolves #17852
Resolves #17756
smitpatel added a commit that referenced this issue Oct 30, 2019
Issue: Earlier we expanded navigation in selector and include in separate phase. This causes issue because if the latter visitor expands a reference navigation then former visitor's collection expansions have incorrect references.
Fix: Fix is to make include expansion part of selector expansion as next phase. So by the time we apply include, the correlation predicate in collection hasn't been converted to actual reference. So when it gets converted, it takes correct reference.

Also apply pending selector inside lambda expression since it is a subquery. This caused issue when subquery has a projection which has navigation to expand, which we never visited.

Resolves #18127
Resolves #18090
Resolves #17852
Resolves #17756
@fschlaef
Copy link
Author

fschlaef commented Nov 7, 2019

@smitpatel confirmed fixed as of 3.1.0-preview3.19554.8 👍

@clement911
Copy link
Contributor

How can such obvious bugs make it to RTM?
This has thrown our 2.2 to 3.0 migration completely off. Now do we have to park all of this migration and wait for 3.1?

@clement911
Copy link
Contributor

Or can we use this preview package? I don't see it published on nuget. Is there another way of getting it?

@Suchiman
Copy link
Contributor

@clement911 on nuget there are only preview releases, this is fixed for preview 3 but that's not out yet. It is available in daily builds for which you can find instructions here https://github.com/aspnet/AspNetCore/blob/master/docs/DailyBuilds.md

@clement911
Copy link
Contributor

clement911 commented Nov 13, 2019

Got it thank you. Seems a bit risky for production so I guess I'll have to wait.
Will this fix be included in 3.1?

@heyligengregory
Copy link

I'm facing to the same problem. If it's included in 3.1, do you have any idea of the release date ?

@ErikEJ
Copy link
Contributor

ErikEJ commented Nov 13, 2019

@heyligengregory 3.1 will come start of December 2019

@clement911
Copy link
Contributor

Can you please include basic regression queries in the test suite? It's not like it's a super complicated query and it throws a runtime error.

@fschlaef
Copy link
Author

@clement911 No offense but did you read the issue ? It clearly says that this issue is in the preview3 milestone, that it is closed as fixed (I even confirmed it in a message with the version detail), and if you look at the last commit (47f6cef) you will find tests called Including_reference_navigation_and_projecting_collection_navigation and Including_reference_navigation_and_projecting_collection_navigation_2 which target this issue.

This answers all your questions, and all the information was already there.

@clement911
Copy link
Contributor

No offence taken. I'm a user of EF core and not a contributor. I'm not familiar with the inner workings of EF core or its testing code base (I wish I had time to).

I come from a user perspective and I just wanted to point out that it seems like such a severe regression on such a simple query should have been caught before EF Core 3.0 was released.
Is that not fair?

@ajcvickers
Copy link
Member

@clement911 Yes, that's fair. That's why we run more than 50,000 tests on every build. However, the problem space for different queries is huge. Combine this with different models and model configurations and it gets even bigger. So sometimes things get missed. We do our best and continually strive to do better, but we make mistakes.

@clement911
Copy link
Contributor

Fair enough. Apologies if I came out too strong but it was quite frustrating to have to abort our 3.0 migration after battling through all of the asp.net core migration issues because of this issue.

@Suchiman
Copy link
Contributor

@clement911 with dotnet/announcements#139 or https://www.nuget.org/packages/Microsoft.EntityFrameworkCore/3.1.0-preview3.19554.8 it should be fixed now

@clement911
Copy link
Contributor

Sweet thank you @Suchiman

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-bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants