-
Notifications
You must be signed in to change notification settings - Fork 3.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
One-to-one relationship with bad data behaviors differently for bi-directional verses uni-directional relationships #8137
Comments
@JonPSmith Please share your Chapter07DbContext and your ConfigureServices method in Startup.cs file. |
@MaherJendoubi . I have added my |
@JonPSmith How about ConfigureServices()? to reproduce the issue. |
I don't have a ConfigureServices(). This is a unit test using in-memory Sqlite, which I have used in other issues I have posted here. |
@JonPSmith Would you please share a repro project sample? |
@JonPSmith I didn't succeed to reproduce the issue because of this : |
OK. The repo is at https://github.com/JonPSmith/EfCoreInAction and you need branch Some of the things you need are already available.
About the only thing you need is the SqlLite in-memory, but you can use SqlServer. Its just some code to set up the SqlLite in-memory. PS. You don't need the logging. I use that to check how EF Core builds the database. |
@JonPSmith I'm finding it a little hard to understand exactly what you are describing, but two things may help clarify the behaviors of EF:
|
Thanks for your comments - they make me go back to the unit test and look more closely at the database etc. It turns out there is not difference in the database/EF Core configuration between both cases - they both create a one-to-one relationship with a UNIQUE constraint on the FK. This made me look further and I found that not all of the entities were being written to the database. I have updated my initial post with the new information, but basically the call to SaveChanges detaches all but one of the entities that has a duplicate one-on-one relationship. I didn't expect that! I have no idea what is happening here, but that is what the problem is. I have created a branch |
@JonPSmith This is what is happening:
The result is that, as part of tracking the entities, the relationship was changed twice to associate different Tickets and Attendees. |
OK, that was not obvious, but I can see how that works. Another learning point. Not sure I see why the two cases - with inverse navigation and without inverse navigation affect that, but don't bother to explain as you have far better things to do. |
@JonPSmith The two cases should behave the same way, so something is not working correctly. I will investigate further. |
I found a difference between the two. The SQL tables creation is identical, but...
CREATE INDEX "IX_Tickets_AttendeeId" ON "Tickets" ("AttendeeId");
Because EF Core makes decisions to detach an entity prior to writing the data to the database I assume this is a symptom and not the cause. But maybe this will help diagnose the issue, as I would expected a required relationship like this should have a unique index. PS. I'm glad you have marked this as a bug, as I was tearing my hair out trying to understand why it was working this way! |
Hi @ajcvickers, I see you have marked this as a bug, and the milestone is 2.0. My chapter 7, on configuring relationships, will come out before Version 2.0 is out, so can I check on what is wrong with the relationship so I can note this. My tests show that this issue arises if a fully defined one-to-one relationship is configured by convention or via Fluent API. The only difference I can see between what I expect (and see in a partially defined one-to-one relationship) is that a Is there anything else I missed? |
@JonPSmith The general concept here is that once you have a one-to-one relationship, then assigning any entity to that relationship will cause the newly assigned entity to replace any entity that is already there. Replacing the entity that is already there can mean different things:
However, if the entity has not yet been saved (i.e. it is in the Added state), then there is nothing to be deleted from the entity, so deleting really means "not saving", and hence it will be detached. I think a principle here is that EF will attempt to do what makes sense given that your model is correct and your use of EF is geared towards doing valid things. Trying to do invalid things (like trying to create multiple conflicting dependents) and then trying to have EF tell you this isn't really an appropriate use of EF. |
Thanks for that. I agree that EF Core does a sensible (actually quite clever) action in that case. But you have marked this as a bug - I assume because the fully defined relationship exhibits a different action to the partially defined relationship. I just wanted to know how you plan to change it, again I assume you will change it to work the same as the partially defined relationship. Further tests shows that the most probable duplicate error, which is a duplicate being added in another DbContext session, does throw an exception. I therefore think the difference between the two cases is quite small. I therefore think I won't worry about this in the book, but I will mark the failing unit test with this issue number. I'm happy with that. |
@JonPSmith Yes, the reason the issue is open is because of the different behaviors, but I won't have a crisp idea of what is going on until I investigate more. |
…stealing using navigations and/or FK properties Mark the dependent as detached/deleted when breaking a required relationship even if the FK doesn't overlap PK Fixes #8137
… new one is reference stealing using navigations and/or FK properties If the FK was non-identifying make sure the properties are set to null instead of the dependent being removed. Fixes #8137
… new one is reference stealing using navigations and/or FK properties If the FK was non-identifying make sure the properties are set to null instead of the dependent being removed. Fixes #8137
@AndriySvyryd Wait, throw where? We should only throw if you attempt to null out an FK property on a required relationship without cascade delete and then call SaveChanges. Is that what you mean? |
@ajcvickers Yes, seems that my comment doesn't actually clarify anything, so I'll delete it. |
I am building some tests/examples for one-to-one relationships, and I found that EF Core differs in its handling of the relationship if an inverse navigational relationship exists or not:
I found this with a test to show that duplicate one-on-one entities should throw an exception: in case 1 an exception is thrown if I have a duplicate. In case 2 no exception is thrown.
UPDATE: I have found change in behaviour is due to all but one of the entities that has a duplicate one-to-one relationship are set to
Detached
after SaveChanges is called. They have not been saved to the database, so they don't trigger the exception on a UNIQUE constraint.NOTE: I agree this seems very strange and it could be my mistake, but I have looked at it and can't see anything wrong with my code. Let me know if you spot anything.
Steps to reproduce
My main entity
My sub-entity
My configuration
NOTE: The configuration shown does NOT work (case 2), but if I replace the
WithOne(p => p.Attendee)
with.WithOne()
then it works.My DbContext
My unit test is
Notes
The output of state of the Attendee entities in case 2 (the failure state) are:
Further technical details
EF Core version: 1.1.1
Database Provider: Microsoft.EntityFrameworkCore.Sqlite
Operating system: Windows 10
IDE: Visual Studio 2017
The text was updated successfully, but these errors were encountered: