Open oleg-varlamov opened 9 months ago
Note for EF team: this also repros with SqlServer and still fails on the latest daily build. It is not a regression from EF7, at least for SQL Server. Npgsql didn't support EF7 JSON in 7, so also not a regression there.
In RelationalSqlTranslatingExpressionVisitor.TryRewriteStructuralTypeEquality
for json type comparison we use the same logic as for any other entity (i.e. compare their PK properties). Problem is, Post is a JSON collection, it has two key properties - BlogId (which maps to it's parent Id), and synthesized ordinal key Id
which is not mapped to anything but rather used only in materialization (ordinal of each element in the collection).
Instead, we should be using property-by-property comparison, just like we do for complex types.
@oleg-varlamov on related note, currently we have a problem with JSON (collection) entities that define property named Id
. Even when comparison problem is fixed, you might run into some errors. Consider renaming Post.Id to something else. Relevant issue for context: https://github.com/dotnet/efcore/issues/29380
Thank you, @maumar! In a real project, I temporarily renamed the Id
to Key
at the mapping level and this helped to work around the problem #29380
Instead, we should be using property-by-property comparison, just like we do for complex types.
I don't necessarily disagree, but note that this starts to applies value equality semantics on owned entities, where we usually have identity/reference semantics (e.g. when assigning). It may start to become quite inconsistent if we start doing value semantics in some places and reference semantics in other places.
This also goes to the general question of what we want to do with owned entities going forward.
I don't necessarily disagree, but note that this starts to applies value equality semantics on owned entities, where we usually have identity/reference semantics (e.g. when assigning). It may start to become quite inconsistent if we start doing value semantics in some places and reference semantics in other places.
Yeah, I thought about it overnight as also realized it would be quite problematic to mix the two. Alternatively we could just block this scenario for JSON mapped owned entities (at least until we implement https://github.com/dotnet/efcore/issues/28594). This comparison would make much more sense for complex types, when we have the support.
at least until we implement https://github.com/dotnet/efcore/issues/28594
Even if we had the ID persisted in the JSON, it's still not clear to me what we'd do - would we compare all the properties (including the ID)? If so, that's still value semantics (same discussion as above).
Otherwise, if we compare only the ID, then that wouldn't be what the OP (IMHO very reasonably expects) - the Contains would basically ignore all properties except for the ID. IMHO this is quite counterintuitive - and not very useful - but it indeed corresponds to what we currently do elsewhere:
// Entity equality to inline entity
_ = await ctx.Blogs.Where(b => b == new Blog { Id = 1, Name = "foo" }).ToListAsync();
// Entity equality to parameterized entity
var blog = new Blog { Id = 1, Name = "foo" };
_ = await ctx.Blogs.Where(b => b == blog).ToListAsync();
Result:
SELECT [b].[Id], [b].[Name]
FROM [Blogs] AS [b]
WHERE [b].[Id] = 1
Executed DbCommand (15ms) [Parameters=[@__entity_equality_blog_0_Id='1' (Nullable = true)], CommandType='Text', CommandTimeout='30']
SELECT [b].[Id], [b].[Name]
FROM [Blogs] AS [b]
WHERE [b].[Id] = @__entity_equality_blog_0_Id
I personally think the above behavior is bad; comparing keys makes a lot of sense when an entity in the database is compared to another entity in the database, but much less so when it's compared to an entity that's not in the database (inline/parameterized).
We should probably discuss all this in design...
Re my last comment, the team thinks that this behavior may be a bit unintuitive, but the rule is simple and the behavior is consistent with other places. For example, when an untracked entity instance is passed to DbSet.Delete(), that's interpreted as meaning "delete the entity with that ID", disregarding the others.
note: this also fails for non-json scenarios. We try to generate the following sql:
query:
var postToSearch = new Post() { Foo = 1, Title = "First" };
var result = ctx.Set<Blog>()
.Where(x => x.Posts.Contains(postToSearch))
.ToList();
SELECT b.Id, b.Name, p0.BlogId, p0.Id, p0.Foo, p0.Title
FROM Blogs AS b
LEFT JOIN Post AS p0 ON b.Id == p0.BlogId
WHERE EXISTS (
SELECT 1
FROM Post AS p
WHERE (b.Id == p.BlogId) && ((p.BlogId == @__entity_equality_postToSearch_0_BlogId) && (p.Id == @__entity_equality_postToSearch_0_Id)))
ORDER BY b.Id ASC, p0.BlogId ASC
exception:
No backing field could be found for property 'Post.BlogId' and the property does not have a getter.
Stack Trace:
IPropertyBase.GetMemberInfo(Boolean forMaterialization, Boolean forSet) line 52
ClrPropertyGetterFactory.GetMemberInfo(IPropertyBase propertyBase) line 64
ClrAccessorFactory`1.CreateBase(IPropertyBase propertyBase) line 76
ClrPropertyGetterFactory.Create(IPropertyBase property) line 36
IPropertyBase.GetGetter>b__40_0(RuntimePropertyBase property) line 210
NonCapturingLazyInitializer.EnsureInitialized[TParam,TValue](TValue& target, TParam param, Func`2 valueFactory) line 26
IPropertyBase.GetGetter() line 209
RelationalSqlTranslatingExpressionVisitor.ParameterValueExtractor[T](QueryContext context, String baseParameterName, List`1 complexPropertyChain, IProperty property) line 2153
lambda_method77(Closure, QueryContext)
QueryCompiler.ExecuteCore[TResult](Expression query, Boolean async, CancellationToken cancellationToken) line 86
QueryCompiler.Execute[TResult](Expression query) line 59
EntityQueryProvider.Execute[TResult](Expression expression) line 64
EntityQueryable`1.GetEnumerator() line 78
Searching for a complete match of the properties of an element in an array that is mapped as jsonb does not work. Expected behavior - an SQL query similar to this is generated:
Include your code
Include stack traces
Include provider and version information
EF Core version: 8.0.1 Database provider: Npgsql.EntityFrameworkCore.PostgreSQL 8.0.0 Target framework: .NET 8.0 Operating system: Windows 11 IDE: Jetbrains Rider 2023.3.3