-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Breaking change in EFCore 5.0 #2731
Comments
You cannot apply include after applying Distinct. Exception message also makes it clear which scenarios would not work. |
Nope, same exception for
In this case the Includes are first added (extension method) and than filtered. Still the same behavior. Anyway, it worked prior to 3.0 and now works for most, but not all, scenarios. If this really is a new behavior it should get mentioned in breaking changes and how to avoid it. In my scenario this wouldn't work anyway: my service generates a pre-filtered IQueryable the client can use / include the way be wants. This worked perfectly with 3.0. |
It does not matter in order. You cannot do it either way. If you put collection Include before distinct then distinct cannot be done on server side so it will throw client eval error. Your issue is that you are only projecting one column and applying distinct. We don't have enough data to bring additional children from include. It is not a breaking change it is a bug fix. If your query worked earlier then either by co-incidence or just giving you incorrect results.
Prefiltered IQueryable works fully. Selecting just 1 column and applying Distinct makes any collection include to fail. If you are certain of correct collection groupings, you can write a manual join to bring back all the data you want. Beyond that it is just a bad query and there is no common way to rewrite. How to write a proper query will depend on what is the intention of the query. |
@maumar to de-dupe. |
Sorry, but I don't think that this is right! EFCore 3.1 just built a prefectly correct SQL statement:
Since the "CanAccessId" property is not unique I could multiply my resultset, if I do net use the Distinct-Operator. And that's the most performant way the SQL Server can execute this statement. A "Contains", interpreted to "WHERE UserIdCreated IN" would in the best case be not less performant, depinding on the Servers execution plan. The old behavior was perfectly right! And: how about two ForeignKeys referencing the same Entity? |
Just saw that there was some code missing that maybe helps understanding the issue:
Gues there's no reason why this shouldn't work in all scenarios (like it did with 3.1). |
I'm also hitting this one with a query that was working on 3.1. I managed to reproduce it on very basic example, in my case projecting a child entity causes this exception while not projecting the child successfully run the query. Expected output should be "Entities count: 0" for both queries. The second one differs from the first one just because it's projecting the children navigation property, that projection is what fails the second query. |
@smitpatel Below is the code from @ilmax with logs from 3.1 and 5.0 GA daily. internal class Program
{
private static void Main(string[] args)
{
using var context = new Context();
context.Database.EnsureDeleted();
context.Database.EnsureCreated();
context.SaveChanges();
var groupedQuery = context.EntityWithChildren.GroupBy(k => new {k.Id})
.Select(x => new {x.Key.Id, Version = x.Max(v => v.Version)});
var maxVersionQuery = context.EntityWithChildren.Join(groupedQuery,
l => new {l.Id, l.Version}, r => new {r.Id, r.Version}, (l, r) => l);
var entities = maxVersionQuery.ToList();
Console.WriteLine($"Entities count: {entities.Count}");
var maxVersionQuery2 = context.EntityWithChildren.Join(groupedQuery,
l => new {l.Id, l.Version}, r => new {r.Id, r.Version}, (l, r) => l)
.Select(x => new {x.Id, x.Version, Children = x.Children.Select(y => y.Id)});
var entities2 = maxVersionQuery2.ToList();
Console.WriteLine($"Entities count: {entities2.Count}");
}
}
public class EntityWithChildren
{
public Guid Id { get; set; }
public int Version { get; set; }
public ICollection<Child> Children { get; set; }
}
public class Child
{
public int Id { get; set; }
}
public class Context : DbContext
{
private static readonly ILoggerFactory
Logger = LoggerFactory.Create(x => x.AddConsole()); //.SetMinimumLevel(LogLevel.Debug));
public DbSet<EntityWithChildren> EntityWithChildren { get; set; }
protected override void OnModelCreating(ModelBuilder modelBuilder)
{
modelBuilder.Entity<EntityWithChildren>(e =>
{
e.Property(x => x.Id).ValueGeneratedNever();
e.HasKey(x => new {x.Id, x.Version});
e.ToTable("EntitiesWithChildren");
});
}
protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
{
optionsBuilder
.EnableSensitiveDataLogging()
.UseLoggerFactory(Logger)
.UseSqlServer(@"Server=(localdb)\mssqllocaldb;Database=TestX;ConnectRetryCount=0");
//optionsBuilder.LogTo(Console.WriteLine);
}
} Log from 3.1.8:
From 5.0 GA:
|
We threw exception because we tried to find Version from grouping query (since it is part of PK), but we couldn't due to group by. Unique identifier for GroupBy query can be grouping key. cc: @maumar |
Though @ilmax issue is separate issue and should be tracked in a different issue. The original issue remains the same, UserHierarchys is projecting CanAccessId only which is non-PK and applying distinct over that, we don't have a way to uniquely identify UserHierarchy to form collections on client side. At the least, we need runnable simplified repro code for it. There are multiple queries and without exact query, we cannot confirm if it can be translated or not. |
Ok, finally took the time to build a full working example simulation my issue. Perfectly runs on 3.1.8, doesn't with 5.0 RC1: If this is not possible anymore it's sad but ok. But in this case it should be mentioned as a breaking change. Code: internal class Program
{
private static void Main(string[] args)
{
var context = new TestDbContext();
context.Database.EnsureDeleted();
context.Database.EnsureCreated();
var query = context.Offers.AsQueryable();
query = query.Join(context.UserHierarchys.Select(h => h.CanAccessId).Distinct(), q => q.UserIdCreated, k => k,
(q, h) => q);
query = query.Include(p => p.Etikettes);
var q = query.ToArray();
}
}
public class TestDbContext : DbContext
{
private static readonly ILoggerFactory Logger
= LoggerFactory.Create(x => x.AddConsole());//.SetMinimumLevel(LogLevel.Debug));
public DbSet<UserHierarchy> UserHierarchys { get; set; }
public DbSet<Offer> Offers { get; set; }
protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
=> optionsBuilder
.EnableSensitiveDataLogging()
.UseLoggerFactory(Logger)
.UseSqlServer(Your.ConnectionString);
protected override void OnModelCreating(ModelBuilder modelBuilder)
{
modelBuilder.Entity<UserHierarchy>(uh =>
{
uh.HasKey(p => new
{
p.Hierarchy,
p.Id,
p.CanAccessId
});
});
modelBuilder.Entity<Offer>(o => { o.HasMany(p => p.Etikettes).WithOne(p => p.Offer); });
}
}
public class UserHierarchy
{
public string Hierarchy { get; set; }
public Guid Id { get; set; }
public Guid CanAccessId { get; set; }
public string Privilege { get; set; }
}
public class Offer
{
public Guid Id { get; set; }
public Guid? UserIdCreated { get; set; }
public virtual ICollection<OfferEtikette> Etikettes { get; set; }
}
public class OfferEtikette
{
public Guid Id { get; set; }
public Guid OfferId { get; set; }
public virtual Offer Offer { get; set; }
} 3.1:
5.0:
|
And here is the code for my 2nd issue (two relations to the same object). If this is not a simple one I'm going to create a new issue for it: |
@tsproesser Your second issue throws the same error and has the same model validation warning on both 3.1 and 5.0:
|
You can rewrite your query to do filtering via Where rather than Join.
|
Note from triage: we will document as a breaking change the differences in 5.0 here, since some queries generated by 3.1 did return correct results. We should try to add examples (like shown above) for ways to re-write the queries for 5.0. |
@maumar - Can you "implement" this? |
Yeah, sorry! This probably was a bit unclear. It is absolutely off-topic and is worth a new issue I'll create one. Is this a bug? Should this work? Or did I just use it wrong? |
@tsproesser Please file a new issue and we'll look there. This issue is already way overloaded! |
This sure is a suitable workaround, but generates a less performant execution plan! Even though it's still quite fast this is not a real solution for my issue but more a workaround. Edit: did some more tests and there are cases where this is less performant. But it is more or less negligible. Anyway: shouldn't it be still possible? |
Hey @smitpatel do you know what's the issue that tracks fixing the breaking change I reported above? Thanks |
@ilmax - dotnet/efcore#22892 should track that. Though the earlier release it was actually incorrect result since we ignored it altogether. |
Thank you! |
After switching from EF3 to EF5 I get an System.InvalidOperationException on a method that previously worked:
System.InvalidOperationException: "Not enough information to uniquely identify outer element in correlated collection scenario. This can happen when trying to correlate on keyless entity or when using 'Distinct' or 'GroupBy' operations without projecting all of the key columns."
I'm afraid it is a bit complicated to reproduce since it is part of a very huge project, but I'll do my very best and can try to create a sample project, if the misbehavior is unknown and not reproducable:
I'm having a Queryable of Offers that gets authorized by a generic method:
where UserHierarchy is a simple table:
Offer inherits from CreateableBase and its UserIdCreated is joined:
If an authorized Queryable now gets some includes, e.g.
the exception is thrown.
The query works without the Authorize / Include, but not if both are present. Not all includes break the program, but I couldn't figure out what includes do and what don't. Maybe some developer here has some suggestions what to check?
And there's another, maybe correlating issue I hoped to get rid of with EFCore 5.0 but don't:
I have a base class ChangeableBase inheriting from CreateableBase:
I need to set UserChanged to not mapped, since the AutoMapper seams to get confused with relations of the same type pointing to different fields (UserCreated from BaseClass and UserChanged).
Thank you very much for your support!
EF Core version: 5.0.0 rc.1.20451.13
Database provider: Microsoft.EntityFrameworkCore.SqlServer
Target framework: .NET Core 3.1
The text was updated successfully, but these errors were encountered: