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

Revert behavior to throw when attempting to modify an unconstrained alternate key property #32492

Merged
merged 1 commit into from
Dec 5, 2023

Conversation

ajcvickers
Copy link
Member

@ajcvickers ajcvickers commented Dec 2, 2023

Fixes #32383
Reverts #30213 for #28961

In #32385, an unconstrained alternate key was added to the model purely to make a non-identifying relationship artificially identifying. #30213 attempted to fix this by throwing that the key was being modified. However, this scenario is very similar to the case for a many-to-many join type, where the composite primary key is also not the end of any relationship, but forces the two many-to-one relationships to be identifying.

I prepared a PR that would only throw if the key involved is alternate, but on reflection that doesn't seem like an appropriate distinction to make. So overall, I think we should just revert this change, which is what this PR does.

…lternate key property

Fixes #28961
Reverts #30213 for #32385

In #32385, an unconstrained alternate key was added to the model purely to make a non-identifying relationship artificially identifying. #30213 attempted to fix this by throwing that the key was being modified. However, this scenario is very similar to the case for a many-to-many join type, where the composite primary key is also not the end of any relationship, but forces the two many-to-one relationships to be identifying.

I prepared a PR that would only throw if the key involved is alternate, but on reflection that doesn't seem like an appropriate distinction to make. So overall, I think we should just revert this change, which is what this PR does.
@ajcvickers
Copy link
Member Author

Will port to 8.0 for servicing consider.

@ajcvickers ajcvickers requested a review from a team December 2, 2023 11:03
@ajcvickers
Copy link
Member Author

Note from triage: agreed to revert the change and not special case alternate keys over primary keys. Which is what this PR does. :-)

@ajcvickers
Copy link
Member Author

@dotnet/efteam Can I get a review on this so I can merge and prepare the patch PR?

@ajcvickers ajcvickers merged commit 9d84cf4 into main Dec 5, 2023
7 checks passed
@ajcvickers ajcvickers deleted the 231202_LessAggressive branch December 5, 2023 18:30
ajcvickers added a commit that referenced this pull request Dec 5, 2023
…lternate key property (#32492)

Fixes #28961
Reverts #30213 for #32385

In #32385, an unconstrained alternate key was added to the model purely to make a non-identifying relationship artificially identifying. #30213 attempted to fix this by throwing that the key was being modified. However, this scenario is very similar to the case for a many-to-many join type, where the composite primary key is also not the end of any relationship, but forces the two many-to-one relationships to be identifying.

I prepared a PR that would only throw if the key involved is alternate, but on reflection that doesn't seem like an appropriate distinction to make. So overall, I think we should just revert this change, which is what this PR does.
wtgodbe pushed a commit that referenced this pull request Jan 3, 2024
…nconstrained alternate key property (#32523)

* Revert behavior to throw when attempting to modify an unconstrained alternate key property (#32492)

Fixes #28961
Reverts #30213 for #32385

In #32385, an unconstrained alternate key was added to the model purely to make a non-identifying relationship artificially identifying. #30213 attempted to fix this by throwing that the key was being modified. However, this scenario is very similar to the case for a many-to-many join type, where the composite primary key is also not the end of any relationship, but forces the two many-to-one relationships to be identifying.

I prepared a PR that would only throw if the key involved is alternate, but on reflection that doesn't seem like an appropriate distinction to make. So overall, I think we should just revert this change, which is what this PR does.

* Quirk
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[EF Core 8] [Breaking Change] - error on working with explicit many-to-many relations with OnDelete.Restrict
2 participants