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

EF Core explicit loading query with owned type collection causes cartesian explosion error #32876

Open
lonix1 opened this issue Jan 21, 2024 · 18 comments

Comments

@lonix1
Copy link

lonix1 commented Jan 21, 2024

.NET 7.0.405
EF Core version: 7
Database provider: Npgsql.EntityFrameworkCore.PostgreSQL 7.0.4
Target framework: .NET 7
Operating system: linux
IDE: vscode

I have an EF Core entity with a collection of owned entities (Customer.Contacts) and a collection of regular entities (Customer.OrderedProducts).

When I load data using "explicit loading", I get a "cartesian explosion" error.

A complete minimal reproduction follows (for .NET 7).

dotnet new console -o Repro
cd Repro
dotnet add package Npgsql.EntityFrameworkCore.PostgreSQL
dotnet new tool-manifest
dotnet tool install dotnet-ef
dotnet add package Microsoft.EntityFrameworkCore.Design
# ...add Model.cs and Program.cs files...
dotnet ef migrations add Init
dotnet ef database update
dotnet run

Model.cs:

public class Customer   // owner
{
  public long Id { get; init; }
  public ICollection<Contact> Contacts { get; } = new List<Contact>();         // owned
  public ICollection<Product> OrderedProducts { get; } = new List<Product>();  // regular
}

public class Contact   // owned
{
  public long Id { get; init; }
  public string Name { get; init; }
}

public class Product   // regular
{
  public long Id { get; init; }
  public string Category { get; init; }
}

public class Context : DbContext                      // uses postgres database
{
  public DbSet<Customer> Customers { get; set; }
  public DbSet<Product> Products { get; set; }

  protected override void OnConfiguring(DbContextOptionsBuilder options)
  {
    options.UseNpgsql("Host=localhost;Port=5432;Database=database;Username=username;Password=password;Include Error Detail=True;");
    options.LogTo(Console.WriteLine, LogLevel.Warning);
    options.ConfigureWarnings(x => x.Throw(RelationalEventId.MultipleCollectionIncludeWarning));
  }

  protected override void OnModelCreating(ModelBuilder builder)
  {
    base.OnModelCreating(modelBuilder);
    builder.Entity<Customer>().HasKey(x => x.Id);
    builder.Entity<Customer>().OwnsMany(x => x.Contacts);
    builder.Entity<Customer>().HasMany(x => x.OrderedProducts).WithMany();
    builder.Entity<Product>().HasKey(x => x.Id);
  }
}

Program.cs:

using Microsoft.EntityFrameworkCore;

// seed
using var context = new Context();
var customer = new Customer();
customer.Contacts.Add(new() { Name = "Bob" });
customer.OrderedProducts.Add(new() { Category = "Shoes" });
await context.AddAsync(customer);
await context.SaveChangesAsync();

// read entity
var customer1 = await context.Customers.OrderBy(x => x.Id).FirstAsync();

// load data for tracked entity using explicit loading
await context
  .Entry(customer1)
  .Collection(x => x.OrderedProducts)
  .LoadAsync();                           // ERROR

Error:

Unhandled exception. System.InvalidOperationException: An error was generated for warning 'Microsoft.EntityFrameworkCore.Query.MultipleCollectionIncludeWarning': Compiling a query which loads related collections for more than one collection navigation, either via 'Include' or through projection, but no 'QuerySplittingBehavior' has been configured. By default, Entity Framework will use 'QuerySplittingBehavior.SingleQuery', which can potentially result in slow query performance. See https://go.microsoft.com/fwlink/?linkid=2134277 for more information. To identify the query that's triggering this warning call 'ConfigureWarnings(w => w.Throw(RelationalEventId.MultipleCollectionIncludeWarning))'. This exception can be suppressed or logged by passing event ID 'RelationalEventId.MultipleCollectionIncludeWarning' to the 'ConfigureWarnings' method in 'DbContext.OnConfiguring' or 'AddDbContext'.
   at Microsoft.EntityFrameworkCore.Diagnostics.EventDefinition.Log[TLoggerCategory](IDiagnosticsLogger`1 logger, Exception exception)
   at Microsoft.EntityFrameworkCore.Diagnostics.RelationalLoggerExtensions.MultipleCollectionIncludeWarning(IDiagnosticsLogger`1 diagnostics)
   at Microsoft.EntityFrameworkCore.Query.RelationalShapedQueryCompilingExpressionVisitor.VisitShapedQuery(ShapedQueryExpression shapedQueryExpression)
   at Microsoft.EntityFrameworkCore.Query.ShapedQueryCompilingExpressionVisitor.VisitExtension(Expression extensionExpression)
   at Microsoft.EntityFrameworkCore.Query.RelationalShapedQueryCompilingExpressionVisitor.VisitExtension(Expression extensionExpression)
   at Microsoft.EntityFrameworkCore.Query.QueryCompilationContext.CreateQueryExecutor[TResult](Expression query)
   at Microsoft.EntityFrameworkCore.Storage.Database.CompileQuery[TResult](Expression query, Boolean async)
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.CompileQueryCore[TResult](IDatabase database, Expression query, IModel model, Boolean async)
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.<>c__DisplayClass12_0`1.<ExecuteAsync>b__0()
   at Microsoft.EntityFrameworkCore.Query.Internal.CompiledQueryCache.GetOrAddQuery[TResult](Object cacheKey, Func`1 compiler)
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.ExecuteAsync[TResult](Expression query, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.Query.Internal.EntityQueryProvider.ExecuteAsync[TResult](Expression expression, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.Query.Internal.EntityQueryable`1.GetAsyncEnumerator(CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.EntityFrameworkQueryableExtensions.IncludableQueryable`2.GetAsyncEnumerator(CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.EntityFrameworkQueryableExtensions.LoadAsync[TSource](IQueryable`1 source, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.Internal.ManyToManyLoader`2.LoadAsync(InternalEntityEntry entry, CancellationToken cancellationToken)
   at Program.<Main>$(String[] args) in /tmp/Repro/Program.cs:line 33
   at Program.<Main>(String[] args)

The error is the one expected for a query with multiple .Include(x => x.Foos).Include(x => x.Bars), which leads to "Cartesian Explosion". The solution in those cases is to use AsSplitQuery(). I don't see how that applies in this case.

When I remove the owned type collection, it works as expected.

Is this a bug in my code or in EF?

(I know I can ignore the warning, but I don't want to do that. It is useful for detecting ACTUAL cartesian explosion issues elsewhere in my code.)

UPDATE:
I also posted on SO without resolution, so I assume this is a bug. If so, is there a workaround where I can continue to rely on "MultipleCollectionIncludeWarning" to detect cartesian explosion in my code, but disable it for this particular false positive?

@ajcvickers
Copy link
Contributor

Note from triage: avoid generating the warning when EF is creating the query.

@GertArnold
Copy link

GertArnold commented Jan 25, 2024

Note that the warning/exception is correct and should be generated. When the latter query is allowed to generate & run, this is what it looks like:

      SELECT [t].[Id], [t].[Category], [c].[Id], [t].[CustomerId], [t].[OrderedProductsId], [t0].[CustomerId], [t0].[OrderedProductsId], [t0].[Id], [t0].[CustomerId0], [t0].[Id0], [t0].[Name]
      FROM [Customers] AS [c]
      INNER JOIN (
          SELECT [p].[Id], [p].[Category], [c0].[CustomerId], [c0].[OrderedProductsId]
          FROM [CustomerProduct] AS [c0]
          INNER JOIN [Products] AS [p] ON [c0].[OrderedProductsId] = [p].[Id]
      ) AS [t] ON [c].[Id] = [t].[CustomerId]
      LEFT JOIN (
          SELECT [c1].[CustomerId], [c1].[OrderedProductsId], [c2].[Id], [c3].[CustomerId] AS [CustomerId0], [c3].[Id] AS [Id0], [c3].[Name]
          FROM [CustomerProduct] AS [c1]
          INNER JOIN [Customers] AS [c2] ON [c1].[CustomerId] = [c2].[Id]
          LEFT JOIN [Contact] AS [c3] ON [c2].[Id] = [c3].[CustomerId]
          WHERE [c2].[Id] = @__p_0
      ) AS [t0] ON [t].[Id] = [t0].[OrderedProductsId]
      WHERE [c].[Id] = @__p_0
      ORDER BY [c].[Id], [t].[CustomerId], [t].[OrderedProductsId], [t].[Id], [t0].[CustomerId], [t0].[OrderedProductsId], [t0].[Id], [t0].[CustomerId0]

As you see, both the products and the contacts are queried (with strange redundancies BTW), making the query "explode". It seems to me that explicitly loading a collection should only query that collection.

@lonix1
Copy link
Author

lonix1 commented Jan 25, 2024

I'm confused... is this a bug or "type-enhancement"? (I'm not being sarcastic - I don't know. I assumed it was a bug.)

If it's not a bug (i.e. problem is in our code), how should we handle such cases?

@ajcvickers
Copy link
Contributor

ajcvickers commented Jan 25, 2024

We generate the warning so that the developer can choose to use split-queries if appropriate. For queries that we generate internally, the user doesn't have the option to do that, so it doesn't make sense to generate the warning. I guess this could be a bug that we shouldn't generate the warning, but since it is strictly true, just not helpful, it would just be better to not generate it.

@lonix1
Copy link
Author

lonix1 commented Jan 25, 2024

Thanks, understood.

So is there no good workaround other than "just ignore the warning till v9 / November"?

@ajcvickers
Copy link
Contributor

@lonix1 Can you explain a bit more why you need a workaround?

@lonix1
Copy link
Author

lonix1 commented Jan 25, 2024

Because we get the typical "You have not defined query splitting behaviour... visit this link for info" warning during runtime.

@ajcvickers
Copy link
Contributor

@lonix1 As a workaround, disable the warning.

@lonix1
Copy link
Author

lonix1 commented Jan 25, 2024

That works. But (like mentioned above) it would be disabled globally - so it would not detect real cases of this problem. It's a very useful warning, saved me many times.

So there doesn't seem to be a workaround.

@ajcvickers
Copy link
Contributor

@lonix1 Typically, a workaround is something that is not a perfect fix. If you really don't want the warning, then you can either use our query as a starting point or write your own query to do the loading.

@lonix1
Copy link
Author

lonix1 commented Jan 26, 2024

A workaround for anyone who needs it:

builder.Entity<Customer>().Navigation(x => x.Contacts).AutoInclude(false);

That prevents EF from automatically loading that owned type collection, which prevents the warning from being shown for this specific entity, but does not change that behaviour for others. That warning is very useful, it's best not to disable it globally.

Of course that introduces a new problem, in that one must manually load the collection whenever needed, e.g.

await context.Customers.Include(x => x.Contacts).ToListAsync();

Hopefully we can get a fix before November/v9.

@roji
Copy link
Member

roji commented Jan 26, 2024

That prevents EF from automatically loading that owned type collection

I'm actually not sure that this is the intended behavior - according to the general design around owned entities, it shouldn't be possible to not load one when its parent is loaded... /cc @ajcvickers

Hopefully we can get a fix before November/v9.

That's quite unlikely, as there isn't an actual bug here.

@ajcvickers
Copy link
Contributor

I'm actually not sure that this is the intended behavior - according to the general design around owned entities, it shouldn't be possible to not load one when its parent is loaded..

Right, but, as with almost anything, that can be overridden, since it just an auto-include by convention. I certainly wouldn't do it, but not sure we should block it. @AndriySvyryd?

@GertArnold
Copy link

Can we return to the root problem? There is a loaded entity object customer1 (obviously with it's owned navigations loaded). Now the only thing it wants is to load one regular 1:n navigation explicitly. Why should that generate a query that once again reads owned navigations with all kinds of derived problems? IMHO that's a bug.

@ajcvickers
Copy link
Contributor

ajcvickers commented Jan 26, 2024

@GertArnold When you load an an entity with owned types, the owned types are always also loaded. That's a big part of what owned types are. This means that when loading entities on the other end of a relationship, the owned types for those entities are loaded also. This is by design.

@GertArnold
Copy link

Maybe I'm missing something, but the explicit load action is expected to load Products only and Product doesn't have owned types. Not sure why it should re-query Contacts, they're already there.

@ajcvickers
Copy link
Contributor

@GertArnold Ah, I see what you are saying. I think you are right. I'll file a new issue for that.

@AndriySvyryd
Copy link
Member

AndriySvyryd commented Jan 26, 2024

Handling auto-included members such as properties, complex properties and owned types will be enabled by #1387

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants