Skip to content

Stop allowing identifying relationships from overriding cascade behavior#30213

Merged
ajcvickers merged 1 commit intomainfrom
ThrowOutTheAlternateRock0204
Feb 7, 2023
Merged

Stop allowing identifying relationships from overriding cascade behavior#30213
ajcvickers merged 1 commit intomainfrom
ThrowOutTheAlternateRock0204

Conversation

@ajcvickers
Copy link
Copy Markdown
Contributor

Fixes #28961

In the issue, an alternate key was added over the the foreign key to make it effectively an identifying relationship. That is, changing the relationship changes the identity of the dependent. This resulted in the special handling for identifying relationships to kick in, which didn't respect the configured DeleteBehavior. This change fixes that.

Fixes #28961

In the issue, an alternate key was added over the the foreign key to make it effectively an identifying relationship. That is, changing the relationship changes the identity of the dependent. This resulted in the special handling for identifying relationships to kick in, which didn't respect the configured DeleteBehavior. This change fixes that.
@ajcvickers ajcvickers requested a review from a team February 5, 2023 11:56
@ajcvickers ajcvickers merged commit 9504711 into main Feb 7, 2023
@ajcvickers ajcvickers deleted the ThrowOutTheAlternateRock0204 branch February 7, 2023 10:46
@mrpmorris
Copy link
Copy Markdown

Hi @ajcvickers

How does this merge affect the desired behaviours requested in #28961 and #10066?

Will it allow us to prevent a parent row from being deleted, and automatically delete any child row that is disassociated from its parent?

@ajcvickers
Copy link
Copy Markdown
Contributor Author

@mrpmorris It fixes #28961. It's not directly related to #10066.

@mrpmorris
Copy link
Copy Markdown

mrpmorris commented Feb 10, 2023

@ajcvickers If I am not mistaken, this is a behaviour I was relying on. We discussed it on Twitter.

https://twitter.com/MrPeterLMorris/status/1564909033315880961

You'll need to read it in an incognito window, for some reason you have me blocked.

Will this no longer work?

@ajcvickers
Copy link
Copy Markdown
Contributor Author

Yes, but this is a bug. I guess it's the classic case of a bug being used as a feature.

@mrpmorris
Copy link
Copy Markdown

I wasn't complaining, just checking how it'll affect me. Thanks for answering :)

ajcvickers added a commit that referenced this pull request Dec 2, 2023
…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 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.
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.

Orphans are deleted when overlapping composite alternate key is mutated

3 participants