Skip to content
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

Update DeleteBehavior.Restrict to not apply when navigations are changed #9703

Closed
Wain123 opened this issue Sep 6, 2017 · 3 comments
Closed
Assignees
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-enhancement
Milestone

Comments

@Wain123
Copy link

Wain123 commented Sep 6, 2017

Since #9686 and #8633 have been closed despite not being resolved at all, I feel that there is some fundamental misunderstanding of what the issues are asking about. I will try to explain more clearly.

I want a relationship that behaves exactly like a nullable FK in a database.

The foreign key should do exactly what the navigation property does. If I set the navigation to null, the FK becomes null. If I set the navigation to an object, the FK becomes the PK of that object. This part currently works perfectly in EFCore 2.1 with a nullable FK, ClientSetNull, ChangingAndChangedNotificationsWithOriginalValues and a navigation property setter that automatically invokes PropertyChanged.

The navigation and FK should ONLY change if I explicitly tell them to change - either by assigning a value to the navigation, or performing add/remove on the collection side. Just like a FK in a database.

If I try to delete the parent while a child points to him, then this is a bug in my code (or a mistake on the end user's part), and I want to be notified of the mistake so I have a chance to correct it. Just like the database tells me that I can't delete the parent because "ORA-02292: integrity constraint [...] violated - child record found". I don't want the FK to be quietly set to null or the child to be cascade deleted or anything of that sort.

When I call SaveChanges, I want the current value of the FK to be saved to the database. If that is an inconsistent state, then I want to get an error telling me I made a mistake. I don't want EF to attempt to guess which consistent state I "meant" to get (because it tends to guess wrong!).

Since issue #8633 has been marked as fixed by ClientSetNull, one would think that ClientSetNull does absolutely nothing on parent deletion and on SaveChanges (as #8633 describes). But in reality, ClientSetNull sets the FK to null in that case. If this is working as intended, then please remove the "fixed" label from issue #8633.

Issue #9686 has been closed with the comment that a non-nullable FK would help. It wouldn't, because I need an optional (i.e. nullable) relationship.

@ajcvickers
Copy link
Member

ajcvickers commented Sep 6, 2017

@Wain123 I see where the disconnect is now. Traditionally, EF keeps FKs in sync with changes to the graph, including deletions and navigation changes. In 2.0, we added a new behavior "Restrict" which stops EF from doing this so that if you want the FK to change, then you need to explicitly set an FK value. This can be considered the same as the database behavior without cascades; that is, any change to an FK needs to be made explicitly.

However, EF does not have the ability to sometimes change the FK when the graph changes (e.g. when navigations change) but not at other times (e.g. when entities are deleted). We will discuss this as a possible new feature.

#8633 is fixed by the Restrict behavior, not by the ClientSetNull behavior. However, #8633 says nothing about what happens when navigation properties change. Separating the delete behavior from the navigation property behavior is a new thing. (It might have been what you wanted in #8633, but it isn't what you said in #8633.)

@ajcvickers
Copy link
Member

We discussed in triage and decided to change the Restrict behavior so that FKs will still be nulled when navigations are changed, but not when related entities are deleted. This is both quite an obscure feature and only a breaking change around negative cases, so it should be fine to do in 2.1.

@ajcvickers ajcvickers self-assigned this Sep 7, 2017
@ajcvickers ajcvickers added this to the 2.1.0 milestone Sep 7, 2017
@ajcvickers ajcvickers changed the title A relationship that behaves exactly the same as a nullable FK in a database Update DeleteBehavior.Restrict to not apply when navigations are changed Sep 7, 2017
@ajcvickers
Copy link
Member

Note: consider updating reverse engineering when making this change: see #9935.

@ajcvickers ajcvickers modified the milestones: 2.1.0-preview1, 2.1.0 Jan 17, 2018
ajcvickers added a commit that referenced this issue Feb 11, 2018
Issue #9703

Restrict now means that EF will never either set an FK to null or perform a cascade delete if an entity is deleted. This is the same as before, although the exception type may change--it's a negative case and we triaged this break as okay. However, if a navigation property is explicitly changed, then the FK associated with that navigation property will still be set to null. This is the change from the previous behavior. This changes some negative cases into positive cases, which was also triaged as okay.
ajcvickers added a commit that referenced this issue Feb 12, 2018
Issue #9703

Restrict now means that EF will never either set an FK to null or perform a cascade delete if an entity is deleted. This is the same as before, although the exception type may change--it's a negative case and we triaged this break as okay. However, if a navigation property is explicitly changed, then the FK associated with that navigation property will still be set to null. This is the change from the previous behavior. This changes some negative cases into positive cases, which was also triaged as okay.
@ajcvickers ajcvickers added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Feb 12, 2018
@ajcvickers ajcvickers modified the milestones: 2.1.0-preview2, 2.1.0 Nov 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-enhancement
Projects
None yet
Development

No branches or pull requests

2 participants