efcore icon indicating copy to clipboard operation
efcore copied to clipboard

ExecuteDelete fails on entity configured to use table splitting

Open kev24uk opened this issue 1 year ago • 6 comments

I am seeing the below error when attempting to use ExecuteDelete against a table which is mapped to two different entities (for lighter data retrieval when the full object is not necessary but don't want an anonymous type).

Below is a minimal project to reproduce the error (should just need to change the connection string to something that works for you): EF-IssueRepro.zip

Unhandled exception. System.InvalidOperationException: The LINQ expression 'DbSet<Contact>()
    .Where(c => c.FirstName == "DeleteMe")
    .ExecuteDelete()' could not be translated. Additional information: The operation 'ExecuteDelete' is being applied on the table 'Contacts' which contains data for multiple entity types. Applying this delete operation will also delete data for other entity type(s), hence it is not supported. See https://go.microsoft.com/fwlink/?linkid=2101038 for more information.
   at Microsoft.EntityFrameworkCore.Query.RelationalQueryableMethodTranslatingExpressionVisitor.VisitMethodCall(MethodCallExpression methodCallExpression)
   at Microsoft.EntityFrameworkCore.Query.QueryableMethodTranslatingExpressionVisitor.Translate(Expression expression)
   at Microsoft.EntityFrameworkCore.Query.RelationalQueryableMethodTranslatingExpressionVisitor.Translate(Expression expression)
   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__DisplayClass9_0`1.<Execute>b__0()
   at Microsoft.EntityFrameworkCore.Query.Internal.CompiledQueryCache.GetOrAddQuery[TResult](Object cacheKey, Func`1 compiler)
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.Execute[TResult](Expression query)
   at Microsoft.EntityFrameworkCore.Query.Internal.EntityQueryProvider.Execute[TResult](Expression expression)
   at Microsoft.EntityFrameworkCore.RelationalQueryableExtensions.ExecuteDelete[TSource](IQueryable`1 source)

kev24uk avatar Feb 19 '24 10:02 kev24uk

I am seeing the below error when attempting to use ExecuteDelete against a table which is mapped to two different entities (for lighter data retrieval when the full object is not necessary but don't want an anonymous type).

I may not understand your full scenario, but using table splitting with partially-duplicated data isn't a good idea just for lighter data retrieval. If you don't want to use anonymous objects, you can simply project out the regular Contact type, populating only the properties you're interested in:

var liteContacts = await context.Contacts.Select(c => new Contact { Id = c.Id, FirstName = c.FirstName }).ToListAsync();

Compared to your current solution, this removes the duplication from the database (and the need to keep Contact and ContactLite in sync in the database), and also provides the flexibility of projecting whichever properties you want on a per-query basis (in your current solution, ContactLite again has a hard-coded - even if smaller - list of properties).

Beyond that, allowing ExecuteDelete with table splitting is something we've discussed before; allowing this would mean that deleting Contact automatically deletes any entity sharing the table, and it's not clear that this makes sense. However, if you model ContactLite as a complex type or owned entity, that should work since that models ContactLite as being contained inside Contact.

roji avatar Feb 20 '24 09:02 roji

@roji @AndriySvyryd I've been thinking about this since I saw it yesterday, and I wonder if we should allow this case at least if the principal is the one being deleted, since there is a clear principal and dependent defined here, and I think in a table splitting case like this that would be the expected behavior. I'm not sure if we should do the same thing if the delete is for the dependent, but I'm also not sure we shouldn't.

ajcvickers avatar Feb 20 '24 09:02 ajcvickers

Yeah, I'm also unsure about it all... Deleting the principal when ExecuteDelete is invoked over the dependent feels very odd, but deleting the dependent when ExecuteDelete is invoked over the principal seems consistent with what happens when you currently delete principals that have dependents in another table (with a required relationship). In other words, if you delete a Blog, we already delete all of its Posts in another table (cascade delete), so I guess it makes sense to also "cascade" delete the dependent here if it happens to be in the same table?

(I'm assuming we don't support table splitting where the dependent can exist without a principal!)

roji avatar Feb 20 '24 11:02 roji

I may be looking at this too simplistically, but when code is written such as dbContext.Table.Where(clause).ExecuteDelete(), I would expect it to generate SQL like DELETE FROM Table WHERE clause and execute it, regardless of how the entity is configured. If there are cascading deletes, I'd also expect those to happen as well.

Using the library https://github.com/borisdj/EFCore.BulkExtensions does do this, and works correctly in my scenario with two entities mapped to a single table too.

Is the intention here perhaps to protect the developer from deleting data they did not intend to (because they didn't know the two entities were in fact sharing the same table perhaps?) or is there some other underlying reason which stops other issues occuring?

Could there be an option to override and "accept" that it will delete data for multiple entities even though you are only targeting one with the delete (and maybe even have EF split out a warning in logging to say that it is doing this)?

kev24uk avatar Feb 20 '24 12:02 kev24uk

@kev24uk there's definitely no question of any behavior here being possible/feasible, it's more a question of what should be considered the right behavior. For example, it feels pretty clear to me that implicitly deleting a principal (Contact) because ExecuteDelete was called on a dependent (ContactLite) is problematic (but the other way may be fine). We'll discuss this as a team etc.

roji avatar Feb 20 '24 17:02 roji

@roji Thats great, I look forward to hearing the team’s decision on this one :)

kev24uk avatar Feb 20 '24 23:02 kev24uk

Duplicate of #28521

roji avatar Feb 26 '24 20:02 roji

@kev24uk we think it's OK to do this - see the full note in https://github.com/dotnet/efcore/issues/28521#issuecomment-1965237293.

roji avatar Feb 26 '24 20:02 roji