efcore icon indicating copy to clipboard operation
efcore copied to clipboard

Stop combining delete and insert pairs into updates

Open ajcvickers opened this issue 2 years ago • 6 comments

When two entity types share the same table, then deleting an instance and inserting a new instance into the same row is combined into an update for the row. This makes sense for table sharing, but we also started doing this for entities mapped to different tables. This causes problems when:

  • The delete and/or insert have database side effects, such as cascading behaviors or default constraints--for example, see #27509 and #30614
  • The inserted item is a different type than the deleted item--see #30611

Therefore, we should stop doing this except where needed for table sharing.

ajcvickers avatar Apr 17 '23 15:04 ajcvickers

I'm not sure what we decided any more, but assuming we support optional dependents for table sharing (do we?), would it still make sense to go through the motions of doing delete and insert like we'd do for non-table-sharing? Basically I'd expect Remove/Add to perform exactly the same regardless of whether there's a SaveChanges in between - except where doing that would raise an exception (i.e. non-optional table sharing).

roji avatar Apr 17 '23 15:04 roji

But then we would have to also delete the other entities sharing the table, and re-create those, which would have different side effects.

ajcvickers avatar Apr 17 '23 15:04 ajcvickers

But I'd assume that if we support optional table sharing, it should already be possible to delete the dependent without deleting the principal no? And if so, there's no reason to merge the delete/add for that case...

roji avatar Apr 17 '23 16:04 roji

We can discuss.

ajcvickers avatar Apr 17 '23 16:04 ajcvickers

See also the issue described here: #33653

ajcvickers avatar May 01 '24 12:05 ajcvickers

https://github.com/dotnet/efcore/issues/1699 needs to be implemented before this, otherwise it would break the case where there's a relationship cycle in the database and all involved entities are replaced at the same time (already covered in tests).

Related: #33750 where we need to do the reverse

AndriySvyryd avatar Sep 06 '24 02:09 AndriySvyryd