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

Fix order of sorting for cyclic dependencies #481

Open
wants to merge 5 commits into
base: 1.7.x
Choose a base branch
from

Conversation

esserj
Copy link

@esserj esserj commented Jun 20, 2024

Fix order of sorting for cyclic dependencies where multiple dependencies are assumed to always
go after the cyclic one

In the test case:
Lets assume none of the FK allow cascading deletes

Removing Node 1 would be blocked by Node 2
Removing Node 2 would be blocked by Node 3 and Node 5
Removing Node 3 would be blocked by Node 4 -> cyclic through 4 -> 2 -> 3
Removing Node 4 would be blocked by Node 2 -> cyclic through 2 -> 3 -> 4
Removing node 5 is not blocked by any nodes

So We need to first remove Node 5, which is not blocked by any node
And Node 1 is not part is a cyclic dependency itself, but Node 2 references it, so Node 1 must always come somewhere after Node 2, which is part of a cyclic dep so Node 1 should then come after the other nodes

The rest of the nodes [2, 3, 4] are locked in cyclic dep. so we cannot really resolve them in a favourable way, unless we
try to identify which dependencies are blocking and which ones are not (deleting/nulling FKs or not), only listing the blocking dependencies should also resolve the issue, making the chance of success higher, this is however done by the Purger

@esserj
Copy link
Author

esserj commented Jun 20, 2024

This will fix #329

where multiple dependencies are assumed to always
go after the cyclic one
@esserj
Copy link
Author

esserj commented Jun 20, 2024

It might also fix #127
Because truncate would no longer be done on a table that is still referenced elsewhere

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.

1 participant