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

Wrong table order in delete_instance(recursive=True) causes missing deletion #2893

Closed
stenci opened this issue May 13, 2024 · 3 comments
Closed

Comments

@stenci
Copy link
Contributor

stenci commented May 13, 2024

I have not created an easily reproducible case because I imagine this is a known issue. Please let me know if the behavior I describe below is unknown and unintended, and I will try to build a simple reproducible case.

I am using snapshots, because it's quicker for me to describe the problem. Please let me know if I was too lazy and you need more details to understand the problem.

Calling inventory.delete_instance(recursive=True) deletes the rows in the following order:
image

The rows from cnc_program_sheet (2nd delete) are deleted before cnc_program_part (4th delete), resulting in the failure to delete anything from cnc_program_part, because the multiple level relation to cnc_program_part - cnc_program_sheet - cnc_program - inventory doesn't see the rows previously deleted.

I don't know if the missing deletion is caused by the particular database design or it's a known limitation in peewee.
I know that I have hit the head in the wall for a while, until I checked the SQL log and understood the reason for the failure.

I have now fixed my code: I delete the rows one table at a time in the correct order instead of using the recursive option, but I thought I would report it, just in case I am misusing peewee or you don't know about this problem.

Here is the relation diagram:
image

@coleifer
Copy link
Owner

We do a topological sort to determine delete order. This does look like a bug assuming there's no cycles or something like that, as we do test this behavior: https://github.com/coleifer/peewee/blob/master/tests/models.py#L1596-L1614 -- If you can provide a simple test program that reproduces the issue I'd be grateful but will take a look later.

@coleifer
Copy link
Owner

Nevermind, I'm replicating it. Looks like a bug alright.

coleifer added a commit that referenced this issue May 14, 2024
This is working well, need to adapt the code for the delete instance
code.
coleifer added a commit that referenced this issue May 14, 2024
This could break if models had multiple FKs at different levels. Code is
also somewhat simplified.

Fixes #2893
coleifer added a commit that referenced this issue May 14, 2024
This could break if models had multiple FKs at different levels. Code is
also somewhat simplified.

Fixes #2893
@coleifer
Copy link
Owner

This should be fixed now, thank you for the report.

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

No branches or pull requests

2 participants